[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

diff --git a/lib/ARCMigrate/TransRetainReleaseDealloc.cpp b/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
index fe80a43a6aeaeada890b0e32a6317360442dfd76..d64250faf0206773cea4afef884f58e384a2797e 100644 (file)
--- 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 ed1592416b55ca60aec1f65be81ce3e0361912c4..4dea3b71dff8a70542200aa2dde11798fa892c3b 100644 (file)
--- 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 ab402e41cfa8c8bfd6561cb5d966e3f84e3b649c..aba3f7568b031dc8008d31fcfb5b381bd6e50a18 100644 (file)
--- 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 c433ec9913c166cb679ece0104f5db1e5a16848d..7e102961263a142f8552c5eeba2eb50c1e3e28e6 100644 (file)
--- 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 d7938376ab85e7345851d9ec3cb2def0e42c507d..fa90f218dd5d4217f2342350306f02a9c97b779b 100644 (file)
--- 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];