On Fri, Feb 22, 2013 at 12:42 AM, Ismail Pazarbasi <[email protected]> wrote: > On Fri, Feb 22, 2013 at 12:18 AM, Richard Smith <[email protected]> wrote: >> On Thu, Feb 21, 2013 at 3:11 PM, Ismail Pazarbasi >> <[email protected]> wrote: >>> >>> On Thu, Feb 21, 2013 at 11:34 PM, Richard Smith <[email protected]> >>> wrote: >>> > Hi! >>> > >>> > On Thu, Feb 21, 2013 at 1:53 PM, Ismail Pazarbasi >>> > <[email protected]> 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). >> >> >> 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. > > OK, thank you very much, I think I understood it. I will check what > can I do about IsCXX11MemberFunction function first. > >>> >>> > >>> >> 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! >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >>
Hi again, As you said, IsCXX11MemberFunction was not taking 'friend' specifier into account. I have added the check, and expression is not transformed to a class member access expression. Thanks!
iscxx11memberfunction-friend-missing.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
