From: Fariborz Jahanian Date: Mon, 7 Oct 2013 17:20:02 +0000 (+0000) Subject: ObjectiveC: Warn when 'readonly' property has explicit X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dc1031bf257457bd58cc19426a21e0fa1fa95789;p=clang ObjectiveC: Warn when 'readonly' property has explicit ownership attribute (such as 'copy', 'assign' etc.) // rdar://15131088 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@192115 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index 5323ad47ca..8658b687c1 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/lib/Sema/SemaObjCProperty.cpp @@ -321,6 +321,21 @@ static unsigned getOwnershipRule(unsigned attr) { ObjCPropertyDecl::OBJC_PR_unsafe_unretained); } +static const char *NameOfOwnershipAttribute(unsigned attr) { + if (attr & ObjCPropertyDecl::OBJC_PR_assign) + return "assign"; + if (attr & ObjCPropertyDecl::OBJC_PR_retain ) + return "retain"; + if (attr & ObjCPropertyDecl::OBJC_PR_copy) + return "copy"; + if (attr & ObjCPropertyDecl::OBJC_PR_weak) + return "weak"; + if (attr & ObjCPropertyDecl::OBJC_PR_strong) + return "strong"; + assert(attr & ObjCPropertyDecl::OBJC_PR_unsafe_unretained); + return "unsafe_unretained"; +} + ObjCPropertyDecl * Sema::HandlePropertyInClassExtension(Scope *S, SourceLocation AtLoc, @@ -434,6 +449,8 @@ Sema::HandlePropertyInClassExtension(Scope *S, // with continuation class's readwrite property attribute! unsigned PIkind = PIDecl->getPropertyAttributesAsWritten(); if (isReadWrite && (PIkind & ObjCPropertyDecl::OBJC_PR_readonly)) { + PIkind &= ~ObjCPropertyDecl::OBJC_PR_readonly; + PIkind |= ObjCPropertyDecl::OBJC_PR_readwrite; PIkind |= deduceWeakPropertyFromType(*this, PIDecl->getType()); unsigned ClassExtensionMemoryModel = getOwnershipRule(Attributes); unsigned PrimaryClassMemoryModel = getOwnershipRule(PIkind); @@ -775,75 +792,6 @@ DiagnosePropertyMismatchDeclInProtocols(Sema &S, SourceLocation AtLoc, S.Diag(AtLoc, diag::note_property_synthesize); } -/// DiagnoseClassAndClassExtPropertyMismatch - diagnose inconsistant property -/// attribute declared in primary class and attributes overridden in any of its -/// class extensions. -static void -DiagnoseClassAndClassExtPropertyMismatch(Sema &S, ObjCInterfaceDecl *ClassDecl, - ObjCPropertyDecl *property) { - unsigned Attributes = property->getPropertyAttributesAsWritten(); - bool warn = (Attributes & ObjCDeclSpec::DQ_PR_readonly); - for (ObjCInterfaceDecl::known_extensions_iterator - Ext = ClassDecl->known_extensions_begin(), - ExtEnd = ClassDecl->known_extensions_end(); - Ext != ExtEnd; ++Ext) { - ObjCPropertyDecl *ClassExtProperty = 0; - DeclContext::lookup_result R = Ext->lookup(property->getDeclName()); - for (unsigned I = 0, N = R.size(); I != N; ++I) { - ClassExtProperty = dyn_cast(R[0]); - if (ClassExtProperty) - break; - } - - if (ClassExtProperty) { - warn = false; - unsigned classExtPropertyAttr = - ClassExtProperty->getPropertyAttributesAsWritten(); - // We are issuing the warning that we postponed because class extensions - // can override readonly->readwrite and 'setter' attributes originally - // placed on class's property declaration now make sense in the overridden - // property. - if (Attributes & ObjCDeclSpec::DQ_PR_readonly) { - if (!classExtPropertyAttr || - (classExtPropertyAttr & - (ObjCDeclSpec::DQ_PR_readwrite| - ObjCDeclSpec::DQ_PR_assign | - ObjCDeclSpec::DQ_PR_unsafe_unretained | - ObjCDeclSpec::DQ_PR_copy | - ObjCDeclSpec::DQ_PR_retain | - ObjCDeclSpec::DQ_PR_strong))) - continue; - warn = true; - break; - } - } - } - if (warn) { - unsigned setterAttrs = (ObjCDeclSpec::DQ_PR_assign | - ObjCDeclSpec::DQ_PR_unsafe_unretained | - ObjCDeclSpec::DQ_PR_copy | - ObjCDeclSpec::DQ_PR_retain | - ObjCDeclSpec::DQ_PR_strong); - if (Attributes & setterAttrs) { - const char * which = - (Attributes & ObjCDeclSpec::DQ_PR_assign) ? - "assign" : - (Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) ? - "unsafe_unretained" : - (Attributes & ObjCDeclSpec::DQ_PR_copy) ? - "copy" : - (Attributes & ObjCDeclSpec::DQ_PR_retain) ? - "retain" : "strong"; - - S.Diag(property->getLocation(), - diag::warn_objc_property_attr_mutually_exclusive) - << "readonly" << which; - } - } - - -} - /// ActOnPropertyImplDecl - This routine performs semantic checks and /// builds the AST node for a property implementation declaration; declared /// as \@synthesize or \@dynamic. @@ -942,8 +890,6 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, } if (Synthesize && isa(property->getDeclContext())) DiagnosePropertyMismatchDeclInProtocols(*this, AtLoc, IDecl, property); - - DiagnoseClassAndClassExtPropertyMismatch(*this, IDecl, property); } else if ((CatImplClass = dyn_cast(ClassImpDecl))) { if (Synthesize) { @@ -2055,55 +2001,30 @@ void Sema::CheckObjCPropertyAttributes(Decl *PDecl, // FIXME: Improve the reported location. if (!PDecl || PDecl->isInvalidDecl()) return; - + + if ((Attributes & ObjCDeclSpec::DQ_PR_readonly) && + (Attributes & ObjCDeclSpec::DQ_PR_readwrite)) + Diag(Loc, diag::err_objc_property_attr_mutually_exclusive) + << "readonly" << "readwrite"; + ObjCPropertyDecl *PropertyDecl = cast(PDecl); - QualType PropertyTy = PropertyDecl->getType(); - - if (getLangOpts().ObjCAutoRefCount && - (Attributes & ObjCDeclSpec::DQ_PR_readonly) && - PropertyTy->isObjCRetainableType()) { - // 'readonly' property with no obvious lifetime. - // its life time will be determined by its backing ivar. - unsigned rel = (ObjCDeclSpec::DQ_PR_unsafe_unretained | - ObjCDeclSpec::DQ_PR_copy | - ObjCDeclSpec::DQ_PR_retain | - ObjCDeclSpec::DQ_PR_strong | - ObjCDeclSpec::DQ_PR_weak | - ObjCDeclSpec::DQ_PR_assign); - if ((Attributes & rel) == 0) + QualType PropertyTy = PropertyDecl->getType(); + unsigned PropertyOwnership = getOwnershipRule(Attributes); + + if (Attributes & ObjCDeclSpec::DQ_PR_readonly) { + if (getLangOpts().ObjCAutoRefCount && + PropertyTy->isObjCRetainableType() && + !PropertyOwnership) { + // 'readonly' property with no obvious lifetime. + // its life time will be determined by its backing ivar. return; - } - - if (propertyInPrimaryClass) { - // we postpone most property diagnosis until class's implementation - // because, its readonly attribute may be overridden in its class - // extensions making other attributes, which make no sense, to make sense. - if ((Attributes & ObjCDeclSpec::DQ_PR_readonly) && - (Attributes & ObjCDeclSpec::DQ_PR_readwrite)) - Diag(Loc, diag::err_objc_property_attr_mutually_exclusive) - << "readonly" << "readwrite"; - } - // readonly and readwrite/assign/retain/copy conflict. - else if ((Attributes & ObjCDeclSpec::DQ_PR_readonly) && - (Attributes & (ObjCDeclSpec::DQ_PR_readwrite | - ObjCDeclSpec::DQ_PR_assign | - ObjCDeclSpec::DQ_PR_unsafe_unretained | - ObjCDeclSpec::DQ_PR_copy | - ObjCDeclSpec::DQ_PR_retain | - ObjCDeclSpec::DQ_PR_strong))) { - const char * which = (Attributes & ObjCDeclSpec::DQ_PR_readwrite) ? - "readwrite" : - (Attributes & ObjCDeclSpec::DQ_PR_assign) ? - "assign" : - (Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) ? - "unsafe_unretained" : - (Attributes & ObjCDeclSpec::DQ_PR_copy) ? - "copy" : "retain"; - - Diag(Loc, (Attributes & (ObjCDeclSpec::DQ_PR_readwrite)) ? - diag::err_objc_property_attr_mutually_exclusive : - diag::warn_objc_property_attr_mutually_exclusive) - << "readonly" << which; + } + else if (PropertyOwnership) { + if (!getSourceManager().isInSystemHeader(Loc)) + Diag(Loc, diag::warn_objc_property_attr_mutually_exclusive) + << "readonly" << NameOfOwnershipAttribute(Attributes); + return; + } } // Check for copy or retain on non-object types. diff --git a/test/SemaObjC/overriding-property-in-class-extension.m b/test/SemaObjC/overriding-property-in-class-extension.m index 77efd55692..8c0e1b98a5 100644 --- a/test/SemaObjC/overriding-property-in-class-extension.m +++ b/test/SemaObjC/overriding-property-in-class-extension.m @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Weverything %s -// expected-no-diagnostics // rdar://12103434 @class NSString; @@ -8,7 +7,7 @@ @interface MyClass : NSObject -@property (nonatomic, copy, readonly) NSString* name; +@property (nonatomic, copy, readonly) NSString* name; // expected-warning {{property attributes 'readonly' and 'copy' are mutually exclusive}} @end diff --git a/test/SemaObjC/property-in-class-extension-1.m b/test/SemaObjC/property-in-class-extension-1.m index ab461ef6c1..51837fd212 100644 --- a/test/SemaObjC/property-in-class-extension-1.m +++ b/test/SemaObjC/property-in-class-extension-1.m @@ -8,19 +8,20 @@ @property (nonatomic, readonly) NSString* addingMemoryModel; -@property (nonatomic, copy, readonly) NSString* matchingMemoryModel; +@property (nonatomic, copy, readonly) NSString* matchingMemoryModel; // expected-warning {{property attributes 'readonly' and 'copy' are mutually exclusive}} -@property (nonatomic, retain, readonly) NSString* addingNoNewMemoryModel; +@property (nonatomic, retain, readonly) NSString* addingNoNewMemoryModel; // expected-warning {{property attributes 'readonly' and 'retain' are mutually exclusive}} @property (readonly) NSString* none; @property (readonly) NSString* none1; -@property (assign, readonly) NSString* changeMemoryModel; // expected-note {{property declared here}} +@property (assign, readonly) NSString* changeMemoryModel; // expected-note {{property declared here}} \ + // expected-warning {{property attributes 'readonly' and 'assign' are mutually exclusive}} @property (readonly) __weak id weak_prop; @property (readonly) __weak id weak_prop1; -@property (assign, readonly) NSString* assignProperty; +@property (assign, readonly) NSString* assignProperty; // expected-warning {{property attributes 'readonly' and 'assign' are mutually exclusive}} @property (readonly) NSString* readonlyProp; diff --git a/test/SemaObjC/tentative-property-decl.m b/test/SemaObjC/tentative-property-decl.m index f69ac6dace..aa7df5294a 100644 --- a/test/SemaObjC/tentative-property-decl.m +++ b/test/SemaObjC/tentative-property-decl.m @@ -14,7 +14,7 @@ @class NSString; @interface MyClass : Super -@property(nonatomic, copy, readonly) NSString *prop; +@property(nonatomic, copy, readonly) NSString *prop; // expected-warning {{property attributes 'readonly' and 'copy' are mutually exclusive}} @property(nonatomic, copy, readonly) id warnProp; // expected-warning {{property attributes 'readonly' and 'copy' are mutually exclusive}} @end @@ -29,7 +29,7 @@ @protocol P -@property(nonatomic, copy, readonly) NSString *prop; +@property(nonatomic, copy, readonly) NSString *prop; // expected-warning {{property attributes 'readonly' and 'copy' are mutually exclusive}} @property(nonatomic, copy, readonly) id warnProp; // expected-warning {{property attributes 'readonly' and 'copy' are mutually exclusive}} @end