[cfe-commits] r163962 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp test/SemaObjC/warn-retain-cycle.m

[Jordan Rose]

Author: jrose
Date: Fri Sep 14 21:48:31 2012
New Revision: 163962

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

Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/SemaObjC/warn-retain-cycle.m

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=163962&r1=163961&r2=163962&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Sep 14 21:48:31 2012
@@ -6784,6 +6784,7 @@
   /// 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.

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=163962&r1=163961&r2=163962&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Sep 14 21:48:31 2012
@@ -5299,7 +5299,8 @@
     return false;
 
   owner.Variable = var;
-  owner.setLocsFrom(ref);
+  if (ref)
+    owner.setLocsFrom(ref);
   return true;
 }
 
@@ -5499,6 +5500,20 @@
     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();

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=163962&r1=163961&r2=163962&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Sep 14 21:48:31 2012
@@ -6582,9 +6582,13 @@
   // 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);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=163962&r1=163961&r2=163962&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep 14 21:48:31 2012
@@ -7698,10 +7698,18 @@
     }
 
     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"

Modified: cfe/trunk/test/SemaObjC/warn-retain-cycle.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/warn-retain-cycle.m?rev=163962&r1=163961&r2=163962&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/warn-retain-cycle.m (original)
+++ cfe/trunk/test/SemaObjC/warn-retain-cycle.m Fri Sep 14 21:48:31 2012
@@ -127,3 +127,29 @@
 }
 @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}}
+  };
+}
+