ahatanak created this revision.

This is a follow-up patch to r314370.

Rather than throwing away the enclosing parentheses in maybeUndoReclaimObject, 
this patch walks down the expression until an ARCReclaimReturnedObject cast is 
found and removes just the cast, preserving the syntactic sugar expressions 
(parens and casts) that were visited up to that point.

rdar://problem/34705720


https://reviews.llvm.org/D38659

Files:
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/arc-bridged-cast.m


Index: test/CodeGenObjC/arc-bridged-cast.m
===================================================================
--- test/CodeGenObjC/arc-bridged-cast.m
+++ test/CodeGenObjC/arc-bridged-cast.m
@@ -102,5 +102,6 @@
   // CHECK-NOT: call i8* @objc_retainAutoreleasedReturnValue(
   // CHECK-NOT: call void @objc_release(
   CFStringRef r = (__bridge CFStringRef)(CreateNSString());
+  r = (__bridge CFStringRef)((NSString *)(CreateNSString()));
   return r;
 }
Index: lib/Sema/SemaExprObjC.cpp
===================================================================
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -4317,14 +4317,37 @@
 
 /// Look for an ObjCReclaimReturnedObject cast and destroy it.
 static Expr *maybeUndoReclaimObject(Expr *e) {
-  // For now, we just undo operands that are *immediately* reclaim
-  // expressions, which prevents the vast majority of potential
-  // problems here.  To catch them all, we'd need to rebuild arbitrary
-  // value-propagating subexpressions --- we can't reliably rebuild
-  // in-place because of expression sharing.
-  if (auto *ice = dyn_cast<ImplicitCastExpr>(e->IgnoreParens()))
-    if (ice->getCastKind() == CK_ARCReclaimReturnedObject)
-      return ice->getSubExpr();
+  Expr *curExpr = e, *prevExpr = nullptr;
+
+  // Walk down the expression until we hit an implicit cast of kind
+  // ARCReclaimReturnedObject or an Expr that is neither a Paren nor a Cast.
+  while (true) {
+    if (auto *pe = dyn_cast<ParenExpr>(curExpr)) {
+      prevExpr = curExpr;
+      curExpr = pe->getSubExpr();
+      continue;
+    }
+
+    if (auto *ce = dyn_cast<CastExpr>(curExpr)) {
+      if (auto *ice = dyn_cast<ImplicitCastExpr>(ce))
+        if (ice->getCastKind() == CK_ARCReclaimReturnedObject) {
+          if (!prevExpr)
+            return ice->getSubExpr();
+          if (auto *pe = dyn_cast<ParenExpr>(prevExpr))
+            pe->setSubExpr(ice->getSubExpr());
+          else
+            cast<CastExpr>(prevExpr)->setSubExpr(ice->getSubExpr());
+          return e;
+        }
+
+      prevExpr = curExpr;
+      curExpr = ce->getSubExpr();
+      continue;
+    }
+
+    // Break out of the loop if curExpr is neither a Paren nor a Cast.
+    break;
+  }
 
   return e;
 }


Index: test/CodeGenObjC/arc-bridged-cast.m
===================================================================
--- test/CodeGenObjC/arc-bridged-cast.m
+++ test/CodeGenObjC/arc-bridged-cast.m
@@ -102,5 +102,6 @@
   // CHECK-NOT: call i8* @objc_retainAutoreleasedReturnValue(
   // CHECK-NOT: call void @objc_release(
   CFStringRef r = (__bridge CFStringRef)(CreateNSString());
+  r = (__bridge CFStringRef)((NSString *)(CreateNSString()));
   return r;
 }
Index: lib/Sema/SemaExprObjC.cpp
===================================================================
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -4317,14 +4317,37 @@
 
 /// Look for an ObjCReclaimReturnedObject cast and destroy it.
 static Expr *maybeUndoReclaimObject(Expr *e) {
-  // For now, we just undo operands that are *immediately* reclaim
-  // expressions, which prevents the vast majority of potential
-  // problems here.  To catch them all, we'd need to rebuild arbitrary
-  // value-propagating subexpressions --- we can't reliably rebuild
-  // in-place because of expression sharing.
-  if (auto *ice = dyn_cast<ImplicitCastExpr>(e->IgnoreParens()))
-    if (ice->getCastKind() == CK_ARCReclaimReturnedObject)
-      return ice->getSubExpr();
+  Expr *curExpr = e, *prevExpr = nullptr;
+
+  // Walk down the expression until we hit an implicit cast of kind
+  // ARCReclaimReturnedObject or an Expr that is neither a Paren nor a Cast.
+  while (true) {
+    if (auto *pe = dyn_cast<ParenExpr>(curExpr)) {
+      prevExpr = curExpr;
+      curExpr = pe->getSubExpr();
+      continue;
+    }
+
+    if (auto *ce = dyn_cast<CastExpr>(curExpr)) {
+      if (auto *ice = dyn_cast<ImplicitCastExpr>(ce))
+        if (ice->getCastKind() == CK_ARCReclaimReturnedObject) {
+          if (!prevExpr)
+            return ice->getSubExpr();
+          if (auto *pe = dyn_cast<ParenExpr>(prevExpr))
+            pe->setSubExpr(ice->getSubExpr());
+          else
+            cast<CastExpr>(prevExpr)->setSubExpr(ice->getSubExpr());
+          return e;
+        }
+
+      prevExpr = curExpr;
+      curExpr = ce->getSubExpr();
+      continue;
+    }
+
+    // Break out of the loop if curExpr is neither a Paren nor a Cast.
+    break;
+  }
 
   return e;
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to