From: Jordan Rose Date: Sat, 15 Sep 2012 02:48:31 +0000 (+0000) Subject: -Warc-retain-cycles: warn at variable initialization as well as assignment. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e10f4d384f5ae113d72a5193a1332c1930635ccc;p=clang -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.) 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 3c4271a853..aec76b1848 100644 --- 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 b46081dc99..0e6b5ec9a3 100644 --- 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 134b965a33..edcfcb68e1 100644 --- 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()) + 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 3d143c8097..97591be98e 100644 --- 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(InnerLHS); + if (!DRE || DRE->getDecl()->hasAttr()) + 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 44d450a310..1a13fe3726 100644 --- 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}} + }; +} +