From: Daniel Dunbar Date: Tue, 23 Sep 2008 21:53:23 +0000 (+0000) Subject: Implement type checking of Objective-C property attributes. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=95e61fb7cdd1becf956f897f90abffd8b70575de;p=clang Implement type checking of Objective-C property attributes. - readonly and readwrite are mutually exclusive. - assign, copy, and retain are mutually exclusive. - copy and retain are invalid on non-object types. - Warn about using default 'assign' property on object types (attempting to follow gcc behavior). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@56507 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def index 7326fb1207..d266adb7c7 100644 --- a/include/clang/Basic/DiagnosticKinds.def +++ b/include/clang/Basic/DiagnosticKinds.def @@ -421,6 +421,14 @@ DIAG(err_objc_expected_equal, ERROR, "setter/getter expects '=' followed by name") DIAG(err_objc_expected_property_attr, ERROR, "unknown property attribute detected") +DIAG(err_objc_property_attr_mutually_exclusive, ERROR, + "property attributes '%0' and '%1' are mutually exclusive") +DIAG(warn_objc_property_no_assignment_attribute, WARNING, + "no 'assign', 'retain', or 'copy' attribute is specified - 'assign' is assumed") +DIAG(warn_objc_property_default_assign_on_object, WARNING, + "default property attribute 'assign' not appropriate for non-gc object") +DIAG(err_objc_property_requires_object, ERROR, + "property with '%0' attribute must be of object type") DIAG(err_objc_protocol_required, ERROR, "@required may be specified in protocols only") DIAG(err_objc_protocol_optional, ERROR, diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 4fbd735113..ef5954b629 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -723,6 +723,12 @@ public: unsigned NumProtocols, llvm::SmallVectorImpl &Protocols); + /// Ensure attributes are consistent with type. + /// \param [in, out] Attributes The attributes to check; they will + /// be modified to be consistent with \arg PropertyTy. + void CheckObjCPropertyAttributes(QualType PropertyTy, + SourceLocation Loc, + unsigned &Attributes); void DiagnosePropertyMismatch(ObjCPropertyDecl *Property, ObjCPropertyDecl *SuperProperty, const char *Name); diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index 240d760b75..dbb3cf22ad 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -1038,6 +1038,68 @@ Sema::DeclTy *Sema::ActOnMethodDeclaration( return ObjCMethod; } +void Sema::CheckObjCPropertyAttributes(QualType PropertyTy, + SourceLocation Loc, + unsigned &Attributes) { + // FIXME: Improve the reported location. + + // readonly and readwrite conflict. + if ((Attributes & ObjCDeclSpec::DQ_PR_readonly) && + (Attributes & ObjCDeclSpec::DQ_PR_readwrite)) { + Diag(Loc, diag::err_objc_property_attr_mutually_exclusive, + "readonly", "readwrite"); + Attributes ^= ObjCDeclSpec::DQ_PR_readonly; + } + + // Check for copy or retain on non-object types. + if ((Attributes & (ObjCDeclSpec::DQ_PR_copy | ObjCDeclSpec::DQ_PR_retain)) && + !Context.isObjCObjectPointerType(PropertyTy)) { + Diag(Loc, diag::err_objc_property_requires_object, + Attributes & ObjCDeclSpec::DQ_PR_copy ? "copy" : "retain"); + Attributes &= ~(ObjCDeclSpec::DQ_PR_copy | ObjCDeclSpec::DQ_PR_retain); + } + + // Check for more than one of { assign, copy, retain }. + if (Attributes & ObjCDeclSpec::DQ_PR_assign) { + if (Attributes & ObjCDeclSpec::DQ_PR_copy) { + Diag(Loc, diag::err_objc_property_attr_mutually_exclusive, + "assign", "copy"); + Attributes ^= ObjCDeclSpec::DQ_PR_copy; + } + if (Attributes & ObjCDeclSpec::DQ_PR_retain) { + Diag(Loc, diag::err_objc_property_attr_mutually_exclusive, + "assign", "retain"); + Attributes ^= ObjCDeclSpec::DQ_PR_retain; + } + } else if (Attributes & ObjCDeclSpec::DQ_PR_copy) { + if (Attributes & ObjCDeclSpec::DQ_PR_retain) { + Diag(Loc, diag::err_objc_property_attr_mutually_exclusive, + "copy", "retain"); + Attributes ^= ObjCDeclSpec::DQ_PR_retain; + } + } + + // Warn if user supplied no assignment attribute, property is + // readwrite, and this is an object type. + if (!(Attributes & (ObjCDeclSpec::DQ_PR_assign | ObjCDeclSpec::DQ_PR_copy | + ObjCDeclSpec::DQ_PR_retain)) && + !(Attributes & ObjCDeclSpec::DQ_PR_readonly) && + Context.isObjCObjectPointerType(PropertyTy)) { + // 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: + // + // (please trim this list while you are at it). + } +} + Sema::DeclTy *Sema::ActOnProperty(Scope *S, SourceLocation AtLoc, FieldDeclarator &FD, ObjCDeclSpec &ODS, @@ -1045,6 +1107,11 @@ Sema::DeclTy *Sema::ActOnProperty(Scope *S, SourceLocation AtLoc, Selector SetterSel, tok::ObjCKeywordKind MethodImplKind) { QualType T = GetTypeForDeclarator(FD.D, S); + unsigned Attributes = ODS.getPropertyAttributes(); + + // May modify Attributes. + CheckObjCPropertyAttributes(T, AtLoc, Attributes); + ObjCPropertyDecl *PDecl = ObjCPropertyDecl::Create(Context, AtLoc, FD.D.getIdentifier(), T); // Regardless of setter/getter attribute, we save the default getter/setter @@ -1052,28 +1119,28 @@ Sema::DeclTy *Sema::ActOnProperty(Scope *S, SourceLocation AtLoc, PDecl->setGetterName(GetterSel); PDecl->setSetterName(SetterSel); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_readonly) + if (Attributes & ObjCDeclSpec::DQ_PR_readonly) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_readonly); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_getter) + if (Attributes & ObjCDeclSpec::DQ_PR_getter) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_getter); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_setter) + if (Attributes & ObjCDeclSpec::DQ_PR_setter) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_setter); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_assign) + if (Attributes & ObjCDeclSpec::DQ_PR_assign) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_assign); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_readwrite) + if (Attributes & ObjCDeclSpec::DQ_PR_readwrite) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_readwrite); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_retain) + if (Attributes & ObjCDeclSpec::DQ_PR_retain) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_retain); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_copy) + if (Attributes & ObjCDeclSpec::DQ_PR_copy) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_copy); - if (ODS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_nonatomic) + if (Attributes & ObjCDeclSpec::DQ_PR_nonatomic) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_nonatomic); if (MethodImplKind == tok::objc_required) @@ -1116,7 +1183,7 @@ Sema::DeclTy *Sema::ActOnPropertyImplDecl(SourceLocation AtLoc, // Look for this property declaration in the @implementation's @interface property = IDecl->FindPropertyDeclaration(PropertyId); if (!property) { - Diag(PropertyLoc, diag::error_bad_property_decl, IDecl->getName()); + Diag(PropertyLoc, diag::error_bad_property_decl, IDecl->getName()); return 0; } } diff --git a/test/SemaObjC/property-1.m b/test/SemaObjC/property-1.m index 6cdf8de021..2641105162 100644 --- a/test/SemaObjC/property-1.m +++ b/test/SemaObjC/property-1.m @@ -6,7 +6,7 @@ int name; } @property int d1; -@property id prop_id; +@property id prop_id; // 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}} @property int name; @end diff --git a/test/SemaObjC/property-10.m b/test/SemaObjC/property-10.m new file mode 100644 index 0000000000..69a5ae80fe --- /dev/null +++ b/test/SemaObjC/property-10.m @@ -0,0 +1,18 @@ +// RUN: clang -fsyntax-only -verify %s + +// Check property attribute consistency. + +@interface I0 +@property(readonly, readwrite) int p0; // expected-error {{property attributes 'readonly' and 'readwrite' are mutually exclusive}} + +@property(retain) int p1; // expected-error {{property with 'retain' attribute must be of object type}} + +@property(copy) int p2; // expected-error {{property with 'copy' attribute must be of object type}} + +@property(assign, copy) id p3_0; // expected-error {{property attributes 'assign' and 'copy' are mutually exclusive}} +@property(assign, retain) id p3_1; // expected-error {{property attributes 'assign' and 'retain' are mutually exclusive}} +@property(copy, retain) id p3_2; // expected-error {{property attributes 'copy' and 'retain' are mutually exclusive}} +@property(assign, copy, retain) id p3_3; // expected-error {{property attributes 'assign' and 'copy' are mutually exclusive}}, expected-error {{property attributes 'assign' and 'retain' are mutually exclusive}} + +@property id p4; // 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}} +@end