diff options
-rw-r--r-- | clang/include/clang/AST/DeclBase.h | 15 | ||||
-rw-r--r-- | clang/include/clang/Sema/Sema.h | 5 | ||||
-rw-r--r-- | clang/lib/Sema/SemaDecl.cpp | 31 | ||||
-rw-r--r-- | clang/lib/Sema/SemaDeclObjC.cpp | 1 | ||||
-rw-r--r-- | clang/lib/Sema/SemaExpr.cpp | 8 | ||||
-rw-r--r-- | clang/test/SemaObjC/warn-implicit-self-in-block.m | 18 | ||||
-rw-r--r-- | clang/test/SemaObjCXX/warn-implicit-self-in-block.mm | 42 |
7 files changed, 97 insertions, 23 deletions
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 4b63e540d5d..b3581778937 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -41,6 +41,7 @@ namespace clang { class ASTContext; class ASTMutationListener; class Attr; +class BlockDecl; class DeclContext; class ExternalSourceSymbolAttr; class FunctionDecl; @@ -1792,6 +1793,20 @@ public: bool isClosure() const { return getDeclKind() == Decl::Block; } + /// Return this DeclContext if it is a BlockDecl. Otherwise, return the + /// innermost enclosing BlockDecl or null if there are no enclosing blocks. + const BlockDecl *getInnermostBlockDecl() const { + const DeclContext *Ctx = this; + + do { + if (Ctx->isClosure()) + return cast<BlockDecl>(Ctx); + Ctx = Ctx->getParent(); + } while (Ctx); + + return nullptr; + } + bool isObjCContainer() const { switch (getDeclKind()) { case Decl::ObjCCategory: diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 351b6aabb18..8582872d00f 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1213,6 +1213,11 @@ public: /// of -Wselector. llvm::MapVector<Selector, SourceLocation> ReferencedSelectors; + /// List of SourceLocations where 'self' is implicitly retained inside a + /// block. + llvm::SmallVector<std::pair<SourceLocation, const BlockDecl *>, 1> + ImplicitlyRetainedSelfLocs; + /// Kinds of C++ special members. enum CXXSpecialMember { CXXDefaultConstructor, diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index fbe258bb1d5..8e0badf444a 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -13150,6 +13150,35 @@ private: bool IsLambda = false; }; +static void diagnoseImplicitlyRetainedSelf(Sema &S) { + llvm::DenseMap<const BlockDecl *, bool> EscapeInfo; + + auto IsOrNestedInEscapingBlock = [&](const BlockDecl *BD) { + if (EscapeInfo.count(BD)) + return EscapeInfo[BD]; + + bool R = false; + const BlockDecl *CurBD = BD; + + do { + R = !CurBD->doesNotEscape(); + if (R) + break; + CurBD = CurBD->getParent()->getInnermostBlockDecl(); + } while (CurBD); + + return EscapeInfo[BD] = R; + }; + + // If the location where 'self' is implicitly retained is inside a escaping + // block, emit a diagnostic. + for (const std::pair<SourceLocation, const BlockDecl *> &P : + S.ImplicitlyRetainedSelfLocs) + if (IsOrNestedInEscapingBlock(P.second)) + S.Diag(P.first, diag::warn_implicitly_retains_self) + << FixItHint::CreateInsertion(P.first, "self->"); +} + Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, bool IsInstantiation) { FunctionDecl *FD = dcl ? dcl->getAsFunction() : nullptr; @@ -13361,6 +13390,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, diag::warn_objc_secondary_init_missing_init_call); getCurFunction()->ObjCWarnForNoInitDelegation = false; } + + diagnoseImplicitlyRetainedSelf(*this); } else { // Parsing the function declaration failed in some way. Pop the fake scope // we pushed on. diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp index d0674c641a7..430c2a2a183 100644 --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/clang/lib/Sema/SemaDeclObjC.cpp @@ -359,6 +359,7 @@ HasExplicitOwnershipAttr(Sema &S, ParmVarDecl *Param) { /// ActOnStartOfObjCMethodDef - This routine sets up parameters; invisible /// and user declared, in the method definition's AST. void Sema::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, Decl *D) { + ImplicitlyRetainedSelfLocs.clear(); assert((getCurMethodDecl() == nullptr) && "Methodparsing confused"); ObjCMethodDecl *MDecl = dyn_cast_or_null<ObjCMethodDecl>(D); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index ef390928217..e64eeecb2a4 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -2575,11 +2575,9 @@ Sema::LookupInObjCMethod(LookupResult &Lookup, Scope *S, !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->"); - } + if (getLangOpts().ObjCAutoRefCount) + if (const BlockDecl *BD = CurContext->getInnermostBlockDecl()) + ImplicitlyRetainedSelfLocs.push_back({Loc, BD}); return Result; } diff --git a/clang/test/SemaObjC/warn-implicit-self-in-block.m b/clang/test/SemaObjC/warn-implicit-self-in-block.m deleted file mode 100644 index a7ee16ec700..00000000000 --- a/clang/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 diff --git a/clang/test/SemaObjCXX/warn-implicit-self-in-block.mm b/clang/test/SemaObjCXX/warn-implicit-self-in-block.mm new file mode 100644 index 00000000000..4842b4b10ba --- /dev/null +++ b/clang/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 |