From: Argyrios Kyrtzidis Date: Tue, 12 Jul 2011 04:30:16 +0000 (+0000) Subject: [ARC] Complain about property without storage attribute when @synthesizing it, not... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0a68dc7f04be1542ce249ff4adb169453698ad91;p=clang [ARC] Complain about property without storage attribute when @synthesizing it, not at its declaration. For this sample: @interface Foo @property id x; @end we get: t.m:2:1: error: ARC forbids properties of Objective-C objects with unspecified storage attribute @property id x; ^ 1 error generated. The error should be imposed on the implementor of the interface, not the user. If the user uses a header of a non-ARC library whose source code he does not have, we are basically asking him to go change the header of the library (bad in general), possible overriding how the property is implemented if he gets confused and says "Oh I'll just add 'copy' then" (even worse). Second issue is that we don't emit any error for 'readonly' properties, e.g: @interface Foo @property (readonly) id x; // no error here @end @implementation Foo @synthesize x; // no error here too @end We should give an error when the implementor is @synthesizing a property which doesn't have any storage specifier; this is when the explicit specifier is important, because we are going to create an ivar and we want its ownership to be explicit. Related improvements: -OBJC_PR_unsafe_unretained turned out to not fit in ObjCPropertyDecl's bitfields, fix it. -For properties of extension classes don't drop PropertyAttributesAsWritten values. -Have PropertyAttributesAsWritten actually only reflect what the user wrote rdar://9756610. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@134960 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h index a60d2c92f7..91126f1054 100644 --- a/include/clang/AST/DeclObjC.h +++ b/include/clang/AST/DeclObjC.h @@ -1414,7 +1414,10 @@ public: OBJC_PR_atomic = 0x100, OBJC_PR_weak = 0x200, OBJC_PR_strong = 0x400, - OBJC_PR_unsafe_unretained = 0x800 + OBJC_PR_unsafe_unretained = 0x800, + + /// \brief Number of bits fitting all the property attributes. + OBJC_PR_NumBits = 12 }; enum SetterKind { Assign, Retain, Copy }; @@ -1422,8 +1425,8 @@ public: private: SourceLocation AtLoc; // location of @property TypeSourceInfo *DeclType; - unsigned PropertyAttributes : 11; - unsigned PropertyAttributesAsWritten : 11; + unsigned PropertyAttributes : OBJC_PR_NumBits; + unsigned PropertyAttributesAsWritten : OBJC_PR_NumBits; // @required/@optional unsigned PropertyImplementation : 2; @@ -1466,6 +1469,12 @@ public: PropertyAttributeKind getPropertyAttributesAsWritten() const { return PropertyAttributeKind(PropertyAttributesAsWritten); } + + bool hasWrittenStorageAttribute() const { + return PropertyAttributesAsWritten & (OBJC_PR_assign | OBJC_PR_copy | + OBJC_PR_unsafe_unretained | OBJC_PR_retain | OBJC_PR_strong | + OBJC_PR_weak); + } void setPropertyAttributesAsWritten(PropertyAttributeKind PRVal) { PropertyAttributesAsWritten = PRVal; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index c1b09f7d6d..97414f23d7 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2602,7 +2602,7 @@ def err_arc_mismatched_cast : Error< def err_arc_objc_object_in_struct : Error< "ARC forbids Objective-C objects in structs or unions">; def err_arc_objc_property_default_assign_on_object : Error< - "ARC forbids properties of Objective-C objects " + "ARC forbids synthesizing a property of an Objective-C object " "with unspecified storage attribute">; def err_arc_illegal_selector : Error< "ARC forbids use of %0 in a @selector">; diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index b47d62780b..c830c3e166 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/lib/Sema/SemaObjCProperty.cpp @@ -200,10 +200,6 @@ Sema::HandlePropertyInClassExtension(Scope *S, ObjCCategoryDecl *CDecl, CreatePropertyDecl(S, CCPrimary, AtLoc, FD, GetterSel, SetterSel, isAssign, isReadWrite, Attributes, T, MethodImplKind, DC); - // Mark written attribute as having no attribute because - // this is not a user-written property declaration in primary - // class. - PDecl->setPropertyAttributesAsWritten(ObjCPropertyDecl::OBJC_PR_noattr); // A case of continuation class adding a new property in the class. This // is not what it was meant for. However, gcc supports it and so should we. @@ -336,6 +332,35 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S, PDecl->setGetterName(GetterSel); PDecl->setSetterName(SetterSel); + unsigned attributesAsWritten = 0; + if (Attributes & ObjCDeclSpec::DQ_PR_readonly) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_readonly; + if (Attributes & ObjCDeclSpec::DQ_PR_readwrite) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_readwrite; + if (Attributes & ObjCDeclSpec::DQ_PR_getter) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_getter; + if (Attributes & ObjCDeclSpec::DQ_PR_setter) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_setter; + if (Attributes & ObjCDeclSpec::DQ_PR_assign) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_assign; + if (Attributes & ObjCDeclSpec::DQ_PR_retain) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_retain; + if (Attributes & ObjCDeclSpec::DQ_PR_strong) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_strong; + if (Attributes & ObjCDeclSpec::DQ_PR_weak) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_weak; + if (Attributes & ObjCDeclSpec::DQ_PR_copy) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_copy; + if (Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_unsafe_unretained; + if (Attributes & ObjCDeclSpec::DQ_PR_nonatomic) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_nonatomic; + if (Attributes & ObjCDeclSpec::DQ_PR_atomic) + attributesAsWritten |= ObjCPropertyDecl::OBJC_PR_atomic; + + PDecl->setPropertyAttributesAsWritten( + (ObjCPropertyDecl::PropertyAttributeKind)attributesAsWritten); + if (Attributes & ObjCDeclSpec::DQ_PR_readonly) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_readonly); @@ -371,11 +396,6 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S, else if (Attributes & ObjCDeclSpec::DQ_PR_atomic) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_atomic); - // FIXME: Why do PropertyAttributesAsWritten get set from PropertyAttributes, - // shouldn't PropertyAttributesAsWritten get set *only* through the attributes - // of the ObjCDeclSpec ? - PDecl->setPropertyAttributesAsWritten(PDecl->getPropertyAttributes()); - // 'unsafe_unretained' is alias for 'assign'. if (Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_assign); @@ -565,6 +585,13 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, ObjCIvarDecl *Ivar = 0; // Check that we have a valid, previously declared ivar for @synthesize if (Synthesize) { + if (getLangOptions().ObjCAutoRefCount && + !property->hasWrittenStorageAttribute() && + property->getType()->isObjCRetainableType()) { + Diag(PropertyLoc, diag::err_arc_objc_property_default_assign_on_object); + Diag(property->getLocation(), diag::note_property_declare); + } + // @synthesize if (!PropertyIvar) PropertyIvar = PropertyId; @@ -1672,17 +1699,13 @@ void Sema::CheckObjCPropertyAttributes(Decl *PDecl, ObjCDeclSpec::DQ_PR_weak)) && !(Attributes & ObjCDeclSpec::DQ_PR_readonly) && PropertyTy->isObjCObjectPointerType()) { - if (getLangOptions().ObjCAutoRefCount) - Diag(Loc, diag::err_arc_objc_property_default_assign_on_object); - else { - // Skip this warning in gc-only mode. - if (getLangOptions().getGCMode() != LangOptions::GCOnly) - Diag(Loc, diag::warn_objc_property_no_assignment_attribute); - - // If non-gc code warn that this is likely inappropriate. - if (getLangOptions().getGCMode() == LangOptions::NonGC) - Diag(Loc, diag::warn_objc_property_default_assign_on_object); - } + // Skip this warning in gc-only mode. + if (getLangOptions().getGCMode() != LangOptions::GCOnly) + Diag(Loc, diag::warn_objc_property_no_assignment_attribute); + + // If non-gc code warn that this is likely inappropriate. + if (getLangOptions().getGCMode() == LangOptions::NonGC) + Diag(Loc, diag::warn_objc_property_default_assign_on_object); // FIXME: Implement warning dependent on NSCopying being // implemented. See also: diff --git a/test/SemaObjC/arc.m b/test/SemaObjC/arc.m index 10df61e600..61dc0e0525 100644 --- a/test/SemaObjC/arc.m +++ b/test/SemaObjC/arc.m @@ -484,10 +484,21 @@ void test26(id y) { // rdar://9525555 @interface Test27 -@property id x; // expected-error {{ARC forbids properties of Objective-C objects with unspecified storage attribute}} +@property id x; // expected-warning {{no 'assign', 'retain', or 'copy' attribute is specified - 'assign' is assumed}} \ + // expected-warning {{default property attribute 'assign' not appropriate for non-gc object}} \ + // expected-note {{declared here}} +@property (readonly) id ro; // expected-note {{declared here}} +@property (readonly) id custom_ro; @property int y; @end +@implementation Test27 +@synthesize x; // expected-error {{ARC forbids synthesizing a property of an Objective-C object with unspecified storage attribute}} +@synthesize ro; // expected-error {{ARC forbids synthesizing a property of an Objective-C object with unspecified storage attribute}} +@synthesize y; +-(id)custom_ro { return 0; } +@end + // rdar://9569264 @interface Test28 @property (nonatomic, assign) __strong id a; // expected-error {{unsafe_unretained property 'a' may not also be declared __strong}}