From 8e1555c7c3bf668fbda3fa9c13f0499332e1bf70 Mon Sep 17 00:00:00 2001 From: Fariborz Jahanian Date: Mon, 12 Jan 2009 19:55:42 +0000 Subject: [PATCH] Patch to supprt case of readonly property being assigned to when it has user declared setter method defined in the class implementation (but no declaration in the class itself). llvm-svn: 62098 --- clang/include/clang/AST/DeclObjC.h | 1 - clang/lib/AST/DeclObjC.cpp | 28 -------------- clang/lib/AST/Expr.cpp | 14 +------ clang/lib/Sema/Sema.h | 3 ++ clang/lib/Sema/SemaDeclObjC.cpp | 45 ++++++++++++++++++++++ clang/lib/Sema/SemaExpr.cpp | 25 +++++++++++- clang/test/SemaObjC/property-user-setter.m | 36 +++++++++++++++++ 7 files changed, 109 insertions(+), 43 deletions(-) diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h index b83f51ad53f8..913905066077 100644 --- a/clang/include/clang/AST/DeclObjC.h +++ b/clang/include/clang/AST/DeclObjC.h @@ -405,7 +405,6 @@ public: ObjCCategoryDecl *FindCategoryDeclaration(IdentifierInfo *CategoryId) const; ObjCIvarDecl *FindIvarDeclaration(IdentifierInfo *IvarId) const; - bool isPropertyReadonly(ObjCPropertyDecl *PropertyDecl) const; typedef ObjCList::iterator protocol_iterator; protocol_iterator protocol_begin() const {return ReferencedProtocols.begin();} diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp index c4caed1e00ff..9fc0f763bbd9 100644 --- a/clang/lib/AST/DeclObjC.cpp +++ b/clang/lib/AST/DeclObjC.cpp @@ -251,34 +251,6 @@ void ObjCMethodDecl::setMethodParams(ParmVarDecl **NewParamInfo, } } -/// isPropertyReadonly - Return true if property is readonly, by searching -/// for the property in the class and in its categories. -/// -bool ObjCInterfaceDecl::isPropertyReadonly(ObjCPropertyDecl *PDecl) const -{ - // Even if property is ready only, if interface has a user defined setter, - // it is not considered read only. - if (!PDecl->isReadOnly() || getInstanceMethod(PDecl->getSetterName())) - return false; - - // Main class has the property as 'readonly'. Must search - // through the category list to see if the property's - // attribute has been over-ridden to 'readwrite'. - for (ObjCCategoryDecl *Category = getCategoryList(); - Category; Category = Category->getNextClassCategory()) { - // Even if property is ready only, if a category has a user defined setter, - // it is not considered read only. - if (Category->getInstanceMethod(PDecl->getSetterName())) - return false; - ObjCPropertyDecl *P = - Category->FindPropertyDeclaration(PDecl->getIdentifier()); - if (P && !P->isReadOnly()) - return false; - } - - return true; -} - /// FindCategoryDeclaration - Finds category declaration in the list of /// categories for this class and returns it. Name of the category is passed /// in 'CategoryId'. If category not found, return 0; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 26260132e7ce..89cad4197914 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -604,19 +604,7 @@ Expr::isModifiableLvalueResult Expr::isModifiableLvalue(ASTContext &Ctx) const { if (!BDR->isByRef() && isa(BDR->getDecl())) return MLV_NotBlockQualified; } - // Assigning to a readonly property? - if (getStmtClass() == ObjCPropertyRefExprClass) { - const ObjCPropertyRefExpr* PropExpr = cast(this); - if (ObjCPropertyDecl *PDecl = PropExpr->getProperty()) { - QualType BaseType = PropExpr->getBase()->getType(); - if (const PointerType *PTy = BaseType->getAsPointerType()) - if (const ObjCInterfaceType *IFTy = - PTy->getPointeeType()->getAsObjCInterfaceType()) - if (ObjCInterfaceDecl *IFace = IFTy->getDecl()) - if (IFace->isPropertyReadonly(PDecl)) - return MLV_ReadonlyProperty; - } - } + // Assigning to an 'implicit' property? else if (getStmtClass() == ObjCKVCRefExprClass) { const ObjCKVCRefExpr* KVCExpr = cast(this); diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index 4db6f646b6a3..f098372d97f4 100644 --- a/clang/lib/Sema/Sema.h +++ b/clang/lib/Sema/Sema.h @@ -538,6 +538,9 @@ public: ObjCMethodDecl *IntfMethod); NamespaceDecl *GetStdNamespace(); + + bool isPropertyReadonly(ObjCPropertyDecl *PropertyDecl, + ObjCInterfaceDecl *IDecl) const; /// CheckProtocolMethodDefs - This routine checks unimplemented /// methods declared in protocol, and those referenced by it. diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp index c4b085dc1eb6..afcf45622b91 100644 --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/clang/lib/Sema/SemaDeclObjC.cpp @@ -699,6 +699,51 @@ void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl, } } +/// isPropertyReadonly - Return true if property is readonly, by searching +/// for the property in the class and in its categories and implementations +/// +bool Sema::isPropertyReadonly(ObjCPropertyDecl *PDecl, + ObjCInterfaceDecl *IDecl) const { + // by far the most common case. + if (!PDecl->isReadOnly()) + return false; + // Even if property is ready only, if interface has a user defined setter, + // it is not considered read only. + if (IDecl->getInstanceMethod(PDecl->getSetterName())) + return false; + + // Main class has the property as 'readonly'. Must search + // through the category list to see if the property's + // attribute has been over-ridden to 'readwrite'. + for (ObjCCategoryDecl *Category = IDecl->getCategoryList(); + Category; Category = Category->getNextClassCategory()) { + // Even if property is ready only, if a category has a user defined setter, + // it is not considered read only. + if (Category->getInstanceMethod(PDecl->getSetterName())) + return false; + ObjCPropertyDecl *P = + Category->FindPropertyDeclaration(PDecl->getIdentifier()); + if (P && !P->isReadOnly()) + return false; + } + + // Also, check for definition of a setter method in the implementation if + // all else failed. + if (ObjCMethodDecl *OMD = dyn_cast(CurContext)) { + if (ObjCImplementationDecl *IMD = + dyn_cast(OMD->getDeclContext())) { + if (IMD->getInstanceMethod(PDecl->getSetterName())) + return false; + } + else if (ObjCCategoryImplDecl *CIMD = + dyn_cast(OMD->getDeclContext())) { + if (CIMD->getInstanceMethod(PDecl->getSetterName())) + return false; + } + } + return true; +} + /// FIXME: Type hierarchies in Objective-C can be deep. We could most /// likely improve the efficiency of selector lookups and type /// checking by associating with each protocol / interface / category diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index bf04042ef8a9..d3d3d3556c40 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -2920,10 +2920,33 @@ inline QualType Sema::CheckLogicalOperands( // C99 6.5.[13,14] return InvalidOperands(Loc, lex, rex); } +/// IsReadonlyProperty - Verify that otherwise a valid l-value expression +/// is a read-only property; return true if so. A readonly property expression +/// depends on various declarations and thus must be treated specially. +/// +static bool IsReadonlyProperty(Expr *E, Sema &S) +{ + if (E->getStmtClass() == Expr::ObjCPropertyRefExprClass) { + const ObjCPropertyRefExpr* PropExpr = cast(E); + if (ObjCPropertyDecl *PDecl = PropExpr->getProperty()) { + QualType BaseType = PropExpr->getBase()->getType(); + if (const PointerType *PTy = BaseType->getAsPointerType()) + if (const ObjCInterfaceType *IFTy = + PTy->getPointeeType()->getAsObjCInterfaceType()) + if (ObjCInterfaceDecl *IFace = IFTy->getDecl()) + if (S.isPropertyReadonly(PDecl, IFace)) + return true; + } + } + return false; +} + /// CheckForModifiableLvalue - Verify that E is a modifiable lvalue. If not, /// emit an error and return true. If so, return false. static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) { - Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context); + Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context); + if (IsLV == Expr::MLV_Valid && IsReadonlyProperty(E, S)) + IsLV = Expr::MLV_ReadonlyProperty; if (IsLV == Expr::MLV_Valid) return false; diff --git a/clang/test/SemaObjC/property-user-setter.m b/clang/test/SemaObjC/property-user-setter.m index cb2091e8cedb..d4da1dfed9f4 100644 --- a/clang/test/SemaObjC/property-user-setter.m +++ b/clang/test/SemaObjC/property-user-setter.m @@ -23,3 +23,39 @@ self.z = 2; // expected-error {{assigning to property with 'readonly' attribute not allowed}} } @end + +// Test when property is 'readonly' but it has a setter in +// its implementation only. +@interface I1 { +} +@property(readonly) int identifier; +@end + + +@implementation I1 +@dynamic identifier; +- (void)setIdentifier:(int)ident {} + +- (id)initWithIdentifier:(int)Arg { + self.identifier = 0; +} + +@end + + +// Also in a category implementation +@interface I1(CAT) +@property(readonly) int rprop; +@end + + +@implementation I1(CAT) +@dynamic rprop; +- (void)setRprop:(int)ident {} + +- (id)initWithIdentifier:(int)Arg { + self.rprop = 0; +} + +@end +