[cfe-commits] r135193 - in /cfe/trunk: lib/ARCMigrate/TransRetainReleaseDealloc.cpp test/ARCMT/checking.m test/ARCMT/cxx-rewrite.mm test/ARCMT/remove-statements.m test/ARCMT/retains.m

[Argyrios Kyrtzidis]

Author: akirtzidis
Date: Thu Jul 14 16:26:49 2011
New Revision: 135193

URL: http://llvm.org/viewvc/llvm-project?rev=135193&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
    cfe/trunk/test/ARCMT/checking.m
    cfe/trunk/test/ARCMT/cxx-rewrite.mm
    cfe/trunk/test/ARCMT/remove-statements.m
    cfe/trunk/test/ARCMT/retains.m

Modified: cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp?rev=135193&r1=135192&r2=135193&view=diff
==============================================================================
--- cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/TransRetainReleaseDealloc.cpp Thu Jul 14 16:26:49 2011
@@ -52,14 +52,26 @@
     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());

Modified: cfe/trunk/test/ARCMT/checking.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/checking.m?rev=135193&r1=135192&r2=135193&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/checking.m (original)
+++ cfe/trunk/test/ARCMT/checking.m Thu Jul 14 16:26:49 2011
@@ -37,13 +37,14 @@
 @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}} \

Modified: cfe/trunk/test/ARCMT/cxx-rewrite.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/cxx-rewrite.mm?rev=135193&r1=135192&r2=135193&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/cxx-rewrite.mm (original)
+++ cfe/trunk/test/ARCMT/cxx-rewrite.mm Thu Jul 14 16:26:49 2011
@@ -12,7 +12,7 @@
     NSString *s;
     foo(NSString *s): s([s retain]){
         NSAutoreleasePool *pool = [NSAutoreleasePool new];
-        [[NSString string] autorelease];
+        [[[NSString string] retain] release];
         [pool drain];
     }
     ~foo(){ [s release]; }

Modified: cfe/trunk/test/ARCMT/remove-statements.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/remove-statements.m?rev=135193&r1=135192&r2=135193&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/remove-statements.m (original)
+++ cfe/trunk/test/ARCMT/remove-statements.m Thu Jul 14 16:26:49 2011
@@ -13,7 +13,7 @@
 
 @implementation myController
 -(id) test:(id) x {
-  [[x retain] autorelease];
+  [[x retain] release];
   return [[x retain] autorelease];
 }
 

Modified: cfe/trunk/test/ARCMT/retains.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ARCMT/retains.m?rev=135193&r1=135192&r2=135193&view=diff
==============================================================================
--- cfe/trunk/test/ARCMT/retains.m (original)
+++ cfe/trunk/test/ARCMT/retains.m Thu Jul 14 16:26:49 2011
@@ -38,8 +38,8 @@
 
   [[self retain] something];
 
-  [[IhaveSideEffect() retain] autorelease];
-  [[x retain] autorelease];
+  [[IhaveSideEffect() retain] release];
+  [[x retain] release];
   // do stuff with x;
   [x release];
   return [self retain];