r358624 - [Sema][ObjC] Don't warn about an implicitly retained self if the
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 17 16:14:44 PDT 2019
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
-
- at interface Root @end
-
- at interface I : Root
-{
- int _bar;
-}
- at end
-
- at implementation I
- - (void)foo{
- ^{
- _bar = 3; // expected-warning {{block implicitly retains 'self'; explicitly mention 'self' to indicate this is intended behavior}}
- }();
- }
- at 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);
+
+ at interface Root @end
+
+ at interface I : Root
+{
+ int _bar;
+}
+ at end
+
+ at 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}}
+ }
+ at end
More information about the cfe-commits
mailing list