[PATCH] Reject 'this' in typedefs

[Richard Smith]

On Thu Jan 16 2014 at 3:49:11 PM, Harald van Dijk <harald at gigawatt.nl>
wrote:

> I was playing around with the idea of tracking where 'this' is seen, in
> order to properly diagnose it after determining whether the function
> definition is static, but it needs some more work, and regardless, it
> seems too invasive when the point of my current patch was actually
> something else.
>
> How about just rejecting 'this' in typedefs, then? These don't have the
> same problem and so don't need the same special treatment as 'static'.
> I've rebased, retested, and all tests pass.
>
> I've changed the check for typedefs to be performed regardless of
> declarator context. What I had, when taking the invalid testcase
>
> struct Foo { };
> namespace Bar { };
> typedef auto Foo::f() -> decltype(this);
> typedef auto Bar::f() -> decltype(this);
>
> generated one error on line three, but two on line four:
>
> test.cc:3:19: error: typedef declarator cannot be qualified
> typedef auto Foo::f() -> decltype(this);
>              ~~~~~^
> test.cc:4:35: error: invalid use of 'this' outside of a non-static
> member function
> typedef auto Bar::f() -> decltype(this);
>                                   ^
> test.cc:4:19: error: typedef declarator cannot be qualified
> typedef auto Bar::f() -> decltype(this);
>              ~~~~~^
>
> Ideally, the two lines would give the same errors, as they both use
> 'this' outside of a non-static member function, and that's what I've now
> implemented, but I have no strong feelings either way about that.
>

I think consistently issuing two distinct errors here is a step forward.


> Any other thoughts or comments on this patch?
>

Patch LGTM, thanks! Do you need someone to commit this for you?


> Cheers,
> Harald van Dijk
>
> On 16/01/14 07:38, Harald van Dijk wrote:
> > Ah, thanks, you're right. If the in-class declaration doesn't use
> > 'this', but the definition does, then 'this' would not be diagnosed with
> > my approach. Normally, if the definition does, so would the in-class
> > declaration, where it would be diagnosed, but an example of where it
> > doesn't is
> >
> > struct Foo {
> >   static bool f();
> > };
> >
> > auto Foo::f() -> decltype(this != 0) {
> >   return true;
> > }
> >
> > Cheers,
> > Harald van Dijk
> >
> > On 16/01/14 01:25, Richard Smith wrote:
> >> How does this deal with the case where 'this' appears in a declarator
> >> for an out-of-line definition of a static member function? In that case,
> >> we don't know we're parsing a static member function declaration until
> >> *after* we've already parsed the 'this' expression. I believe the
> >> existing complexity was aimed at correctly handling that case; it seems
> >> surprising that we don't have any tests for it.
> >>
> >> On Wed Jan 15 2014 at 3:33:07 PM, Harald van Dijk <harald at gigawatt.nl
> >> <mailto:harald at gigawatt.nl>> wrote:
> >>
> >>     Hi all,
> >>
> >>     This came up on StackOverflow recently:
> >>
> >>     'this' is rejected by clang in static member functions:
> >>
> >>     error: 'this' cannot be used in a static member function declaration
> >>       static auto f() -> decltype(this);
> >>                                   ^
> >>
> >>     However, what the standard actually says is that 'this' is not
> allowed
> >>     except in non-static member functions (and some more bits not
> relevant
> >>     here). Declarations that look like member functions but aren't, not
> even
> >>     static ones, shouldn't allow 'this' either.
> >>
> >>       typedef auto f() -> decltype(this);
> >>
> >>     Looking to see what clang implements, I was very surprised. There is
> >>     already perfectly functional code that rejects 'this' in 'friend'
> >>     functions, and that code is not reused for 'static' functions (and
> >>     'typedef's): instead, the checks have effectively been rewritten for
> >>     'static'.
> >>
> >>     I'm wondering, why is that? If I do attempt to re-use the existing
> code,
> >>     as in the attached patch, then the only changes in the test results
> are
> >>     actually correct, as references to non-static data members are
> permitted
> >>     even in 'static' functions, so long as they appear in unevaluated
> >>     expressions. There is even a FIXME comment about this in the test.
> But
> >>     I'm sure the current checks have been added for a good reason. Am I
> >>     overlooking some important details that are not covered by the
> >>     testsuite?
> >>
> >>     Cheers,
> >>     Harald van Dijk
> >>     _________________________________________________
> >>     cfe-commits mailing list
> >>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> >>     http://lists.cs.uiuc.edu/__mailman/listinfo/cfe-commits
> >>     <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/20140117/42876346/attachment.html>