diff options
-rw-r--r-- | clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp | 40 | ||||
-rw-r--r-- | clang/test/ARCMT/checking.m | 9 |
2 files changed, 39 insertions, 10 deletions
diff --git a/clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp b/clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp index 6f917b32465..ed6ed0adfdf 100644 --- a/clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp +++ b/clang/lib/ARCMigrate/TransRetainReleaseDealloc.cpp @@ -19,9 +19,7 @@ #include "Transforms.h" #include "Internals.h" -#include "clang/Sema/Sema.h" #include "clang/Sema/SemaDiagnostic.h" -#include "clang/Lex/Preprocessor.h" #include "clang/AST/ParentMap.h" using namespace clang; @@ -39,9 +37,14 @@ class RetainReleaseDeallocRemover : ExprSet Removables; llvm::OwningPtr<ParentMap> StmtMap; + Selector DelegateSel; + public: RetainReleaseDeallocRemover(MigrationPass &pass) - : Body(0), Pass(pass) { } + : Body(0), Pass(pass) { + DelegateSel = + Pass.Ctx.Selectors.getNullarySelector(&Pass.Ctx.Idents.get("delegate")); + } void transformBody(Stmt *body) { Body = body; @@ -60,10 +63,9 @@ public: // will likely die immediately while previously it was kept alive // by the autorelease pool. This is bad practice in general, leave it // and emit an error to force the user to restructure his code. - std::string err = "it is not safe to remove an unused '"; - err += E->getSelector().getAsString() + "'; its receiver may be " - "destroyed immediately"; - Pass.TA.reportError(err, E->getLocStart(), E->getSourceRange()); + Pass.TA.reportError("it is not safe to remove an unused 'autorelease' " + "message; its receiver may be destroyed immediately", + E->getLocStart(), E->getSourceRange()); return true; } // Pass through. @@ -89,6 +91,14 @@ public: Pass.TA.reportError(err, rec->getLocStart()); return true; } + + if (E->getMethodFamily() == OMF_release && isDelegateMessage(rec)) { + Pass.TA.reportError("it is not safe to remove 'retain' " + "message on the result of a 'delegate' message; " + "the object that was passed to 'setDelegate:' may not be " + "properly retained", rec->getLocStart()); + return true; + } } case OMF_dealloc: break; @@ -120,8 +130,7 @@ public: // Change the -release to "receiver = nil" in a finally to avoid a leak // when an exception is thrown. Pass.TA.replace(E->getSourceRange(), rec->getSourceRange()); - if (Pass.SemaRef.getPreprocessor() - .getIdentifierInfo("nil")->hasMacroDefinition()) + if (Pass.Ctx.Idents.get("nil").hasMacroDefinition()) Pass.TA.insertAfterToken(rec->getLocEnd(), " = nil"); else Pass.TA.insertAfterToken(rec->getLocEnd(), " = 0"); @@ -145,6 +154,19 @@ private: loc); } + bool isDelegateMessage(Expr *E) const { + if (!E) return false; + + E = E->IgnoreParenCasts(); + if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(E)) + return (ME->isInstanceMessage() && ME->getSelector() == DelegateSel); + + if (ObjCPropertyRefExpr *propE = dyn_cast<ObjCPropertyRefExpr>(E)) + return propE->getGetterSelector() == DelegateSel; + + return false; + } + bool isInAtFinally(Expr *E) const { assert(E); Stmt *S = E; diff --git a/clang/test/ARCMT/checking.m b/clang/test/ARCMT/checking.m index 2e4ec122d64..eea7a9f2547 100644 --- a/clang/test/ARCMT/checking.m +++ b/clang/test/ARCMT/checking.m @@ -20,6 +20,7 @@ struct UnsafeS { - (oneway void)release; - (void)dealloc; -(void)test; +-(id)delegate; @end @implementation A @@ -34,11 +35,17 @@ struct UnsafeS { - (id)retainCount { return self; } // expected-error {{ARC forbids implementation}} - (id)autorelease { return self; } // expected-error {{ARC forbids implementation}} - (oneway void)release { } // expected-error {{ARC forbids implementation}} + +-(id)delegate { return self; } @end id global_foo; void test1(A *a, BOOL b, struct UnsafeS *unsafeS) { + [[a delegate] release]; // expected-error {{it is not safe to remove 'retain' message on the result of a 'delegate' message; the object that was passed to 'setDelegate:' may not be properly retained}} \ + // expected-error {{ARC forbids explicit message send}} + [a.delegate release]; // expected-error {{it is not safe to remove 'retain' message on the result of a 'delegate' message; the object that was passed to 'setDelegate:' may not be properly retained}} \ + // expected-error {{ARC forbids explicit message send}} [unsafeS->unsafeObj retain]; // expected-error {{it is not safe to remove 'retain' message on an __unsafe_unretained type}} \ // expected-error {{ARC forbids explicit message send}} id foo = [unsafeS->unsafeObj retain]; // no warning. @@ -50,7 +57,7 @@ void test1(A *a, BOOL b, struct UnsafeS *unsafeS) { [a retain]; [a retainCount]; // expected-error {{ARC forbids explicit message send of 'retainCount'}} [a release]; - [a autorelease]; // expected-error {{it is not safe to remove an unused 'autorelease'; its receiver may be destroyed immediately}} \ + [a autorelease]; // expected-error {{it is not safe to remove an unused 'autorelease' message; its receiver may be destroyed immediately}} \ // expected-error {{ARC forbids explicit message send}} CFStringRef cfstr; |