From e0e40768cc8c4b2a9093dac3d777e0d362cb7a88 Mon Sep 17 00:00:00 2001
From: Argyrios Kyrtzidis <akyrtzi@gmail.com>
Date: Thu, 14 Jul 2011 21:26:49 +0000
Subject: [PATCH] [arcmt] Emit an error for unused -autorelease messages.

An unused autorelease is badness. If we remove it the receiver
will likely die immediately while previously it was kept alive
by the autorelease pool. This is bad practice in general, so leave it
and emit an error to force the user to restructure his code.

rdar://9599884

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135193 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/ARCMigrate/TransRetainReleaseDealloc.cpp | 16 ++++++++++++++--
 test/ARCMT/checking.m                        |  5 +++--
 test/ARCMT/cxx-rewrite.mm                    |  2 +-
 test/ARCMT/remove-statements.m               |  2 +-
 test/ARCMT/retains.m                         |  4 ++--
 5 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
index fe80a43a6a..d64250faf0 100644
--- a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
+++ b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
@@ -52,14 +52,26 @@ public:
     switch (E->getMethodFamily()) {
     default:
       return true;
+    case OMF_autorelease:
+      if (isRemovable(E)) {
+        // An unused autorelease is badness. If we remove it the receiver
+        // 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());
+        return true;
+      }
+      // Pass through.
     case OMF_retain:
     case OMF_release:
-    case OMF_autorelease:
       if (E->getReceiverKind() == ObjCMessageExpr::Instance)
         if (Expr *rec = E->getInstanceReceiver()) {
           rec = rec->IgnoreParenImpCasts();
           if (rec->getType().getObjCLifetime() == Qualifiers::OCL_ExplicitNone){
-            std::string err = "It is not safe to remove '";
+            std::string err = "it is not safe to remove '";
             err += E->getSelector().getAsString() + "' message on "
                 "an __unsafe_unretained type";
             Pass.TA.reportError(err, rec->getLocStart());
diff --git a/test/ARCMT/checking.m b/test/ARCMT/checking.m
index ed1592416b..4dea3b71df 100644
--- a/test/ARCMT/checking.m
+++ b/test/ARCMT/checking.m
@@ -37,13 +37,14 @@ struct UnsafeS {
 @end
 
 void test1(A *a, BOOL b, struct UnsafeS *unsafeS) {
-  [unsafeS->unsafeObj retain]; // expected-error {{It is not safe to remove 'retain' message on an __unsafe_unretained type}} \
+  [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}}
   [a dealloc];
   [a retain];
   [a retainCount]; // expected-error {{ARC forbids explicit message send of 'retainCount'}}
   [a release];
-  [a autorelease];
+  [a autorelease]; // expected-error {{it is not safe to remove an unused 'autorelease'; its receiver may be destroyed immediately}} \
+                   // expected-error {{ARC forbids explicit message send}}
 
   CFStringRef cfstr;
   NSString *str = (NSString *)cfstr; // expected-error {{cast of C pointer type 'CFStringRef' (aka 'const struct __CFString *') to Objective-C pointer type 'NSString *' requires a bridged cast}} \
diff --git a/test/ARCMT/cxx-rewrite.mm b/test/ARCMT/cxx-rewrite.mm
index ab402e41cf..aba3f7568b 100644
--- a/test/ARCMT/cxx-rewrite.mm
+++ b/test/ARCMT/cxx-rewrite.mm
@@ -12,7 +12,7 @@ struct foo {
     NSString *s;
     foo(NSString *s): s([s retain]){
         NSAutoreleasePool *pool = [NSAutoreleasePool new];
-        [[NSString string] autorelease];
+        [[[NSString string] retain] release];
         [pool drain];
     }
     ~foo(){ [s release]; }
diff --git a/test/ARCMT/remove-statements.m b/test/ARCMT/remove-statements.m
index c433ec9913..7e10296126 100644
--- a/test/ARCMT/remove-statements.m
+++ b/test/ARCMT/remove-statements.m
@@ -13,7 +13,7 @@
 
 @implementation myController
 -(id) test:(id) x {
-  [[x retain] autorelease];
+  [[x retain] release];
   return [[x retain] autorelease];
 }
 
diff --git a/test/ARCMT/retains.m b/test/ARCMT/retains.m
index d7938376ab..fa90f218dd 100644
--- a/test/ARCMT/retains.m
+++ b/test/ARCMT/retains.m
@@ -38,8 +38,8 @@ id IhaveSideEffect();
 
   [[self retain] something];
 
-  [[IhaveSideEffect() retain] autorelease];
-  [[x retain] autorelease];
+  [[IhaveSideEffect() retain] release];
+  [[x retain] release];
   // do stuff with x;
   [x release];
   return [self retain];
-- 
2.40.0