[cfe-commits] r134522 - in /cfe/trunk: include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/AttributeList.h lib/Sema/AttributeList.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaType.cpp test/SemaObjC/arc-unavailable-for-weakref.m

[jahanian]

On Jul 6, 2011, at 12:46 PM, Douglas Gregor wrote:

> 
> On Jul 6, 2011, at 12:24 PM, Fariborz Jahanian wrote:
> 
>> Author: fjahanian
>> Date: Wed Jul  6 14:24:05 2011
>> New Revision: 134522
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=134522&view=rev
>> Log:
>> objc-arc: Support objc_arc_weak_unavailable on those
>> classes which are incompatible with weak references.
>> // rdar://9693477
> 
> Excellent. A few minor comments below.
> 
>> Modified: cfe/trunk/lib/Sema/SemaType.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=134522&r1=134521&r2=134522&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaType.cpp Wed Jul  6 14:24:05 2011
>> @@ -3207,7 +3207,30 @@
>>    attr.setInvalid();
>>    return true;
>>  }
>> -
>> +    
>> +  // Forbid __weak for class objects marked as 
>> +  // objc_arc_weak_reference_unavailable
>> +  if (lifetime == Qualifiers::OCL_Weak) {
>> +    QualType T = type;
>> +    if (T->isReferenceType()) {
>> +      T = T->getAs<ReferenceType>()->getPointeeType();
>> +    }
> 
> if (const ReferenceType *Ref = T->getAs<ReferenceType>())
>  T = Ref->getPointeeType();

dumb of me :).
> 
> ?
> 
> Since you used that pattern below.
> 
>> +    while (const PointerType *ptr = T->getAs<PointerType>())
>> +      T = ptr->getPointeeType();
>> +    if (const ObjCObjectPointerType *ObjT = T->getAs<ObjCObjectPointerType>()) {
>> +      ObjCInterfaceDecl *Class = ObjT->getInterfaceDecl();
>> +      while (Class) {
>> +        if (Class->hasAttr<ArcWeakrefUnavailableAttr>()) {
>> +          S.Diag(attr.getLoc(), diag::err_arc_unsupported_weak_class);
>> +          S.Diag(ObjT->getInterfaceDecl()->getLocation(), 
>> +                 diag::note_class_declared);
>> +          break;
>> +        }
>> +        Class = Class->getSuperClass();
>> +      }
>> +    }
>> +  }
>> +  
>>  return true;
>> }
> 
> Should this hasAttr<ArcWeakrefUnavailableAttr>() loop over superclasses be moved in an isArcWeakrefUnavailable() method on the ObjCInterfaceDecl? I could imagine it being reused by, e.g., the migrator.

Sure.

- fj

> 
> 	- Doug