-Warc-retain-cycles: warn at variable initialization as well as assignment.

Specifically, this should warn:

  __block block_t a = ^{ a(); };

Furthermore, this case which previously warned now does not, since the value
of 'b' is captured before the assignment occurs:

  block_t b; // not __block
  b = ^{ b(); };

(This will of course warn under -Wuninitialized, as before.)

<rdar://problem/11015883>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@163962 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 3c4271a8538b2e1bd4044e93a6fd9932f517882b..aec76b18484970a96e012883acd0918b32afebbe 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -6784,6 +6784,7 @@ public:
   /// might create an obvious retain cycle.
   void checkRetainCycles(ObjCMessageExpr *msg);
   void checkRetainCycles(Expr *receiver, Expr *argument);
+  void checkRetainCycles(VarDecl *Var, Expr *Init);
 
   /// checkUnsafeAssigns - Check whether +1 expr is being assigned
   /// to weak/__unsafe_unretained type.

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index b46081dc99ad8222a0605d500a909cce3a6deaf0..0e6b5ec9a39b1696380eb7ec4488b25f68397ed8 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -5299,7 +5299,8 @@ static bool considerVariable(VarDecl *var, Expr *ref, RetainCycleOwner &owner) {
     return false;
 
   owner.Variable = var;
-  owner.setLocsFrom(ref);
+  if (ref)
+    owner.setLocsFrom(ref);
   return true;
 }
 
@@ -5499,6 +5500,20 @@ void Sema::checkRetainCycles(Expr *receiver, Expr *argument) {
     diagnoseRetainCycle(*this, capturer, owner);
 }
 
+void Sema::checkRetainCycles(VarDecl *Var, Expr *Init) {
+  RetainCycleOwner Owner;
+  if (!considerVariable(Var, /*DeclRefExpr=*/0, Owner))
+    return;
+  
+  // Because we don't have an expression for the variable, we have to set the
+  // location explicitly here.
+  Owner.Loc = Var->getLocation();
+  Owner.Range = Var->getSourceRange();
+  
+  if (Expr *Capturer = findCapturingExpr(*this, Init, Owner))
+    diagnoseRetainCycle(*this, Capturer, Owner);
+}
+
 bool Sema::checkUnsafeAssigns(SourceLocation Loc,
                               QualType LHS, Expr *RHS) {
   Qualifiers::ObjCLifetime LT = LHS.getObjCLifetime();

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 134b965a33aa98f4ff094fe6962a22536ed32450..edcfcb68e1f73db2810fb8a18d2f8c5660356623 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -6582,9 +6582,13 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init,
   // Check any implicit conversions within the expression.
   CheckImplicitConversions(Init, VDecl->getLocation());
 
-  if (!VDecl->isInvalidDecl())
+  if (!VDecl->isInvalidDecl()) {
     checkUnsafeAssigns(VDecl->getLocation(), VDecl->getType(), Init);
 
+    if (VDecl->hasAttr<BlocksAttr>())
+      checkRetainCycles(VDecl, Init);
+  }
+
   Init = MaybeCreateExprWithCleanups(Init);
   // Attach the initializer to the decl.
   VDecl->setInit(Init);

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 3d143c8097fae77de8433cc9e27bf5a6d017412f..97591be98eb2971a0a97c1733a05edc919f5d918 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -7698,10 +7698,18 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
     }
 
     if (ConvTy == Compatible) {
-      if (LHSType.getObjCLifetime() == Qualifiers::OCL_Strong)
-        checkRetainCycles(LHSExpr, RHS.get());
-      else if (getLangOpts().ObjCAutoRefCount)
+      if (LHSType.getObjCLifetime() == Qualifiers::OCL_Strong) {
+        // Warn about retain cycles where a block captures the LHS, but
+        // not if the LHS is a simple variable into which the block is
+        // being stored...unless that variable can be captured by reference!
+        const Expr *InnerLHS = LHSExpr->IgnoreParenCasts();
+        const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(InnerLHS);
+        if (!DRE || DRE->getDecl()->hasAttr<BlocksAttr>())
+          checkRetainCycles(LHSExpr, RHS.get());
+
+      } else if (getLangOpts().ObjCAutoRefCount) {
         checkUnsafeExprAssigns(Loc, LHSExpr, RHS.get());
+      }
     }
   } else {
     // Compound assignment "x += y"

diff --git a/test/SemaObjC/warn-retain-cycle.m b/test/SemaObjC/warn-retain-cycle.m
index 44d450a3101c7f10641c0ba71df885b74d80bd6a..1a13fe37260ba0e81daef0770bb362ed5500b0ae 100644 (file)
--- a/test/SemaObjC/warn-retain-cycle.m
+++ b/test/SemaObjC/warn-retain-cycle.m
@@ -127,3 +127,29 @@ void doSomething(unsigned v);
 }
 @end
 
+
+void testBlockVariable() {
+  typedef void (^block_t)(void);
+  
+  // This case will be caught by -Wuninitialized, and does not create a
+  // retain cycle.
+  block_t a1 = ^{
+    a1(); // no-warning
+  };
+
+  // This case will also be caught by -Wuninitialized.
+  block_t a2;
+  a2 = ^{
+    a2(); // no-warning
+  };
+  
+  __block block_t b1 = ^{ // expected-note{{block will be retained by the captured object}}
+    b1(); // expected-warning{{capturing 'b1' strongly in this block is likely to lead to a retain cycle}}
+  };
+
+  __block block_t b2;
+  b2 = ^{ // expected-note{{block will be retained by the captured object}}
+    b2(); // expected-warning{{capturing 'b2' strongly in this block is likely to lead to a retain cycle}}
+  };
+}
+