[ARC] Complain about property without storage attribute when @synthesizing it, not...

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 a60d2c92f73cab9f79da42ba1a44a32a1a97b454..91126f10547bd1c51c295ec93afbc23fd84b1a03 100644 (file)
--- 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 c1b09f7d6d7bf1a09e45adc06766a2edb519a8da..97414f23d796def0088e6e5b8ecdae6594a43f44 100644 (file)
--- 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 b47d62780b90d670d65e6b67b2a3de2cb4380904..c830c3e166261f2b4899d11020f22a471c5d29d5 100644 (file)
--- 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 10df61e6009a14c546456446d6729bab1199a1f7..61dc0e052541edcd879e86763ef2c2dba7672cd0 100644 (file)
--- 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}}