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

[Richard Smith]

On Thu, Feb 21, 2013 at 3:11 PM, Ismail Pazarbasi <
ismail.pazarbasi at gmail.com> wrote:

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


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.


> >
> >> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130221/a1b81eaf/attachment.html>