From b0c70192625fc2eaea3a4df4f1d9e2a639afc34b Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 9 Dec 2015 22:57:32 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@255174 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaObjCProperty.cpp | 73 ++++++++++++++----- test/SemaObjC/property-3.m | 2 +- test/SemaObjC/property-atomic-redecl.m | 57 +++++++++++++++ test/SemaObjC/property-in-class-extension-1.m | 2 +- 4 files changed, 112 insertions(+), 22 deletions(-) create mode 100644 test/SemaObjC/property-atomic-redecl.m diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index e204936b50..0beb336092 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/lib/Sema/SemaObjCProperty.cpp @@ -339,6 +339,54 @@ static bool LocPropertyAttribute( ASTContext &Context, const char *attrName, } +/// 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(OldDC)) + OldContextName = Category->getClassInterface()->getIdentifier(); + else + OldContextName = cast(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(Scope *S, // 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(PIDecl->getDeclContext())->getName(); - Diag(PIDecl->getLocation(), diag::note_property_declare); - } - } + checkAtomicPropertyMismatch(*this, PIDecl, PDecl); *isOverridingProperty = true; @@ -1326,12 +1361,10 @@ Sema::DiagnosePropertyMismatch(ObjCPropertyDecl *Property, } } - 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; diff --git a/test/SemaObjC/property-3.m b/test/SemaObjC/property-3.m index 7c0ba579ee..3f82bcc3b7 100644 --- a/test/SemaObjC/property-3.m +++ b/test/SemaObjC/property-3.m @@ -29,5 +29,5 @@ typedef signed char BOOL; @interface EKCalendar () @property (nonatomic, assign) BOOL allowReminders; -@property (nonatomic, assign) BOOL allowNonatomicProperty; // expected-warning {{'atomic' attribute on property 'allowNonatomicProperty' does not match the property inherited from EKProtocolCalendar}} +@property (nonatomic, assign) BOOL allowNonatomicProperty; // expected-warning {{'atomic' attribute on property 'allowNonatomicProperty' does not match the property inherited from 'EKProtocolCalendar'}} @end diff --git a/test/SemaObjC/property-atomic-redecl.m b/test/SemaObjC/property-atomic-redecl.m new file mode 100644 index 0000000000..8fd778048b --- /dev/null +++ b/test/SemaObjC/property-atomic-redecl.m @@ -0,0 +1,57 @@ +// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s + +@interface A +@end + +// Readonly, atomic public redeclaration of property in subclass. +@interface AtomicInheritanceSuper +@property (readonly) A *property; +@end + +@interface AtomicInheritanceSuper() +@property (nonatomic,readwrite,retain) A *property; +@end + +@interface AtomicInheritanceSub : AtomicInheritanceSuper +@property (readonly) A *property; +@end + +// Readonly, atomic public redeclaration of property in subclass. +@interface AtomicInheritanceSuper2 +@property (readonly) A *property; +@end + +@interface AtomicInheritanceSub2 : AtomicInheritanceSuper2 +@property (nonatomic, readwrite, retain) A *property; // FIXME: should be okay +@end + +@interface ReadonlyAtomic +@property (readonly, nonatomic) A *property; // expected-note{{property declared here}} +@end + +@interface ReadonlyAtomic () +@property (readwrite) A *property; // expected-warning{{'atomic' attribute on property 'property' does not match the property inherited from 'ReadonlyAtomic'}} +@end + +// Readonly, atomic public redeclaration of property in subclass. +@interface AtomicInheritanceSuper3 +@property (readonly,atomic) A *property; // expected-note{{property declared here}} +@end + +@interface AtomicInheritanceSuper3() +@property (nonatomic,readwrite,retain) A *property; // expected-warning{{'atomic' attribute on property 'property' does not match the property inherited from 'AtomicInheritanceSuper3'}} +@end + +@interface AtomicInheritanceSub3 : AtomicInheritanceSuper3 +@property (readonly) A *property; +@end + +// Readonly, atomic public redeclaration of property in subclass. +@interface AtomicInheritanceSuper4 +@property (readonly, atomic) A *property; // expected-note{{property declared here}} +@end + +@interface AtomicInheritanceSub4 : AtomicInheritanceSuper4 +@property (nonatomic, readwrite, retain) A *property; // expected-warning{{atomic' attribute on property 'property' does not match the property inherited from 'AtomicInheritanceSuper4'}} +@end + diff --git a/test/SemaObjC/property-in-class-extension-1.m b/test/SemaObjC/property-in-class-extension-1.m index b25639cf06..67b57e5fae 100644 --- a/test/SemaObjC/property-in-class-extension-1.m +++ b/test/SemaObjC/property-in-class-extension-1.m @@ -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}} -@property (atomic, readwrite) float propertyName2; // expected-warning {{'atomic' attribute on property 'propertyName2' does not match the property inherited from radar12214070}} +@property (atomic, readwrite) float propertyName2; // expected-warning {{'atomic' attribute on property 'propertyName2' does not match the property inherited from 'radar12214070'}} @end -- 2.40.0