[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@358624 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 4b63e540d5d489d9b50206c198a84f88ee2b722f..b35817789377885cfe653dd5b0f30eb0db5389b2 100644 (file)
--- a/include/clang/AST/DeclBase.h
+++ b/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/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 351b6aabb18770998990144718cd3a706eb56ebb..8582872d00f9a0b3d8fdbb0a481fe7c9bac4e0ed 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/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/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index fbe258bb1d5dbb9a3bc99038e27a70348e042db3..8e0badf444add705373cd9eb0ebe7c8661abe271 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/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/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp
index d0674c641a7ec1f8dbdacd7f4cc345bf2170e69b..430c2a2a183f3423413dd5a04f8a836796456269 100644 (file)
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/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/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index ef39092821755ae49a1039eaeb3c92a024351b31..e64eeecb2a4f3dbd80eea72113c8f9aa1f71287b 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/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/test/SemaObjC/warn-implicit-self-in-block.m b/test/SemaObjC/warn-implicit-self-in-block.m
deleted file mode 100644 (file)
index a7ee16e..0000000
--- a/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/test/SemaObjCXX/warn-implicit-self-in-block.mm b/test/SemaObjCXX/warn-implicit-self-in-block.mm
new file mode 100644 (file)
index 0000000..4842b4b
--- /dev/null
+++ b/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