[PATCH] Reject 'this' in typedefs

Harald van Dijk harald at gigawatt.nl
Thu Jan 16 15:49:00 PST 2014


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.

Any other thoughts or comments on this patch?

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 --------------
A non-text attachment was scrubbed...
Name: 0001-Reject-this-in-typedefs.patch
Type: text/x-patch
Size: 1514 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140117/2a33d953/attachment.bin>


More information about the cfe-commits mailing list