[PATCH] Using private types should be allowed as template parametter for explicit instantiation

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Using private types should be allowed as template parametter for explicit instantiation

Olivier Goffart
Hello.

Attached you will find my attempt to fix
http://llvm.org/bugs/show_bug.cgi?id=7267

The problem is that in this code

class X { class Y; };
template<class T> struct Test {};
template<> struct Test<X::Y> {};

Clang would complains that X::Y is private.

While the C++ specification 14.7.2 [temp.explicit] p11
says it should compile (The usual access checking rules do not apply to names
used to specify explicit instantiations)

(This break compilation of Qt's linguist tool)


The patch is only one line but was not trivial.
By having a ParsingDeclRAIIObject while parsing that kind of close, we make
sure any diagnostic are delayed. And because I do not call complete() on it,
the delayed diagnostic will not be issued.

Does my patch makes sens this time?

Regards.
--
Olivier

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

bug7267.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Using private types should be allowed as template parametter for explicit instantiation

Olivier Goffart
On Thursday 17 June 2010, Olivier Goffart wrote :

> Hello.
>
> Attached you will find my attempt to fix
> http://llvm.org/bugs/show_bug.cgi?id=7267
>
> The problem is that in this code
>
> class X { class Y; };
> template<class T> struct Test {};
> template<> struct Test<X::Y> {};
>
> Clang would complains that X::Y is private.
>
> While the C++ specification 14.7.2 [temp.explicit] p11
> says it should compile (The usual access checking rules do not apply to
> names used to specify explicit instantiations)
>
> (This break compilation of Qt's linguist tool)
>
>
> The patch is only one line but was not trivial.
> By having a ParsingDeclRAIIObject while parsing that kind of close, we make
> sure any diagnostic are delayed. And because I do not call complete() on
> it, the delayed diagnostic will not be issued.
>
> Does my patch makes sens this time?
>
> Regards.
Ping?

(btw, this is the right list for patches, right?)


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

bug7267.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Using private types should be allowed as template parametter for explicit instantiation

John McCall

On Jun 22, 2010, at 11:39 AM, Olivier Goffart wrote:

> On Thursday 17 June 2010, Olivier Goffart wrote :
>> Hello.
>>
>> Attached you will find my attempt to fix
>> http://llvm.org/bugs/show_bug.cgi?id=7267
>>
>> The problem is that in this code
>>
>> class X { class Y; };
>> template<class T> struct Test {};
>> template<> struct Test<X::Y> {};
>>
>> Clang would complains that X::Y is private.
>>
>> While the C++ specification 14.7.2 [temp.explicit] p11
>> says it should compile (The usual access checking rules do not apply to
>> names used to specify explicit instantiations)
>>
>> (This break compilation of Qt's linguist tool)
>>
>>
>> The patch is only one line but was not trivial.
>> By having a ParsingDeclRAIIObject while parsing that kind of close, we make
>> sure any diagnostic are delayed. And because I do not call complete() on
>> it, the delayed diagnostic will not be issued.
>>
>> Does my patch makes sens this time?
>>
>> Regards.
>
> Ping?

Sorry, missed this the last time.  This patch won't work for several reasons.

1.  You're suppressing access checks whenever ParseTemplateIdAfterTemplateName
parses a template argument list.  That's actually a large number of cases, most of
which are not explicit instantiations.  It happens to be the case that certain very
common cases follow a different code path, which is why your test case works,
but there are many other cases where this breaks things.

2.  You're suppressing access checks for explicit specializations, not just explicit
instantiations.  I see no justification of that in the standard.

3.  You're only suppressing access checks in the template arguments, not in the
template name itself (it could be a private member template) or in the associated
types (if it's a function template).  *All* access checks in the declaration need to be
suppressed.

4.  The use of ParsingDeclRAIIObject is clever, but I suspect it won't work as
desired for explicit instantiations of function templates.  If so, you'll need to make
a different mechanism which just completely suppresses access checks.  Fortunately,
it doesn't need to be recursive;  I think you can just make a new Action method to
enable/disable access checks, have the method set a flag on Sema, and have
CheckAccess in SemaAccess.cpp check that flag (right before the code about
delaying diagnostics when parsing declarators).

You can trigger the new mechanism in ParseExplicitInstantiation.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Using private types should be allowed as template parametter for explicit instantiation

Olivier Goffart
Le Tuesday 22 June 2010, John McCall a écrit :

> On Jun 22, 2010, at 11:39 AM, Olivier Goffart wrote:
> > On Thursday 17 June 2010, Olivier Goffart wrote :
> >> Hello.
> >>
> >> Attached you will find my attempt to fix
> >> http://llvm.org/bugs/show_bug.cgi?id=7267
> >>
> >> The problem is that in this code
> >>
> >> class X { class Y; };
> >> template<class T> struct Test {};
> >> template<> struct Test<X::Y> {};
> >>
> >> Clang would complains that X::Y is private.
> >>
> >> While the C++ specification 14.7.2 [temp.explicit] p11
> >> says it should compile (The usual access checking rules do not apply to
> >> names used to specify explicit instantiations)
> >>
> >> (This break compilation of Qt's linguist tool)
> >>
> >>
> >> The patch is only one line but was not trivial.
> >> By having a ParsingDeclRAIIObject while parsing that kind of close, we
> >> make sure any diagnostic are delayed. And because I do not call
> >> complete() on it, the delayed diagnostic will not be issued.
> >>
> >> Does my patch makes sens this time?
> >>
> >> Regards.
> >
> > Ping?
>
> Sorry, missed this the last time.  This patch won't work for several
> reasons.
>
> 1.  You're suppressing access checks whenever
> ParseTemplateIdAfterTemplateName parses a template argument list.  That's
> actually a large number of cases, most of which are not explicit
> instantiations.  It happens to be the case that certain very common cases
> follow a different code path, which is why your test case works, but there
> are many other cases where this breaks things.

Would you have examples of that?

>
> 2.  You're suppressing access checks for explicit specializations, not just
> explicit instantiations.  I see no justification of that in the standard.

I  see explicit specializations as a special case of explicit instantiations

This is specially the case I have error with.
How else do you define traits for private types?

All the other compilers support that. And even clang already support it
already for function explicit specializations, only the class is missing.

> 3.  You're only suppressing access checks in the template arguments, not in
> the template name itself (it could be a private member template) or in the
> associated types (if it's a function template).  *All* access checks in
> the declaration need to be suppressed.

Function parameters case is taken elsewhere; (it already works for functions.)


> 4.  The use of ParsingDeclRAIIObject is clever, but I suspect it won't work
> as desired for explicit instantiations of function templates.

It work with function template and this is why i used it there.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Using private types should be allowed as template parametter for explicit instantiation

John McCall

On Jun 23, 2010, at 1:17 AM, Olivier Goffart wrote:

> Le Tuesday 22 June 2010, John McCall a écrit :
>> On Jun 22, 2010, at 11:39 AM, Olivier Goffart wrote:
>>> On Thursday 17 June 2010, Olivier Goffart wrote :
>>>> Hello.
>>>>
>>>> Attached you will find my attempt to fix
>>>> http://llvm.org/bugs/show_bug.cgi?id=7267
>>>>
>>>> The problem is that in this code
>>>>
>>>> class X { class Y; };
>>>> template<class T> struct Test {};
>>>> template<> struct Test<X::Y> {};
>>>>
>>>> Clang would complains that X::Y is private.
>>>>
>>>> While the C++ specification 14.7.2 [temp.explicit] p11
>>>> says it should compile (The usual access checking rules do not apply to
>>>> names used to specify explicit instantiations)
>>>>
>>>> (This break compilation of Qt's linguist tool)
>>>>
>>>>
>>>> The patch is only one line but was not trivial.
>>>> By having a ParsingDeclRAIIObject while parsing that kind of close, we
>>>> make sure any diagnostic are delayed. And because I do not call
>>>> complete() on it, the delayed diagnostic will not be issued.
>>>>
>>>> Does my patch makes sens this time?
>>>>
>>>> Regards.
>>>
>>> Ping?
>>
>> Sorry, missed this the last time.  This patch won't work for several
>> reasons.
>>
>> 1.  You're suppressing access checks whenever
>> ParseTemplateIdAfterTemplateName parses a template argument list.  That's
>> actually a large number of cases, most of which are not explicit
>> instantiations.  It happens to be the case that certain very common cases
>> follow a different code path, which is why your test case works, but there
>> are many other cases where this breaks things.
>
> Would you have examples of that?

Just look for callers of AnnotateTemplateIdToken (as opposed to
AnnotateTemplateIdTokenAsType).  You're suppressing access checks in
all those cases.

It's just not appropriate to hack a general routine like this, even if it does
appear to work for all the cases that you personally care about.

>> 2.  You're suppressing access checks for explicit specializations, not just
>> explicit instantiations.  I see no justification of that in the standard.
>
> I  see explicit specializations as a special case of explicit instantiations

Nevertheless, they aren't.

> This is specially the case I have error with.
> How else do you define traits for private types?

That is an excellent question!  It certainly seems like a defect.

> All the other compilers support that. And even clang already support it
> already for function explicit specializations, only the class is missing.

A lot of compilers support an awful lot of things by accident.  e.g. clang
implements the correct semantics here for function templates because it
parses them as out-of-line function declarations and thus enters the
specialized context before performing access checks.  So as long as the
original declaration is well-formed....

Anyway, I have no problem with supporting this for specialization as an
extension.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev