[PATCH] Fix for Bug 15290 - Segmentation fault during template instantiation

[Ismail Pazarbasi]

On Thu, Feb 21, 2013 at 11:34 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> Hi!
>
> On Thu, Feb 21, 2013 at 1:53 PM, Ismail Pazarbasi
> <ismail.pazarbasi at gmail.com> wrote:
>>
>> Hi,
>>
>> when operand of a noexcept operator in a friend function definition
>> involves a member of a template class enclosing the friend definition,
>> Clang casts current context to either a CXXMethodDecl or a
>> CXXRecordDecl. Since friend function is neither of these, cast fails.
>> The same test in a non-template class works fine.
>>
>> The first patch tries to retrieve the lexical parent of the function
>> definition (per class.friend/p6-7), which is expected to be a record
>> type. A test is also provided (also, pasting below for reference).
>>
>> template<typename T>
>> class noexcept_member_operand_at_friend_definition {
>>   T v_;
>>   friend int add_to_v(noexcept_member_operand_at_friend_definition &t)
>> noexcept(noexcept(v_ + 42))
>>   {
>>     return t.v_ + 42;
>>   }
>> };
>>
>> void this_expr()
>> {
>>   noexcept_member_operand_at_friend_definition<int> t;
>>   add_to_v(t);
>> }
>
>
> Thanks for looking into this. There is a more fundamental problem here,
> which this patch is masking rather than fixing. Per 5.1.1/3, 'this' is not
> permitted to appear in the declaration of a friend function, so per 9.3.1/3,
> we should not transform the mention of 'v_' into a class member access.
> Therefore, we should build a DeclRefExpr for the member, not a
> CXXMemberExpr, in this case. It looks like the bug is that
> Parser::ParseFunctionDeclarator's IsCXX11MemberFunction is not taking
> 'friend' into account when determining whether a function in a class is a
> member function.
>
> To demonstrate the difference, here's a testcase which we should reject, but
> we currently accept (and still accept with your patch):
>
> struct S {
>   friend auto f() -> decltype(this);
> };
Thank you for reviewing these.

I first thought we should have rejected this and the case with a
non-template class as well. But both GCC and Clang accepted, and I
honestly started to look what am I missing. I found class.friend/p6-7,
which has nothing to do with 'this' pointer, but only lexical scope;
so I thought name lookup can find identifier in the class' scope, and
because 'this' is not used inside the function definition itself, it
might be legal to use 'this' inside noexcept operator (which is
clearly not permitted by 5.1.1/3).

>
>> The second patch is a minor one; it merges two if statements that have
>> the same condition.
>
>
> I don't think this patch is correct. Note that the body of the first 'if'
> statement can cause the condition of the second to no longer be 'true'.
Yes, you are right!