[cfe-commits] pr12328 PATCH: Friend method declaration doesn't require method to be visible

[Stepan Dyatkovskiy]

Hi, John.
After-all, it seems that we needn't insert additional check in 
SemaTemplateInstantiateDecl ?

-Stepan.

Stepan Dyatkovskiy wrote:
> Oh.. You right of course. Anyway I want to figure that clang doesn't
> performs checks in depended declaration contexts.
> BTW, clang can't detect error in example below, while gcc does:
>
> template<class T>
> class X {
>     void f();
> };
>
> template<class T>
> class Y {
>     friend void X<T>::not_found();
>     void f() {
>       X<T>  x;
>       x.not_found();
>     }
> };
>
> int main() {
>     Y<int>  y; // GCC: error: no ‘void X<int>::not_found()’
>               // member function declared in class ‘X<int>’
>     return 0;
> }
>
> -Stepan
>
> David Blaikie wrote:
>> On Wed, Apr 25, 2012 at 7:52 AM, Stepan Dyatkovskiy<stpworld at narod.ru>   wrote:
>>> Hi John. I'm afraid that it will problematically to fix it for templates
>>> right now. Consider this example:
>>>
>>> template<class T>
>>> class X {
>>>     void f();
>>> };
>>>
>>> template<class T>
>>> class Y {
>>>     friend void X<T>::f();
>>> };
>>>
>>> If I'm get right here friend is declared in depended context. I need
>>> learn more how to lookup previous declarations in depended contexts.
>>> Currently in clang doesn't checks anything for friend declarations like
>>> this.
>>> Even
>>>
>>> friend void X<T>::uh_man_i_didnt_see_u_b4();
>>>
>>> will compiled.
>>
>> Sorry, I haven't been following this whole thread - but I believe
>> these specific examples (where X is a template as above, not the cases
>> below where X is a non-template) are currently correct. You can't fail
>> to compile the examples you gave because X<T>   might have explicit
>> specializations for some particular T that does have "void
>> uh_man_i_didnt_see_u_b4()" for example. And so long as you only
>> instantiate Y<T>   with Ts such that X<T>'s specializations have those
>> operations, I would expect it should compile.
>>
>> But perhaps I've missed some detail here,
>> - David
>>
>>> Also note, that it also compiled with GCC without any errors. GCC also
>>> doesn't detect errors in next example, while clang does (after fix):
>>>
>>> // NOT a template.
>>> class X {
>>>     void f();
>>> };
>>>
>>> template<class T>
>>> class Y {
>>>     friend void X<T>::f(); // GCC passes. clang will emit
>>>                            // error after my patch.
>>>     friend void X<T>::f_not_found(); // GCC passes. clang emits error.
>>>
>>> };
>>>
>>> In short. I'd glad to write a fix when both X and Y are templates, but I
>>> need more time and I propose to do it in separated patch.
>>>
>>> -Stepan.
>>>
>>> Stepan Dyatkovskiy wrote:
>>>> Hi John. Ok, I propose two versions then.
>>>>
>>>> 1. Add "bool SuppressDelayedDiagnostics" param to the CheckLookupAccess
>>>> method, that will "false" by default. IMHO looks like a workaround.
>>>> 2. Extend DelayedDiagnostics with enable() feature.
>>>>
>>>> #1 is attached as pr12328.patch
>>>> #2 is attached as pr12328-2.patch
>>>>
>>>> Also I attached new + fixed tests in pr12328-testcases.patch.
>>>>
>>>> -Stepan.
>>>>
>>>> John McCall wrote:
>>>>> On Apr 19, 2012, at 3:45 AM, Stepan Dyatkovskiy wrote:
>>>>>> ping.
>>>>>> Stepan Dyatkovskiy wrote:
>>>>>>> ping.
>>>>>>> Stepan Dyatkovskiy wrote:
>>>>>>>> ping.
>>>>>>>> Stepan Dyatkovskiy wrote:
>>>>>>>>> Hi all.
>>>>>>>>> Please find patch in attachment for review.
>>>>>>>>> I suppose that this check was not implemented yet, since I couldn't
>>>>>>>>> found it anywhere. The friend method availability check was
>>>>>>>>> inserted in
>>>>>>>>> Sema::ActOnFriendFunctionDecl.
>>>>>>>>> I also added "DeclContext *FromContext" to the AccessedEntity
>>>>>>>>> class. It
>>>>>>>>> allows to ask DelayedDiagnostic to use context that was "current"
>>>>>>>>> when
>>>>>>>>> this diagnostics was requested. It is usefull in our case, since we
>>>>>>>>> requested diagnostics when we're parsing class with "friend"
>>>>>>>>> declaration, but diagnostics is invoked for class with definition of
>>>>>>>>> friend member itself.
>>>>>
>>>>> Sorry for the delay.
>>>>>
>>>>> Please include test cases when you submit patches. Also, I'm afraid your
>>>>> patch is out-of-date.
>>>>>
>>>>> + if (Previous.getResultKind() == LookupResult::Found&&
>>>>> + Previous.getFoundDecl()->isCXXClassMember())
>>>>> + CheckLookupAccess(Previous);
>>>>>
>>>>> This is a bit silly, because we know exactly what we're checking
>>>>> access to,
>>>>> and we're getting nothing out of delaying the diagnostic. It would be
>>>>> better
>>>>> to provide a different CheckFriendAccess entrypoint that happens to
>>>>> suppress diagnostic delay.
>>>>>
>>>>> Also, you'll need to do this same access check in
>>>>> SemaTemplateInstantiateDecl
>>>>> so that it happens in template instantiations.
>>>>>
>>>>> John.
>>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits