[cfe-commits] r134960 - in /cfe/trunk: include/clang/AST/DeclObjC.h include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaObjCProperty.cpp test/SemaObjC/arc.m
Argyrios Kyrtzidis
akyrtzi at gmail.com
Mon Jul 11 21:30:16 PDT 2011
Author: akirtzidis
Date: Mon Jul 11 23:30:16 2011
New Revision: 134960
URL: http://llvm.org/viewvc/llvm-project?rev=134960&view=rev
Log:
[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.
Modified:
cfe/trunk/include/clang/AST/DeclObjC.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaObjCProperty.cpp
cfe/trunk/test/SemaObjC/arc.m
Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=134960&r1=134959&r2=134960&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Mon Jul 11 23:30:16 2011
@@ -1414,7 +1414,10 @@
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 @@
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 @@
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;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=134960&r1=134959&r2=134960&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 11 23:30:16 2011
@@ -2602,7 +2602,7 @@
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">;
Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=134960&r1=134959&r2=134960&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Mon Jul 11 23:30:16 2011
@@ -200,10 +200,6 @@
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 @@
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 @@
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 @@
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 @@
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:
Modified: cfe/trunk/test/SemaObjC/arc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc.m?rev=134960&r1=134959&r2=134960&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc.m (original)
+++ cfe/trunk/test/SemaObjC/arc.m Mon Jul 11 23:30:16 2011
@@ -484,10 +484,21 @@
// rdar://9525555
@interface Test27
- at property id x; // expected-error {{ARC forbids properties of Objective-C objects with unspecified storage attribute}}
+ at 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}}
+ at property (readonly) id ro; // expected-note {{declared here}}
+ at property (readonly) id custom_ro;
@property int y;
@end
+ at implementation Test27
+ at synthesize x; // expected-error {{ARC forbids synthesizing a property of an Objective-C object with unspecified storage attribute}}
+ at synthesize ro; // expected-error {{ARC forbids synthesizing a property of an Objective-C object with unspecified storage attribute}}
+ at synthesize y;
+-(id)custom_ro { return 0; }
+ at end
+
// rdar://9569264
@interface Test28
@property (nonatomic, assign) __strong id a; // expected-error {{unsafe_unretained property 'a' may not also be declared __strong}}
More information about the cfe-commits
mailing list