[cfe-commits] r139989 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp test/SemaObjC/class-type-conversion.m

[jahanian]

On Sep 19, 2011, at 7:50 AM, Douglas Gregor wrote:

> 
> On Sep 17, 2011, at 12:23 PM, Fariborz Jahanian wrote:
> 
>> Author: fjahanian
>> Date: Sat Sep 17 14:23:40 2011
>> New Revision: 139989
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=139989&view=rev
>> Log:
>> objc - Treat type of 'self' in class methods as root of
>> class of this method. // rdar://10109725
> 
> I'm still quite dubious about the semantics behind this change, because it seems very NeXT-runtime centric, but we can take discussion that off-line. Another comment below…
I agree, btw.

> 
>> Added:
>>   cfe/trunk/test/SemaObjC/class-type-conversion.m
>> Modified:
>>   cfe/trunk/include/clang/Sema/Sema.h
>>   cfe/trunk/lib/Sema/SemaExpr.cpp
>>   cfe/trunk/lib/Sema/SemaExprObjC.cpp
>> 
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=139989&r1=139988&r2=139989&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Sat Sep 17 14:23:40 2011
>> @@ -675,8 +675,9 @@
>> 
>>  GlobalMethodPool::iterator ReadMethodPool(Selector Sel);
>> 
>> -  /// Private Helper predicate to check for 'self'.
>> -  bool isSelfExpr(Expr *RExpr);
>> +  /// Private Helper predicate to check for 'self'. Upon success, it
>> +  /// returns method declaration where 'self' is referenced.
>> +  const ObjCMethodDecl *GetMethodIfSelfExpr(Expr *RExpr);
>> public:
>>  Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
>>       TranslationUnitKind TUKind = TU_Complete,
>> 
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=139989&r1=139988&r2=139989&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Sep 17 14:23:40 2011
>> @@ -5074,6 +5074,23 @@
>>                              OK));
>> }
>> 
>> +/// SelfInClassMethodType - convet type of 'self' in class method
>> +/// to pointer to root of method's class.
>> +static void
>> +SelfInClassMethodType(Sema &S, Expr *selfExpr, QualType &SelfType) {
>> +  if (const ObjCMethodDecl *MD = S.GetMethodIfSelfExpr(selfExpr))
>> +    if (MD->isClassMethod()) {
>> +      const ObjCInterfaceDecl *Root = 0;
>> +      if (const ObjCInterfaceDecl * IDecl = MD->getClassInterface())
>> +      do {
>> +        Root = IDecl;
>> +      } while ((IDecl = IDecl->getSuperClass()));
>> +      if (Root)
>> +        SelfType = S.Context.getObjCObjectPointerType(
>> +                                                   S.Context.getObjCInterfaceType(Root)); 
>> +    }
>> +}
> 
> I suggest returning a QualType with the requested result; it's cleaner than modifying the parameter. Also, can the function name say something about returning the root? That's not at all apparent from the interface (nor is it particularly intuitive).
> 
>> // checkPointerTypesForAssignment - This is a very tricky routine (despite
>> // being closely modeled after the C99 spec:-). The odd characteristic of this
>> // routine is it effectively iqnores the qualifiers on the top level pointee.
>> @@ -5309,6 +5326,8 @@
>>    return Compatible;
>>  }
>> 
>> +  SelfInClassMethodType(*this, RHS.get(), RHSType);
> 
> I'm not a big fan of speculatively modifying the RHSType here, because that has an impact on *all* of the checking that follows. This special conversion should be checked in isolation, down where we're checking the other Objective-C conversions.

In r140031.

- Fariborz