[PATCH] Allow implicit 'this' in static member function declarations, add extra tests for use of 'this'

Harald van Dijk harald at gigawatt.nl
Sat Jan 18 06:15:24 PST 2014


On 18/01/14 13:39, Harald van Dijk wrote:
> Hi all,
> 
> Looking closer at the differences between 'this' in static member
> function declarations, and non-member function declarations, I was able
> to come up with a few more cases that highlight the special treatment it
> needs, and hopefully prevent others from making the same mistake I did.
> I no longer think it makes sense to merge the two separate checks for
> 'this', at least not right now.
> 
> Allowing implicit 'this', as noted in the test suite already, is
> trivially done, especially given that the check to see whether the use
> is implicit already existed.

However, it is incorrect, and the patch as it stands should not be
applied. Sorry for the noise.

References to non-static data members are treated differently in the
standard from references to non-static member functions, and even though
the unevaluated calls to non-static member functions do not violate the
rules on 'this', it has been pointed out to me that they are still
invalid for another reason. It may make more sense to pretend in clang
that it's the use of 'this' that's invalid, as the effect is still the
same. I hope to come up with a revised patch soon.

Cheers,
Harald van Dijk

> I was unfortunately unable to come up with a testcase that uses 'this'
> (whether explicitly or implicitly) in a C++11 attribute for a member
> function declaration, so I also cannot tell whether
> checkThisInStaticMemberFunctionAttributes is still needed. Removing it
> doesn't make any test fail, but that isn't telling much.
> 
> Alp, is this the sort of test suite modification you were asking for?
> 
> Cheers,
> Harald van Dijk
> 
>> On 17/01/14 01:03, Alp Toker wrote:
>>> On 16/01/2014 06: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.
>>>
>>> Hi Harald,
>>>
>>> If you have the time could you throw together a couple of tests for
>>> those cases as well?
>>>
>>> checkThisInStaticMemberFunctionAttributes() looks like it can be dropped
>>> now that RecursiveASTVisitor visits attributes (r198224) so I'm hopeful
>>> we can still apply the bulk of the cleanup from your original patch
>>> without regressing.




More information about the cfe-commits mailing list