[arcmt] It's not safe to remove the -release on "[[someivar delegate] release];" since it's very likely
that, after migration, the object that was passed to 'setDelegate:' will not be properly retained, e.g:
-whatever {
id x = [[MyDoHicky alloc] init];
[someivar setDelegate: x]; // x won't get retained in ARC.
}
-dealloc {
[[someivar delegate] release]; // give migration error here.
}
rdar://8858009
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135327 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
index 6f917b3..ed6ed0a 100644
--- a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
+++ b/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 @@
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 @@
// 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 @@
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 @@
// 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 @@
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;