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

[Stepan Dyatkovskiy]

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