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

[Stepan Dyatkovskiy]

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