Merge pull request #35898 from rjmccall/diagnose-implicit-nested-self

Diagnose the implicit use of self in nested closures

Author: rjmccall

lib/Sema/MiscDiagnostics.cpp

@@ -1461,14 +1461,31 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
             Closures.push_back(ACE);
         }
 
+    static bool isEnclosingSelfReference(VarDecl *var,
+                                   const AbstractClosureExpr *inClosure) {
+      if (var->isSelfParameter())
+        return true;
+
+      // Capture variables have a DC of the parent function.
+      if (inClosure && var->isSelfParamCapture() &&
+          var->getDeclContext() != inClosure->getParent())
+        return true;
+
+      return false;
+    }
+
     /// Return true if this is an implicit reference to self which is required
     /// to be explicit in an escaping closure. Metatype references and value
     /// type references are excluded.
-    static bool isImplicitSelfParamUseLikelyToCauseCycle(Expr *E) {
+    static bool isImplicitSelfParamUseLikelyToCauseCycle(Expr *E,
+                                   const AbstractClosureExpr *inClosure) {
       auto *DRE = dyn_cast<DeclRefExpr>(E);
 
-      if (!DRE || !DRE->isImplicit() || !isa<VarDecl>(DRE->getDecl()) ||
-          !cast<VarDecl>(DRE->getDecl())->isSelfParameter())
+      if (!DRE || !DRE->isImplicit())
+        return false;
+
+      auto var = dyn_cast<VarDecl>(DRE->getDecl());
+      if (!var || !isEnclosingSelfReference(var, inClosure))
         return false;
 
       // Defensive check for type. If the expression doesn't have type here, it
@@ -1544,7 +1561,7 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
 
       SourceLoc memberLoc = SourceLoc();
       if (auto *MRE = dyn_cast<MemberRefExpr>(E))
-        if (isImplicitSelfParamUseLikelyToCauseCycle(MRE->getBase())) {
+        if (isImplicitSelfParamUseLikelyToCauseCycle(MRE->getBase(), ACE)) {
           auto baseName = MRE->getMember().getDecl()->getBaseName();
           memberLoc = MRE->getLoc();
           Diags.diagnose(memberLoc,
@@ -1554,7 +1571,7 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
 
       // Handle method calls with a specific diagnostic + fixit.
       if (auto *DSCE = dyn_cast<DotSyntaxCallExpr>(E))
-        if (isImplicitSelfParamUseLikelyToCauseCycle(DSCE->getBase()) &&
+        if (isImplicitSelfParamUseLikelyToCauseCycle(DSCE->getBase(), ACE) &&
             isa<DeclRefExpr>(DSCE->getFn())) {
           auto MethodExpr = cast<DeclRefExpr>(DSCE->getFn());
           memberLoc = DSCE->getLoc();
@@ -1569,7 +1586,7 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
       }
 
       // Catch any other implicit uses of self with a generic diagnostic.
-      if (isImplicitSelfParamUseLikelyToCauseCycle(E))
+      if (isImplicitSelfParamUseLikelyToCauseCycle(E, ACE))
         Diags.diagnose(E->getLoc(), diag::implicit_use_of_self_in_closure);
 
       return { true, E };

test/expr/closure/closures.swift

@@ -151,6 +151,7 @@ func anonymousClosureArgsInClosureWithArgs() {
 
 func doStuff(_ fn : @escaping () -> Int) {}
 func doVoidStuff(_ fn : @escaping () -> ()) {}
+func doVoidStuffNonEscaping(_ fn: () -> ()) {}
 
 // <rdar://problem/16193162> Require specifying self for locations in code where strong reference cycles are likely
 class ExplicitSelfRequiredTest {
@@ -528,3 +529,63 @@ class SR3186 {
     print("\(v)")
   }
 }
+
+// Apply the explicit 'self' rule even if it referrs to a capture, if
+// we're inside a nested closure
+class SR14120 {
+  func operation() {}
+
+  func test1() {
+    doVoidStuff { [self] in
+      operation()
+    }
+  }
+
+  func test2() {
+    doVoidStuff { [self] in
+      doVoidStuff {
+        // expected-error@+3 {{call to method 'operation' in closure requires explicit use of 'self'}}
+        // expected-note@-2 {{capture 'self' explicitly to enable implicit 'self' in this closure}}
+        // expected-note@+1 {{reference 'self.' explicitly}}
+        operation()
+      }
+    }
+  }
+
+  func test3() {
+    doVoidStuff { [self] in
+      doVoidStuff { [self] in
+        operation()
+      }
+    }
+  }
+
+  func test4() {
+    doVoidStuff { [self] in
+      doVoidStuff {
+        self.operation()
+      }
+    }
+  }
+
+  func test5() {
+    doVoidStuff { [self] in
+      doVoidStuffNonEscaping {
+        operation()
+      }
+    }
+  }
+
+  func test6() {
+    doVoidStuff { [self] in
+      doVoidStuff { [self] in
+        doVoidStuff {
+          // expected-error@+3 {{call to method 'operation' in closure requires explicit use of 'self'}}
+          // expected-note@-2 {{capture 'self' explicitly to enable implicit 'self' in this closure}}
+          // expected-note@+1 {{reference 'self.' explicitly}}
+          operation()
+        }
+      }
+    }
+  }
+}