Index: include/clang/Sema/ScopeInfo.h =================================================================== --- include/clang/Sema/ScopeInfo.h +++ include/clang/Sema/ScopeInfo.h @@ -151,6 +151,9 @@ /// false if there is an invocation of an initializer on 'self'. bool ObjCWarnForNoInitDelegation : 1; + /// A flag indicating the scope is an ObjC method definition. + bool IsObjCMethod : 1; + /// True only when this function has not already built, or attempted /// to build, the initial and final coroutine suspend points bool NeedsCoroutineSuspends : 1; @@ -202,8 +205,23 @@ /// function. SmallVector CompoundScopes; - /// The set of blocks that are introduced in this function. - llvm::SmallPtrSet Blocks; + /// The list of blocks that are directly or indirectly introduced in this + /// function and aren't enclosed in another block. The boolean flag indicates + /// whether the block is introduced in the current function scope. For + /// example, when function 'foo1' is parsed in the code below, the flag is + /// false for the first block and is true for the second block. The block that + /// calls 'foo2(2)' isn't added to the list since it is nested inside another + /// block. + /// + /// void foo1() { + /// [](){ ^{ foo2(0); }; }(); + /// ^{ foo2(1); }(); + /// ^{ ^{ foo2(2); }(); }(); + /// } + /// + /// The blocks are stored to the list in the order they appear in the + /// function. + llvm::SmallVector, 1> Blocks; /// The set of __block variables that are introduced in this function. llvm::TinyPtrVector ByrefBlockVars; @@ -432,9 +450,17 @@ (HasBranchProtectedScope && HasBranchIntoScope)); } - // Add a block introduced in this function. - void addBlock(const BlockDecl *BD) { - Blocks.insert(BD); + bool isObjCMethod() const { + return IsObjCMethod; + } + + void setIsObjCMethod() { + IsObjCMethod = true; + } + + // Add a block to the list. + void addBlock(const BlockDecl *BD, bool IsIntroducedInCurrentScope) { + Blocks.push_back({BD, IsIntroducedInCurrentScope}); } // Add a __block variable introduced in this function. @@ -442,6 +468,8 @@ ByrefBlockVars.push_back(VD); } + bool isBlockScope() const { return Kind == SK_Block; } + bool isCoroutine() const { return !FirstCoroutineStmtLoc.isInvalid(); } void setFirstCoroutineStmt(SourceLocation Loc, StringRef Keyword) { Index: lib/Sema/Sema.cpp =================================================================== --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -20,6 +20,7 @@ #include "clang/AST/ExprCXX.h" #include "clang/AST/PrettyDeclStackTrace.h" #include "clang/AST/StmtCXX.h" +#include "clang/AST/StmtVisitor.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/TargetInfo.h" @@ -1636,10 +1637,16 @@ static void markEscapingByrefs(const FunctionScopeInfo &FSI, Sema &S) { // Set the EscapingByref flag of __block variables captured by // escaping blocks. - for (const BlockDecl *BD : FSI.Blocks) { - if (BD->doesNotEscape()) + for (const std::pair &P : FSI.Blocks) { + // Skip blocks that aren't directly introduced in this function. We've + // already seen them. + if (!P.second) continue; - for (const BlockDecl::Capture &BC : BD->captures()) { + + if (P.first->doesNotEscape()) + continue; + + for (const BlockDecl::Capture &BC : P.first->captures()) { VarDecl *VD = BC.getVariable(); if (VD->hasAttr()) VD->setEscapingByref(); @@ -1659,6 +1666,54 @@ } } +namespace { + +class DiagImplicitRetainedSelf + : public ConstStmtVisitor { + Sema &S; + + // A flag indicating whether we are visiting a Stmt that is inside a block + // that can possibly escape. + bool VisitingEscapingBlock = false; + +public: + DiagImplicitRetainedSelf(Sema &S) : S(S) {} + + void VisitBlockDecl(const BlockDecl *BD) { + bool OldVisitingEscapingBlock = VisitingEscapingBlock; + VisitingEscapingBlock = VisitingEscapingBlock || !BD->doesNotEscape(); + Visit(BD->getBody()); + VisitingEscapingBlock = OldVisitingEscapingBlock; + } + + void VisitBlockExpr(const BlockExpr *BE) { + VisitBlockDecl(BE->getBlockDecl()); + } + + void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *ORE) { + if (!VisitingEscapingBlock || !ORE->isFreeIvar()) + return; + SourceLocation Loc = ORE->getExprLoc(); + S.Diag(Loc, diag::warn_implicitly_retains_self) + << FixItHint::CreateInsertion(Loc, "self->"); + } + + void VisitStmt(const Stmt *S) { + for (const Stmt *SubStmt : S->children()) + if (SubStmt) + Visit(SubStmt); + } +}; + +} // namespace + +static void diagnoseImplicitlyRetainedSelf(const FunctionScopeInfo &FSI, + Sema &S) { + DiagImplicitRetainedSelf D(S); + for (const std::pair &P : FSI.Blocks) + D.VisitBlockDecl(P.first); +} + void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, const Decl *D, const BlockExpr *blkExpr) { assert(!FunctionScopes.empty() && "mismatched push/pop!"); @@ -1671,6 +1726,16 @@ FunctionScopeInfo *Scope = FunctionScopes.pop_back_val(); + if (getLangOpts().ObjCAutoRefCount && Scope->isObjCMethod()) + diagnoseImplicitlyRetainedSelf(*Scope, *this); + + // If the current scope isn't a block, add the blocks in the list to the + // enclosing function's block list. + if (!Scope->isBlockScope() && !FunctionScopes.empty()) + for (std::pair &p : Scope->Blocks) + FunctionScopes.back()->addBlock( + p.first, /*IsIntroducedInCurrentScope*/false); + if (LangOpts.OpenMP) popOpenMPFunctionRegion(Scope); Index: lib/Sema/SemaDeclObjC.cpp =================================================================== --- lib/Sema/SemaDeclObjC.cpp +++ lib/Sema/SemaDeclObjC.cpp @@ -378,6 +378,7 @@ // Allow all of Sema to see that we are entering a method definition. PushDeclContext(FnBodyScope, MDecl); PushFunctionScope(); + getCurFunction()->setIsObjCMethod(); // Create Decl objects for each parameter, entrring them in the scope for // binding to their use. Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -2575,11 +2575,6 @@ !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, Loc)) getCurFunction()->recordUseOfWeak(Result); } - if (getLangOpts().ObjCAutoRefCount) { - if (CurContext->isClosure()) - Diag(Loc, diag::warn_implicitly_retains_self) - << FixItHint::CreateInsertion(Loc, "self->"); - } return Result; } @@ -13652,6 +13647,9 @@ } } + if (getCurFunction()) + getCurFunction()->addBlock(Block, /*IsIntroducedInCurrentScope*/true); + PushBlockScope(CurScope, Block); CurContext->addDecl(Block); if (CurScope) @@ -13913,9 +13911,6 @@ } } - if (getCurFunction()) - getCurFunction()->addBlock(BD); - return Result; } Index: test/SemaObjC/warn-implicit-self-in-block.m =================================================================== --- test/SemaObjC/warn-implicit-self-in-block.m +++ /dev/null @@ -1,18 +0,0 @@ -// RUN: %clang_cc1 -x objective-c -fobjc-arc -fblocks -Wimplicit-retain-self -verify %s -// rdar://11194874 - -@interface Root @end - -@interface I : Root -{ - int _bar; -} -@end - -@implementation I - - (void)foo{ - ^{ - _bar = 3; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}} - }(); - } -@end Index: test/SemaObjCXX/warn-implicit-self-in-block.mm =================================================================== --- /dev/null +++ test/SemaObjCXX/warn-implicit-self-in-block.mm @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -x objective-c++ -std=c++11 -fobjc-arc -fblocks -Wimplicit-retain-self -verify %s +// rdar://11194874 + +typedef void (^BlockTy)(); + +void noescapeFunc(__attribute__((noescape)) BlockTy); +void escapeFunc(BlockTy); + +@interface Root @end + +@interface I : Root +{ + int _bar; +} +@end + +@implementation I + - (void)foo{ + ^{ + _bar = 3; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}} + }(); + } + + - (void)testNested{ + noescapeFunc(^{ + (void)_bar; + escapeFunc(^{ + (void)_bar; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}} + noescapeFunc(^{ + (void)_bar; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}} + }); + (void)_bar; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}} + }); + (void)_bar; + }); + } + + - (void)testLambdaInBlock{ + noescapeFunc(^{ [&](){ (void)_bar; }(); }); + escapeFunc(^{ [&](){ (void)_bar; }(); }); // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}} + } +@end