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

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.

bug7267.diff (1.46 KB)

Ping?

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

bug7267.diff (1.46 KB)

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.

>> 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.

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.