r255174 - Objective-C properties: loosen 'atomic' checking for readonly properties.

Douglas Gregor via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 9 14:57:32 PST 2015


Author: dgregor
Date: Wed Dec  9 16:57:32 2015
New Revision: 255174

URL: http://llvm.org/viewvc/llvm-project?rev=255174&view=rev
Log:
Objective-C properties: loosen 'atomic' checking for readonly properties.

r251874 reworked the way we handle properties declared within
Objective-C class extensions, which had the effective of tightening up
property checking in a number of places. In this particular class of
cases, we end up complaining about "atomic" mismatches between an
implicitly-atomic, readonly property and a nonatomic, readwrite
property, which doesn't make sense because "atomic" is essentially
irrelevant to readonly properties.

Therefore, suppress this diagnostic when the readonly property is
implicitly atomic. Fixes rdar://problem/23803109.

Added:
    cfe/trunk/test/SemaObjC/property-atomic-redecl.m
Modified:
    cfe/trunk/lib/Sema/SemaObjCProperty.cpp
    cfe/trunk/test/SemaObjC/property-3.m
    cfe/trunk/test/SemaObjC/property-in-class-extension-1.m

Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=255174&r1=255173&r2=255174&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Wed Dec  9 16:57:32 2015
@@ -339,6 +339,54 @@ static bool LocPropertyAttribute( ASTCon
   
 }
 
+/// Check for a mismatch in the atomicity of the given properties.
+static void checkAtomicPropertyMismatch(Sema &S,
+                                        ObjCPropertyDecl *OldProperty,
+                                        ObjCPropertyDecl *NewProperty) {
+  // If the atomicity of both matches, we're done.
+  bool OldIsAtomic =
+    (OldProperty->getPropertyAttributes() & ObjCDeclSpec::DQ_PR_nonatomic) == 0;
+  bool NewIsAtomic =
+    (NewProperty->getPropertyAttributes() & ObjCDeclSpec::DQ_PR_nonatomic) == 0;
+  if (OldIsAtomic == NewIsAtomic) return;
+
+  // Determine whether the given property is readonly and implicitly
+  // atomic.
+  auto isImplicitlyReadonlyAtomic = [](ObjCPropertyDecl *Property) -> bool {
+    // Is it readonly?
+    auto Attrs = Property->getPropertyAttributes();
+    if ((Attrs & ObjCDeclSpec::DQ_PR_readonly) == 0) return false;
+
+    // Is it nonatomic?
+    if (Attrs & ObjCDeclSpec::DQ_PR_nonatomic) return false;
+
+    // Was 'atomic' specified directly?
+    if (Property->getPropertyAttributesAsWritten() & ObjCDeclSpec::DQ_PR_atomic)
+      return false;
+
+    return true;
+  };
+
+  // One of the properties is atomic; if it's a readonly property, and
+  // 'atomic' wasn't explicitly specified, we're okay.
+  if ((OldIsAtomic && isImplicitlyReadonlyAtomic(OldProperty)) ||
+      (NewIsAtomic && isImplicitlyReadonlyAtomic(NewProperty)))
+    return;
+
+  // Diagnose the conflict.
+  const IdentifierInfo *OldContextName;
+  auto *OldDC = OldProperty->getDeclContext();
+  if (auto Category = dyn_cast<ObjCCategoryDecl>(OldDC))
+    OldContextName = Category->getClassInterface()->getIdentifier();
+  else
+    OldContextName = cast<ObjCContainerDecl>(OldDC)->getIdentifier();
+
+  S.Diag(NewProperty->getLocation(), diag::warn_property_attribute)
+    << NewProperty->getDeclName() << "atomic"
+    << OldContextName;
+  S.Diag(OldProperty->getLocation(), diag::note_property_declare);
+}
+
 ObjCPropertyDecl *
 Sema::HandlePropertyInClassExtension(Scope *S,
                                      SourceLocation AtLoc,
@@ -464,20 +512,7 @@ Sema::HandlePropertyInClassExtension(Sco
   
   // Check that atomicity of property in class extension matches the previous
   // declaration.
-  unsigned PDeclAtomicity =
-    PDecl->getPropertyAttributes() & (ObjCDeclSpec::DQ_PR_atomic | ObjCDeclSpec::DQ_PR_nonatomic);
-  unsigned PIDeclAtomicity =
-    PIDecl->getPropertyAttributes() & (ObjCDeclSpec::DQ_PR_atomic | ObjCDeclSpec::DQ_PR_nonatomic);
-  if (PDeclAtomicity != PIDeclAtomicity) {
-    bool PDeclAtomic = (!PDeclAtomicity || PDeclAtomicity & ObjCDeclSpec::DQ_PR_atomic);
-    bool PIDeclAtomic = (!PIDeclAtomicity || PIDeclAtomicity & ObjCDeclSpec::DQ_PR_atomic);
-    if (PDeclAtomic != PIDeclAtomic) {
-      Diag(PDecl->getLocation(), diag::warn_property_attribute)
-        << PDecl->getDeclName() << "atomic"
-        << cast<ObjCContainerDecl>(PIDecl->getDeclContext())->getName();
-      Diag(PIDecl->getLocation(), diag::note_property_declare);
-    }
-  }
+  checkAtomicPropertyMismatch(*this, PIDecl, PDecl);
 
   *isOverridingProperty = true;
 
@@ -1326,12 +1361,10 @@ Sema::DiagnosePropertyMismatch(ObjCPrope
     }
   }
 
-  if ((CAttr & ObjCPropertyDecl::OBJC_PR_nonatomic)
-      != (SAttr & ObjCPropertyDecl::OBJC_PR_nonatomic)) {
-    Diag(Property->getLocation(), diag::warn_property_attribute)
-      << Property->getDeclName() << "atomic" << inheritedName;
-    Diag(SuperProperty->getLocation(), diag::note_property_declare);
-  }
+  // Check for nonatomic; note that nonatomic is effectively
+  // meaningless for readonly properties, so don't diagnose if the
+  // atomic property is 'readonly'.
+  checkAtomicPropertyMismatch(*this, SuperProperty, Property);
   if (Property->getSetterName() != SuperProperty->getSetterName()) {
     Diag(Property->getLocation(), diag::warn_property_attribute)
       << Property->getDeclName() << "setter" << inheritedName;

Modified: cfe/trunk/test/SemaObjC/property-3.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-3.m?rev=255174&r1=255173&r2=255174&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/property-3.m (original)
+++ cfe/trunk/test/SemaObjC/property-3.m Wed Dec  9 16:57:32 2015
@@ -29,5 +29,5 @@ typedef signed char BOOL;
 
 @interface EKCalendar ()  <EKProtocolMutableCalendar>
 @property (nonatomic, assign) BOOL allowReminders;
- at property (nonatomic, assign) BOOL allowNonatomicProperty; // expected-warning {{'atomic' attribute on property 'allowNonatomicProperty' does not match the property inherited from EKProtocolCalendar}}
+ at property (nonatomic, assign) BOOL allowNonatomicProperty; // expected-warning {{'atomic' attribute on property 'allowNonatomicProperty' does not match the property inherited from 'EKProtocolCalendar'}}
 @end

Added: cfe/trunk/test/SemaObjC/property-atomic-redecl.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-atomic-redecl.m?rev=255174&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/property-atomic-redecl.m (added)
+++ cfe/trunk/test/SemaObjC/property-atomic-redecl.m Wed Dec  9 16:57:32 2015
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s
+
+ at interface A
+ at end
+
+// Readonly, atomic public redeclaration of property in subclass.
+ at interface AtomicInheritanceSuper
+ at property (readonly) A *property;
+ at end
+
+ at interface AtomicInheritanceSuper()
+ at property (nonatomic,readwrite,retain) A *property;
+ at end
+
+ at interface AtomicInheritanceSub : AtomicInheritanceSuper
+ at property (readonly) A *property;
+ at end
+
+// Readonly, atomic public redeclaration of property in subclass.
+ at interface AtomicInheritanceSuper2
+ at property (readonly) A *property;
+ at end
+
+ at interface AtomicInheritanceSub2 : AtomicInheritanceSuper2
+ at property (nonatomic, readwrite, retain) A *property; // FIXME: should be okay
+ at end
+
+ at interface ReadonlyAtomic
+ at property (readonly, nonatomic) A *property; // expected-note{{property declared here}}
+ at end
+
+ at interface ReadonlyAtomic ()
+ at property (readwrite) A *property; // expected-warning{{'atomic' attribute on property 'property' does not match the property inherited from 'ReadonlyAtomic'}}
+ at end
+
+// Readonly, atomic public redeclaration of property in subclass.
+ at interface AtomicInheritanceSuper3
+ at property (readonly,atomic) A *property; // expected-note{{property declared here}}
+ at end
+
+ at interface AtomicInheritanceSuper3()
+ at property (nonatomic,readwrite,retain) A *property; // expected-warning{{'atomic' attribute on property 'property' does not match the property inherited from 'AtomicInheritanceSuper3'}}
+ at end
+
+ at interface AtomicInheritanceSub3 : AtomicInheritanceSuper3
+ at property (readonly) A *property;
+ at end
+
+// Readonly, atomic public redeclaration of property in subclass.
+ at interface AtomicInheritanceSuper4
+ at property (readonly, atomic) A *property; // expected-note{{property declared here}}
+ at end
+
+ at interface AtomicInheritanceSub4 : AtomicInheritanceSuper4
+ at property (nonatomic, readwrite, retain) A *property; // expected-warning{{atomic' attribute on property 'property' does not match the property inherited from 'AtomicInheritanceSuper4'}}
+ at end
+

Modified: cfe/trunk/test/SemaObjC/property-in-class-extension-1.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-in-class-extension-1.m?rev=255174&r1=255173&r2=255174&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/property-in-class-extension-1.m (original)
+++ cfe/trunk/test/SemaObjC/property-in-class-extension-1.m Wed Dec  9 16:57:32 2015
@@ -58,6 +58,6 @@
 @property (atomic, nonatomic, readonly, readwrite) float propertyName; // expected-error {{property attributes 'readonly' and 'readwrite' are mutually exclusive}} \
 		// expected-error {{property attributes 'atomic' and 'nonatomic' are mutually exclusive}}
 
- at property (atomic, readwrite) float propertyName2; // expected-warning {{'atomic' attribute on property 'propertyName2' does not match the property inherited from radar12214070}}
+ at property (atomic, readwrite) float propertyName2; // expected-warning {{'atomic' attribute on property 'propertyName2' does not match the property inherited from 'radar12214070'}}
 @end
 




More information about the cfe-commits mailing list