On Thu, Feb 21, 2013 at 3:11 PM, Ismail Pazarbasi <span dir="ltr"><<a href="mailto:ismail.pazarbasi@gmail.com" target="_blank">ismail.pazarbasi@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Thu, Feb 21, 2013 at 11:34 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
> Hi!<br>
><br>
> On Thu, Feb 21, 2013 at 1:53 PM, Ismail Pazarbasi<br>
> <<a href="mailto:ismail.pazarbasi@gmail.com">ismail.pazarbasi@gmail.com</a>> wrote:<br>
>><br>
>> Hi,<br>
>><br>
>> when operand of a noexcept operator in a friend function definition<br>
>> involves a member of a template class enclosing the friend definition,<br>
>> Clang casts current context to either a CXXMethodDecl or a<br>
>> CXXRecordDecl. Since friend function is neither of these, cast fails.<br>
>> The same test in a non-template class works fine.<br>
>><br>
>> The first patch tries to retrieve the lexical parent of the function<br>
>> definition (per class.friend/p6-7), which is expected to be a record<br>
>> type. A test is also provided (also, pasting below for reference).<br>
>><br>
>> template<typename T><br>
>> class noexcept_member_operand_at_friend_definition {<br>
>>   T v_;<br>
>>   friend int add_to_v(noexcept_member_operand_at_friend_definition &t)<br>
>> noexcept(noexcept(v_ + 42))<br>
>>   {<br>
>>     return t.v_ + 42;<br>
>>   }<br>
>> };<br>
>><br>
>> void this_expr()<br>
>> {<br>
>>   noexcept_member_operand_at_friend_definition<int> t;<br>
>>   add_to_v(t);<br>
>> }<br>
><br>
><br>
> Thanks for looking into this. There is a more fundamental problem here,<br>
> which this patch is masking rather than fixing. Per 5.1.1/3, 'this' is not<br>
> permitted to appear in the declaration of a friend function, so per 9.3.1/3,<br>
> we should not transform the mention of 'v_' into a class member access.<br>
> Therefore, we should build a DeclRefExpr for the member, not a<br>
> CXXMemberExpr, in this case. It looks like the bug is that<br>
> Parser::ParseFunctionDeclarator's IsCXX11MemberFunction is not taking<br>
> 'friend' into account when determining whether a function in a class is a<br>
> member function.<br>
><br>
> To demonstrate the difference, here's a testcase which we should reject, but<br>
> we currently accept (and still accept with your patch):<br>
><br>
> struct S {<br>
>   friend auto f() -> decltype(this);<br>
> };<br>
</div></div>Thank you for reviewing these.<br>
<br>
I first thought we should have rejected this and the case with a<br>
non-template class as well. But both GCC and Clang accepted, and I<br>
honestly started to look what am I missing. I found class.friend/p6-7,<br>
which has nothing to do with 'this' pointer, but only lexical scope;<br>
so I thought name lookup can find identifier in the class' scope, and<br>
because 'this' is not used inside the function definition itself, it<br>
might be legal to use 'this' inside noexcept operator (which is<br>
clearly not permitted by 5.1.1/3).</blockquote><div><br></div><div>Your example is valid. While you can't use 'this' in a friend declaration, you *can* use 'v_', because noexcept is an unevaluated context; see 5.1.1/13.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
><br>
>> The second patch is a minor one; it merges two if statements that have<br>
>> the same condition.<br>
><br>
><br>
> I don't think this patch is correct. Note that the body of the first 'if'<br>
> statement can cause the condition of the second to no longer be 'true'.<br>
</div>Yes, you are right!<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>