r180127 - [libclang] Introduce a CXCursor_ObjCSelfExpr cursor, which is the equivalent of CXCursor_CXXThisExpr for C++ code.

[Argyrios Kyrtzidis]

Hi Jordan,

Sorry for the long delay in responding..

On Apr 23, 2013, at 3:17 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Apr 23, 2013, at 13:46 , Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> 
>> On Apr 23, 2013, at 12:37 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>> 
>>> 
>>> On Apr 23, 2013, at 12:16 , Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>>> 
>>>> On Apr 23, 2013, at 12:03 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>>>> 
>>>>> Sorry, why is this right way to implement this? 'self' is very deliberately not a special kind of expression in Objective-C. Why not expose this on the associated ParmVarDecl instead?
>>>> 
>>>> What is the advantage with that approach ? Whether there is a ParmVarDecl or not seems like an implementation detail of the AST. If you are worried that there is one more cursor to consider, I'd argue the same applies for CXCursor_CXXThisExpr.
>>> 
>>> CXXThisExpr matches the AST, which matches the C++ standard. There is no Objective-C standard, but certainly 'self' is not treated specially in most cases.
>> 
>> Hmm, since 'self' is not declared anywhere in the source, I consider it somewhat "special".
>> 
>>> 
>>> I can't put my finger on it, but it makes a lot more sense to me to say "is this variable 'self'?"
>> 
>> What are you suggesting exactly, have "self" be a reference of a CXCursor_ParmDecl and introduce a libclang function to answer "is this parameter 'self'" ?
>> 
>> AFAIK you'd only get such a variable via a "'self' ref expression"; there is no other case where libclang is going to return such a thing, so such a function has limited utility.
>> And unless we start exposing "self" as a CXCursor_ParmDecl "child" of a method (which I don't like) it seems inconsistent that we would report as referenced a ParmDecl that we never reported before in the source.
> 
> Hm. That's different from the Clang ASTs.

For more context, 'self' decl is an ImplicitParamDecl, which is CXCursor_UnexposedDecl. You could only come upon this "unexposed decl" by calling clang_getCursorReferenced on the DeclRefExpr cursor.

> I guess you don't have a Decl for _cmd either?

This is so rare that I'm fine with it working as I described above.

> 
> But that makes sense—returning a Decl you won't see any other way doesn't feel consistent.
> 
> Now if you had something like liveness analysis, or "find all variables of this type", then it might be interesting to have 'self' as an explicit Decl. 
> 
> 
>>> than "is this expression 'self'?", especially in init methods where 'self' can be reassigned. 'this' can never be reassigned.
>> 
>> Not sure why CXCursor_ObjCSelfExpr conflicts with 'self' getting reassigned ?
> 
> It doesn't, but it seems funny to me to think of it as a special type of expression when it accepts assignment. Then again, so do ObjCPropertyRefExprs.
> 
> Actually, it could be important for dependency analysis questions: "what variables might affect the value of this expression?". But now I'm talking about potential future APIs as justification for a decision now, which might not be worth it.

I looked into it a bit more, and I'm afraid that it is a bit disruptive to start making 'self' a visible parameter of all methods. We may revisit this in the future but currently I'm uncomfortable making such a change.

> 
> Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130429/424a8242/attachment.html>