[cfe-commits] r139615 - in /cfe/trunk: include/clang/AST/DeclObjC.h include/clang/Basic/DiagnosticSemaKinds.td lib/CodeGen/CGObjC.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaObjCProperty.cpp test/PCH/chain-remap-types.m

John McCall rjmccall at apple.com
Tue Sep 13 11:31:23 PDT 2011


Author: rjmccall
Date: Tue Sep 13 13:31:23 2011
New Revision: 139615

URL: http://llvm.org/viewvc/llvm-project?rev=139615&view=rev
Log:
Refactoring, mostly to give ObjCPropertyDecls stronger invariants for
their semantic attributes and then to take advantage of that.


Modified:
    cfe/trunk/include/clang/AST/DeclObjC.h
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/CodeGen/CGObjC.cpp
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/lib/Sema/SemaObjCProperty.cpp
    cfe/trunk/test/PCH/chain-remap-types.m

Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Tue Sep 13 13:31:23 2011
@@ -1495,6 +1495,17 @@
     return (PropertyAttributes & OBJC_PR_readonly);
   }
 
+  /// isAtomic - Return true if the property is atomic.
+  bool isAtomic() const {
+    return (PropertyAttributes & OBJC_PR_atomic);
+  }
+
+  /// isRetaining - Return true if the property retains its value.
+  bool isRetaining() const {
+    return (PropertyAttributes &
+            (OBJC_PR_retain | OBJC_PR_strong | OBJC_PR_copy));
+  }
+
   /// getSetterKind - Return the method used for doing assignment in
   /// the property setter. This is only valid if the property has been
   /// defined to have a setter.

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 13 13:31:23 2011
@@ -2886,7 +2886,7 @@
   "existing ivar %1 for property %0 with %select{unsafe_unretained| assign}2 "
   "attribute must be __unsafe_unretained">;
 def err_arc_inconsistent_property_ownership : Error<
-  "%select{strong|weak|unsafe_unretained}1 property %0 may not also be "
+  "%select{|unsafe_unretained|strong|weak}1 property %0 may not also be "
   "declared %select{|__unsafe_unretained|__strong|__weak|__autoreleasing}2">;
 def err_arc_atomic_ownership : Error<
   "cannot perform atomic operation on a pointer to type %0: type has "

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Tue Sep 13 13:31:23 2011
@@ -443,10 +443,10 @@
 PropertyImplStrategy::PropertyImplStrategy(CodeGenModule &CGM,
                                      const ObjCPropertyImplDecl *propImpl) {
   const ObjCPropertyDecl *prop = propImpl->getPropertyDecl();
-  ObjCPropertyDecl::PropertyAttributeKind attrs = prop->getPropertyAttributes();
+  ObjCPropertyDecl::SetterKind setterKind = prop->getSetterKind();
 
-  IsCopy = (attrs & ObjCPropertyDecl::OBJC_PR_copy);
-  IsAtomic = !(attrs & ObjCPropertyDecl::OBJC_PR_nonatomic);
+  IsCopy = (setterKind == ObjCPropertyDecl::Copy);
+  IsAtomic = prop->isAtomic();
   HasStrong = false; // doesn't matter here.
 
   // Evaluate the ivar's size and alignment.
@@ -456,14 +456,14 @@
     = CGM.getContext().getTypeInfoInChars(ivarType);
 
   // If we have a copy property, we always have to use getProperty/setProperty.
+  // TODO: we could actually use setProperty and an expression for non-atomics.
   if (IsCopy) {
     Kind = GetSetProperty;
     return;
   }
 
-  // Handle retain/strong.
-  if (attrs & (ObjCPropertyDecl::OBJC_PR_retain
-               | ObjCPropertyDecl::OBJC_PR_strong)) {
+  // Handle retain.
+  if (setterKind == ObjCPropertyDecl::Retain) {
     // In GC-only, there's nothing special that needs to be done.
     if (CGM.getLangOptions().getGC() == LangOptions::GCOnly) {
       // fallthrough
@@ -663,9 +663,8 @@
     args.add(RValue::get(self), getContext().getObjCIdType());
     args.add(RValue::get(cmd), getContext().getObjCSelType());
     args.add(RValue::get(ivarOffset), getContext().getPointerDiffType());
-
-    assert(strategy.isAtomic());
-    args.add(RValue::get(Builder.getTrue()), getContext().BoolTy);
+    args.add(RValue::get(Builder.getInt1(strategy.isAtomic())),
+             getContext().BoolTy);
 
     // FIXME: We shouldn't need to get the function info here, the
     // runtime already should have computed it to build the function.

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Sep 13 13:31:23 2011
@@ -3862,10 +3862,7 @@
         const ObjCPropertyRefExpr *pre = cast->getSubExpr()->getObjCProperty();
         if (pre->isImplicitProperty()) return false;
         ObjCPropertyDecl *property = pre->getExplicitProperty();
-        if (!(property->getPropertyAttributes() &
-              (ObjCPropertyDecl::OBJC_PR_retain |
-               ObjCPropertyDecl::OBJC_PR_copy |
-               ObjCPropertyDecl::OBJC_PR_strong)) &&
+        if (!property->isRetaining() &&
             !(property->getPropertyIvarDecl() &&
               property->getPropertyIvarDecl()->getType()
                 .getObjCLifetime() == Qualifiers::OCL_Strong))

Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Tue Sep 13 13:31:23 2011
@@ -24,6 +24,37 @@
 // Grammar actions.
 //===----------------------------------------------------------------------===//
 
+/// getImpliedARCOwnership - Given a set of property attributes and a
+/// type, infer an expected lifetime.  The type's ownership qualification
+/// is not considered.
+///
+/// Returns OCL_None if the attributes as stated do not imply an ownership.
+/// Never returns OCL_Autoreleasing.
+static Qualifiers::ObjCLifetime getImpliedARCOwnership(
+                               ObjCPropertyDecl::PropertyAttributeKind attrs,
+                                                QualType type) {
+  // retain, strong, copy, weak, and unsafe_unretained are only legal
+  // on properties of retainable pointer type.
+  if (attrs & (ObjCPropertyDecl::OBJC_PR_retain |
+               ObjCPropertyDecl::OBJC_PR_strong |
+               ObjCPropertyDecl::OBJC_PR_copy)) {
+    return Qualifiers::OCL_Strong;
+  } else if (attrs & ObjCPropertyDecl::OBJC_PR_weak) {
+    return Qualifiers::OCL_Weak;
+  } else if (attrs & ObjCPropertyDecl::OBJC_PR_unsafe_unretained) {
+    return Qualifiers::OCL_ExplicitNone;
+  }
+
+  // assign can appear on other types, so we have to check the
+  // property type.
+  if (attrs & ObjCPropertyDecl::OBJC_PR_assign &&
+      type->isObjCRetainableType()) {
+    return Qualifiers::OCL_ExplicitNone;
+  }
+
+  return Qualifiers::OCL_None;
+}
+
 /// Check the internal consistency of a property declaration.
 static void checkARCPropertyDecl(Sema &S, ObjCPropertyDecl *property) {
   if (property->isInvalidDecl()) return;
@@ -36,26 +67,23 @@
   // Nothing to do if we don't have a lifetime.
   if (propertyLifetime == Qualifiers::OCL_None) return;
 
-  Qualifiers::ObjCLifetime expectedLifetime;
-  unsigned selector;
-
-  // Strong properties should have either strong or no lifetime.
-  if (propertyKind & (ObjCPropertyDecl::OBJC_PR_retain |
-                      ObjCPropertyDecl::OBJC_PR_strong |
-                      ObjCPropertyDecl::OBJC_PR_copy)) {
-    expectedLifetime = Qualifiers::OCL_Strong;
-    selector = 0;
-  } else if (propertyKind & ObjCPropertyDecl::OBJC_PR_weak) {
-    expectedLifetime = Qualifiers::OCL_Weak;
-    selector = 1;
-  } else if (propertyKind & (ObjCPropertyDecl::OBJC_PR_assign |
-                             ObjCPropertyDecl::OBJC_PR_unsafe_unretained) &&
-             property->getType()->isObjCRetainableType()) {
-    expectedLifetime = Qualifiers::OCL_ExplicitNone;
-    selector = 2;
-  } else {
+  Qualifiers::ObjCLifetime expectedLifetime
+    = getImpliedARCOwnership(propertyKind, property->getType());
+  if (!expectedLifetime) {
     // We have a lifetime qualifier but no dominating property
-    // attribute.  That's okay.
+    // attribute.  That's okay, but restore reasonable invariants by
+    // setting the property attribute according to the lifetime
+    // qualifier.
+    ObjCPropertyDecl::PropertyAttributeKind attr;
+    if (propertyLifetime == Qualifiers::OCL_Strong) {
+      attr = ObjCPropertyDecl::OBJC_PR_strong;
+    } else if (propertyLifetime == Qualifiers::OCL_Weak) {
+      attr = ObjCPropertyDecl::OBJC_PR_weak;
+    } else {
+      assert(propertyLifetime == Qualifiers::OCL_ExplicitNone);
+      attr = ObjCPropertyDecl::OBJC_PR_unsafe_unretained;
+    }
+    property->setPropertyAttributes(attr);
     return;
   }
 
@@ -65,7 +93,7 @@
   S.Diag(property->getLocation(),
          diag::err_arc_inconsistent_property_ownership)
     << property->getDeclName()
-    << selector
+    << expectedLifetime
     << propertyLifetime;
 }
 
@@ -393,9 +421,10 @@
   if (isAssign)
     PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_assign);
 
+  // In the semantic attributes, one of nonatomic or atomic is always set.
   if (Attributes & ObjCDeclSpec::DQ_PR_nonatomic)
     PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_nonatomic);
-  else if (Attributes & ObjCDeclSpec::DQ_PR_atomic)
+  else
     PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_atomic);
 
   // 'unsafe_unretained' is alias for 'assign'.
@@ -417,84 +446,49 @@
                                  ObjCIvarDecl *ivar) {
   if (property->isInvalidDecl() || ivar->isInvalidDecl()) return;
 
-  QualType propertyType = property->getType();
-  Qualifiers::ObjCLifetime propertyLifetime = propertyType.getObjCLifetime();
-  ObjCPropertyDecl::PropertyAttributeKind propertyKind
-    = property->getPropertyAttributes();
-
   QualType ivarType = ivar->getType();
   Qualifiers::ObjCLifetime ivarLifetime = ivarType.getObjCLifetime();
-          
-  // Case 1: strong properties.
-  if (propertyLifetime == Qualifiers::OCL_Strong ||
-      (propertyKind & (ObjCPropertyDecl::OBJC_PR_retain |
-                       ObjCPropertyDecl::OBJC_PR_strong |
-                       ObjCPropertyDecl::OBJC_PR_copy))) {
-    switch (ivarLifetime) {
-    case Qualifiers::OCL_Strong:
-      // Okay.
-      return;
-
-    case Qualifiers::OCL_None:
-    case Qualifiers::OCL_Autoreleasing:
-      // These aren't valid lifetimes for object ivars;  don't diagnose twice.
-      return;
-
-    case Qualifiers::OCL_ExplicitNone:
-    case Qualifiers::OCL_Weak:
-      S.Diag(propertyImplLoc, diag::err_arc_strong_property_ownership)
-        << property->getDeclName()
-        << ivar->getDeclName()
-        << ivarLifetime;
-      break;
-    }
-
-  // Case 2: weak properties.
-  } else if (propertyLifetime == Qualifiers::OCL_Weak ||
-             (propertyKind & ObjCPropertyDecl::OBJC_PR_weak)) {
-    switch (ivarLifetime) {
-    case Qualifiers::OCL_Weak:
-      // Okay.
-      return;
-
-    case Qualifiers::OCL_None:
-    case Qualifiers::OCL_Autoreleasing:
-      // These aren't valid lifetimes for object ivars;  don't diagnose twice.
-      return;
-
-    case Qualifiers::OCL_ExplicitNone:
-    case Qualifiers::OCL_Strong:
-      S.Diag(propertyImplLoc, diag::error_weak_property)
-        << property->getDeclName()
-        << ivar->getDeclName();
-      break;
-    }
 
-  // Case 3: assign properties.
-  } else if ((propertyKind & ObjCPropertyDecl::OBJC_PR_assign) &&
-             propertyType->isObjCRetainableType()) {
-    switch (ivarLifetime) {
-    case Qualifiers::OCL_ExplicitNone:
-      // Okay.
-      return;
-
-    case Qualifiers::OCL_None:
-    case Qualifiers::OCL_Autoreleasing:
-      // These aren't valid lifetimes for object ivars;  don't diagnose twice.
-      return;
-
-    case Qualifiers::OCL_Weak:
-    case Qualifiers::OCL_Strong:
-      S.Diag(propertyImplLoc, diag::err_arc_assign_property_ownership)
-        << property->getDeclName()
-        << ivar->getDeclName()
-        << ((property->getPropertyAttributesAsWritten() 
-            & ObjCPropertyDecl::OBJC_PR_assign) != 0);
-      break;
-    }
+  // The lifetime implied by the property's attributes.
+  Qualifiers::ObjCLifetime propertyLifetime =
+    getImpliedARCOwnership(property->getPropertyAttributes(),
+                           property->getType());
+
+  // We're fine if they match.
+  if (propertyLifetime == ivarLifetime) return;
+
+  // These aren't valid lifetimes for object ivars;  don't diagnose twice.
+  if (ivarLifetime == Qualifiers::OCL_None ||
+      ivarLifetime == Qualifiers::OCL_Autoreleasing)
+    return;
 
-  // Any other property should be ignored.
-  } else {
+  switch (propertyLifetime) {
+  case Qualifiers::OCL_Strong:
+    S.Diag(propertyImplLoc, diag::err_arc_strong_property_ownership)
+      << property->getDeclName()
+      << ivar->getDeclName()
+      << ivarLifetime;
+    break;
+
+  case Qualifiers::OCL_Weak:
+    S.Diag(propertyImplLoc, diag::error_weak_property)
+      << property->getDeclName()
+      << ivar->getDeclName();
+    break;
+
+  case Qualifiers::OCL_ExplicitNone:
+    S.Diag(propertyImplLoc, diag::err_arc_assign_property_ownership)
+      << property->getDeclName()
+      << ivar->getDeclName()
+      << ((property->getPropertyAttributesAsWritten() 
+           & ObjCPropertyDecl::OBJC_PR_assign) != 0);
+    break;
+
+  case Qualifiers::OCL_Autoreleasing:
+    llvm_unreachable("properties cannot be autoreleasing");
+
+  case Qualifiers::OCL_None:
+    // Any other property should be ignored.
     return;
   }
 
@@ -593,64 +587,55 @@
       PropertyIvar = PropertyId;
     ObjCPropertyDecl::PropertyAttributeKind kind 
       = property->getPropertyAttributes();
-    QualType PropType = Context.getCanonicalType(property->getType());
-    
-    if ((kind & ObjCPropertyDecl::OBJC_PR_weak) &&
-        !getLangOptions().ObjCAutoRefCount &&
+    QualType PropType = property->getType();
+
+    QualType PropertyIvarType = PropType.getNonReferenceType();
+
+    // Add GC __weak to the ivar type if the property is weak.
+    if ((kind & ObjCPropertyDecl::OBJC_PR_weak) && 
         getLangOptions().getGC() != LangOptions::NonGC) {
-      if (PropType.isObjCGCStrong()) {
-          Diag(PropertyLoc,
-               diag::err_gc_weak_property_strong_type);
-          Diag(property->getLocation(), diag::note_property_declare);    
+      assert(!getLangOptions().ObjCAutoRefCount);
+      if (PropertyIvarType.isObjCGCStrong()) {
+        Diag(PropertyLoc, diag::err_gc_weak_property_strong_type);
+        Diag(property->getLocation(), diag::note_property_declare);
+      } else {
+        PropertyIvarType =
+          Context.getObjCGCQualType(PropertyIvarType, Qualifiers::Weak);
       }
-      else
-        PropType = Context.getObjCGCQualType(PropType, Qualifiers::Weak);
     }
-    QualType PropertyIvarType = PropType;
-    if (PropType->isReferenceType())
-      PropertyIvarType = cast<ReferenceType>(PropType)->getPointeeType();
+
     // Check that this is a previously declared 'ivar' in 'IDecl' interface
     ObjCInterfaceDecl *ClassDeclared;
     Ivar = IDecl->lookupInstanceVariable(PropertyIvar, ClassDeclared);
     if (!Ivar) {
-      // In ARC, give the ivar a lifetime qualifier based on its
+      // In ARC, give the ivar a lifetime qualifier based on the
       // property attributes.
       if (getLangOptions().ObjCAutoRefCount &&
-          !PropertyIvarType.getObjCLifetime()) {
+          !PropertyIvarType.getObjCLifetime() &&
+          PropertyIvarType->isObjCRetainableType()) {
 
+        // It's an error if we have to do this and the user didn't
+        // explicitly write an ownership attribute on the property.
         if (!property->hasWrittenStorageAttribute() &&
-            property->getType()->isObjCRetainableType() &&
-            !(kind & ObjCPropertyDecl::OBJC_PR_strong) ) {
+            !(kind & ObjCPropertyDecl::OBJC_PR_strong)) {
           Diag(PropertyLoc,
                diag::err_arc_objc_property_default_assign_on_object);
           Diag(property->getLocation(), diag::note_property_declare);
-        }
+        } else {
+          Qualifiers::ObjCLifetime lifetime =
+            getImpliedARCOwnership(kind, PropertyIvarType);
+          assert(lifetime && "no lifetime for property?");
 
-        // retain/copy have retaining lifetime.
-        if (kind & (ObjCPropertyDecl::OBJC_PR_retain |
-                    ObjCPropertyDecl::OBJC_PR_strong |
-                    ObjCPropertyDecl::OBJC_PR_copy)) {
-          Qualifiers qs;
-          qs.addObjCLifetime(Qualifiers::OCL_Strong);
-          PropertyIvarType = Context.getQualifiedType(PropertyIvarType, qs);
-        }
-        else if (kind & ObjCPropertyDecl::OBJC_PR_weak) {
-          if (!getLangOptions().ObjCRuntimeHasWeak) {
+          if (lifetime == Qualifiers::OCL_Weak &&
+              !getLangOptions().ObjCRuntimeHasWeak) {
             Diag(PropertyLoc, diag::err_arc_weak_no_runtime);
             Diag(property->getLocation(), diag::note_property_declare);
           }
+
           Qualifiers qs;
-          qs.addObjCLifetime(Qualifiers::OCL_Weak);
+          qs.addObjCLifetime(lifetime);
           PropertyIvarType = Context.getQualifiedType(PropertyIvarType, qs);   
         }
-        else if (kind & ObjCPropertyDecl::OBJC_PR_assign &&
-                 PropertyIvarType->isObjCRetainableType()) {
-            // assume that an 'assign' property synthesizes __unsafe_unretained
-            // ivar
-            Qualifiers qs;
-            qs.addObjCLifetime(Qualifiers::OCL_ExplicitNone);
-            PropertyIvarType = Context.getQualifiedType(PropertyIvarType, qs);  
-        }
       }
 
       if (kind & ObjCPropertyDecl::OBJC_PR_weak &&
@@ -685,7 +670,7 @@
     QualType IvarType = Context.getCanonicalType(Ivar->getType());
 
     // Check that type of property and its ivar are type compatible.
-    if (PropertyIvarType != IvarType) {
+    if (Context.getCanonicalType(PropertyIvarType) != IvarType) {
       bool compat = false;
       if (isa<ObjCObjectPointerType>(PropertyIvarType) 
           && isa<ObjCObjectPointerType>(IvarType))
@@ -1375,10 +1360,10 @@
     bool LookedUpGetterSetter = false;
 
     unsigned Attributes = Property->getPropertyAttributes();
-    unsigned AttributesAsWrittern = Property->getPropertyAttributesAsWritten();
+    unsigned AttributesAsWritten = Property->getPropertyAttributesAsWritten();
 
-    if (!(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_atomic) &&
-        !(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_nonatomic)) {
+    if (!(AttributesAsWritten & ObjCPropertyDecl::OBJC_PR_atomic) &&
+        !(AttributesAsWritten & ObjCPropertyDecl::OBJC_PR_nonatomic)) {
       GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName());
       SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName());
       LookedUpGetterSetter = true;

Modified: cfe/trunk/test/PCH/chain-remap-types.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/chain-remap-types.m?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/test/PCH/chain-remap-types.m (original)
+++ cfe/trunk/test/PCH/chain-remap-types.m Tue Sep 13 13:31:23 2011
@@ -5,7 +5,7 @@
 
 // CHECK: @class X;
 // CHECK: struct Y 
-// CHECK: @property ( assign,readwrite ) X * prop
+// CHECK: @property ( assign,readwrite,atomic ) X * prop
 // CHECK: void h(X *);
 // CHECK: @interface X(Blah)
 // CHECK: void g(X *);





More information about the cfe-commits mailing list