[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

1 files changed, 31 insertions, 9 deletions

diff --git a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
index 6f917b3246..ed6ed0adfd 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 @@ 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;