ahatanak updated this revision to Diff 195498. ahatanak added a comment. Keep a list of pairs of BlockDecl and SourceLocation and, after the body of an ObjC method is parsed, emit a diagnostic if a SourceLocation in the list is inside an escaping block.
Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60736/new/ https://reviews.llvm.org/D60736 Files: include/clang/AST/DeclBase.h include/clang/Sema/Sema.h lib/Parse/ParseObjc.cpp lib/Sema/SemaDeclObjC.cpp lib/Sema/SemaExpr.cpp test/SemaObjC/warn-implicit-self-in-block.m test/SemaObjCXX/warn-implicit-self-in-block.mm
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 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: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -2575,11 +2575,10 @@ !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()) + if (!getDiagnostics().isIgnored(diag::warn_implicitly_retains_self, Loc)) + ImplicitlyRetainedSelfLocs.push_back({Loc, BD}); return Result; } Index: lib/Sema/SemaDeclObjC.cpp =================================================================== --- lib/Sema/SemaDeclObjC.cpp +++ lib/Sema/SemaDeclObjC.cpp @@ -359,6 +359,7 @@ /// 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); @@ -494,6 +495,35 @@ } } +void Sema::ActOnEndOfObjCMethodDef() { + 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 = 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 : + ImplicitlyRetainedSelfLocs) + if (IsOrNestedInEscapingBlock(P.second)) + Diag(P.first, diag::warn_implicitly_retains_self) + << FixItHint::CreateInsertion(P.first, "self->"); +} + namespace { // Callback to only accept typo corrections that are Objective-C classes. Index: lib/Parse/ParseObjc.cpp =================================================================== --- lib/Parse/ParseObjc.cpp +++ lib/Parse/ParseObjc.cpp @@ -3693,6 +3693,9 @@ while (Tok.getLocation() != OrigLoc && Tok.isNot(tok::eof)) ConsumeAnyToken(); } + + Actions.ActOnEndOfObjCMethodDef(); + // Clean up the remaining EOF token. ConsumeAnyToken(); } Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1213,6 +1213,11 @@ /// 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, @@ -2101,6 +2106,7 @@ Decl *ActOnStartOfFunctionDef(Scope *S, Decl *D, SkipBodyInfo *SkipBody = nullptr); void ActOnStartOfObjCMethodDef(Scope *S, Decl *D); + void ActOnEndOfObjCMethodDef(); bool isObjCMethodDecl(Decl *D) { return D && isa<ObjCMethodDecl>(D); } Index: include/clang/AST/DeclBase.h =================================================================== --- include/clang/AST/DeclBase.h +++ include/clang/AST/DeclBase.h @@ -41,6 +41,7 @@ class ASTContext; class ASTMutationListener; class Attr; +class BlockDecl; class DeclContext; class ExternalSourceSymbolAttr; class FunctionDecl; @@ -1792,6 +1793,20 @@ 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:
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits