[cfe-commits] r44156 - in /cfe/trunk: Driver/RewriteTest.cpp Sema/SemaExpr.cpp include/clang/AST/Expr.h

[Chris Lattner]

>>>       if (CurMethodDecl) {
>>>         ObjcInterfaceDecl *IFace = CurMethodDecl- 
>>> >getClassInterface();
>>>         ObjcInterfaceDecl *clsDeclared;
>>>         if (ObjcIvarDecl *IV = IFace->lookupInstanceVariable(&II,  
>>> clsDeclared))
>>>           return new ObjCIvarRefExpr(IV, IV->getType(), Loc);
>>>       }
>>>
>>> For now, I suggest we make the change above and have "Base" be  
>>> null for normal ivar references. As extra credit, we could factor  
>>> this out into two classes for each of the above cases.
>>
>> I think this is a great step in the right direction.  However,  
>> what is the advantage of using ObjCIvarRefExpr over DeclRefExpr?   
>> With the proposed code, you are capturing exactly the same  
>> information as DeclRefExpr would.  Just replace:
>>
>>   return new ObjCIvarRefExpr(IV, IV->getType(), Loc);
>> with:
>>   return new DeclRefExpr(IV, IV->getType(), Loc);
>>
>
> I agree (and came to the same conclusion over my workout this  
> morning).
>
> Decision: We replace ObjCIvarRefExpr with MemberExpr and DeclRefExpr.

Ok, I think that makes the most sense, thanks!

>> Also, I consider the rewriter to be somewhat orthogonal to this  
>> discussion.  This is really about getting the ASTs right.  Getting  
>> the ASTs right requires being aware of the clients, but  
>> simplifying the ASTs is goodness even if any one particular client  
>> gets slightly more complex.
>>
>
> I totally agree...the AST's need to be "right", independent of any  
> particular application.
>
> That said, while this is simplifying, it does increase the  
> complexity of determining if an expression is actually an ivar  
> reference.

You're right, but DeclRefExpr is already overloaded.  Right now it  
can refer to a BlockVarDecl, a global variable, etc.  When we add C++  
support, we don't want to have an "instance method variable expr" as  
well.  In short, I think that DeclRefExpr is really just "a direct  
reference to a decl", and the clients need to look at the decl to  
determine what that means.

-Chris