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; -@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 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 + +@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 + 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}} -@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits