Author: ahatanak Date: Wed Apr 17 16:14:44 2019 New Revision: 358624 URL: http://llvm.org/viewvc/llvm-project?rev=358624&view=rev Log: [Sema][ObjC] Don't warn about an implicitly retained self if the retaining block and all of the enclosing blocks are non-escaping.
If the block implicitly retaining self doesn't escape, there is no risk of creating retain cycles, so clang shouldn't diagnose it and force users to add self-> to silence the diagnostic. Also, fix a bug where clang was failing to diagnose an implicitly retained self inside a c++ lambda nested inside a block. rdar://problem/25059955 Differential Revision: https://reviews.llvm.org/D60736 Added: cfe/trunk/test/SemaObjCXX/warn-implicit-self-in-block.mm Removed: cfe/trunk/test/SemaObjC/warn-implicit-self-in-block.m Modified: cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaDeclObjC.cpp cfe/trunk/lib/Sema/SemaExpr.cpp Modified: cfe/trunk/include/clang/AST/DeclBase.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=358624&r1=358623&r2=358624&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/DeclBase.h (original) +++ cfe/trunk/include/clang/AST/DeclBase.h Wed Apr 17 16:14:44 2019 @@ -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: Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=358624&r1=358623&r2=358624&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Wed Apr 17 16:14:44 2019 @@ -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, Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=358624&r1=358623&r2=358624&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Apr 17 16:14:44 2019 @@ -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 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. Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=358624&r1=358623&r2=358624&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Wed Apr 17 16:14:44 2019 @@ -359,6 +359,7 @@ HasExplicitOwnershipAttr(Sema &S, ParmVa /// 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); Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=358624&r1=358623&r2=358624&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Apr 17 16:14:44 2019 @@ -2575,11 +2575,9 @@ Sema::LookupInObjCMethod(LookupResult &L !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; } Removed: cfe/trunk/test/SemaObjC/warn-implicit-self-in-block.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/warn-implicit-self-in-block.m?rev=358623&view=auto ============================================================================== --- cfe/trunk/test/SemaObjC/warn-implicit-self-in-block.m (original) +++ cfe/trunk/test/SemaObjC/warn-implicit-self-in-block.m (removed) @@ -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 Added: cfe/trunk/test/SemaObjCXX/warn-implicit-self-in-block.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/warn-implicit-self-in-block.mm?rev=358624&view=auto ============================================================================== --- cfe/trunk/test/SemaObjCXX/warn-implicit-self-in-block.mm (added) +++ cfe/trunk/test/SemaObjCXX/warn-implicit-self-in-block.mm Wed Apr 17 16:14:44 2019 @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
