Allow implicit self in inner functions in [weak self] closures, like with [self] closures

Author: calda

lib/AST/UnqualifiedLookup.cpp

@@ -401,19 +401,68 @@ bool implicitSelfReferenceIsUnwrapped(const ValueDecl *selfDecl,
   // and check that both its LHS and RHS are 'self'
   for (auto cond : conditionalStmt->getCond()) {
     if (auto pattern = cond.getPattern()) {
-      if (pattern->getBoundName() != Ctx.Id_self) {
+      bool isSelfRebinding = false;
+
+      if (pattern->getBoundName() == Ctx.Id_self) {
+        isSelfRebinding = true;
+      }
+
+      else if (auto OSP = dyn_cast<OptionalSomePattern>(pattern)) {
+        if (auto subPattern = OSP->getSubPattern()) {
+          if (subPattern->getBoundName() == Ctx.Id_self) {
+            isSelfRebinding = true;
+          }
+        }
+      }
+
+      if (!isSelfRebinding) {
         continue;
       }
     }
 
-    if (auto selfDRE = dyn_cast<DeclRefExpr>(cond.getInitializer())) {
-      return (selfDRE->getDecl()->getName().isSimpleName(Ctx.Id_self));
+    DeclRefExpr *condDRE = nullptr;
+    if (auto DRE = dyn_cast<DeclRefExpr>(cond.getInitializer())) {
+      condDRE = DRE;
+    }
+
+    if (auto LE = dyn_cast<LoadExpr>(cond.getInitializer())) {
+      if (auto DRE = dyn_cast_or_null<DeclRefExpr>(LE->getSubExpr())) {
+        condDRE = DRE;
+      }
+    }
+
+    if (!condDRE) {
+      return false;
     }
+
+    return condDRE->getDecl()->getName().isSimpleName(Ctx.Id_self);
   }
 
   return false;
 }
 
+// Finds the nearest parent closure, which would define the
+// permitted usage of implicit self. In closures this is most
+// often just `dc` itself, but in functions defined in the
+// closure body this would be some parent context.
+AbstractClosureExpr *closestParentClosure(DeclContext *dc) {
+  if (!dc) {
+    return nullptr;
+  }
+
+  if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
+    return closure;
+  }
+
+  // Stop searching if we find a type decl, since types always
+  // redefine what 'self' means, even when nested inside a closure.
+  if (dc->getContextKind() == DeclContextKind::GenericTypeDecl) {
+    return nullptr;
+  }
+
+  return closestParentClosure(dc->getParent());
+}
+
 ValueDecl *UnqualifiedLookupFactory::ResultFinderForTypeContext::lookupBaseDecl(
     const DeclContext *baseDC) const {
   // Perform an unqualified lookup for the base decl of this result. This
@@ -425,7 +474,8 @@ ValueDecl *UnqualifiedLookupFactory::ResultFinderForTypeContext::lookupBaseDecl(
   // self _always_ refers to the context's self `ParamDecl`, even if there
   // is another local decl with the name `self` that would be found by
   // `lookupSingleLocalDecl`.
-  auto closureExpr = dyn_cast<ClosureExpr>(factory->DC);
+  auto parentACE = closestParentClosure(factory->DC);
+  auto closureExpr = dyn_cast_or_null<ClosureExpr>(parentACE);
   if (!closureExpr) {
     return nullptr;
   }

test/expr/closure/closures.swift

@@ -616,6 +616,22 @@ class C_56501 {
       }
     }
   }
+  
+  func test7() {
+    doVoidStuff { [self] in
+      func innerFunction() {
+        operation()
+      }
+    }
+  }
+  
+  func test8() {
+    doVoidStuffNonEscaping { [self] in
+      func innerFunction() {
+        operation()
+      }
+    }
+  }
 }
 
 // https://github.com/apple/swift/issues/57029
@@ -812,6 +828,61 @@ public class TestImplicitSelfForWeakSelfCapture {
       guard let self = self ?? TestImplicitSelfForWeakSelfCapture.staticOptional else { return } // expected-warning {{value 'self' was defined but never used; consider replacing with boolean test}}
       method() // expected-warning {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}
     }
+    
+    doVoidStuff { [weak self] in
+      func innerFunction1() {
+          method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          self?.method()
+      }
+      
+      guard let self else { return }
+      
+      func innerFunction2() {
+          method()
+          self.method()
+      }
+    }
+    
+    doVoidStuffNonEscaping { [weak self] in
+      func innerFunction1() {
+          method()
+          self?.method()
+      }
+      
+      guard let self else { return }
+      
+      func innerFunction2() {
+          method()
+          self.method()
+      }
+    }
+  }
+}
+
+class NoImplicitSelfInInnerClass {
+  func method() { }
+  
+  private init() { // expected-note {{'self' declared here}} expected-note {{'self' declared here}}
+    doVoidStuff {
+      class InnerType { // expected-note {{type declared here}}
+          func functionInsideInnerType() {
+            method() // expected-error {{class declaration cannot close over value 'self' defined in outer scope}}
+            self.method() // expected-error {{value of type 'InnerType' has no member 'method'}}
+          }
+      }
+    }
+    
+    doVoidStuff { [weak self] in
+      guard let self else { return }
+      method()
+      
+      class InnerType { // expected-note {{type declared here}}
+          func functionInsideInnerType() {
+            method() // expected-error {{class declaration cannot close over value 'self' defined in outer scope}}
+            self.method() // expected-error {{value of type 'InnerType' has no member 'method'}}
+          }
+      }
+    }
   }
 }
 

test/expr/closure/closures_swift6.swift

@@ -154,6 +154,61 @@ public class TestImplicitSelfForWeakSelfCapture {
       guard let self = self ?? TestImplicitSelfForWeakSelfCapture.staticOptional else { return }
       method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}
     }
+    
+    doVoidStuff { [weak self] in
+      func innerFunction1() {
+          method() // expected-error {{explicit use of 'self' is required when 'self' is optional, to make control flow explicit}} expected-note {{reference 'self?.' explicitly}}
+          self?.method()
+      }
+      
+      guard let self else { return }
+      
+      func innerFunction2() {
+          method()
+          self.method()
+      }
+    }
+    
+    doVoidStuffNonEscaping { [weak self] in
+      func innerFunction1() {
+          method() // expected-error {{explicit use of 'self' is required when 'self' is optional, to make control flow explicit}} expected-note {{reference 'self?.' explicitly}}
+          self?.method()
+      }
+      
+      guard let self else { return }
+      
+      func innerFunction2() {
+          method()
+          self.method()
+      }
+    }
+  }
+}
+
+class NoImplicitSelfInInnerClass {
+  func method() { }
+  
+  private init() { // expected-note {{'self' declared here}} expected-note {{'self' declared here}}
+    doVoidStuff {
+      class InnerType { // expected-note {{type declared here}}
+          func functionInsideInnerType() {
+            method() // expected-error {{class declaration cannot close over value 'self' defined in outer scope}}
+            self.method() // expected-error {{value of type 'InnerType' has no member 'method'}}
+          }
+      }
+    }
+    
+    doVoidStuff { [weak self] in
+      guard let self else { return }
+      method()
+      
+      class InnerType { // expected-note {{type declared here}}
+          func functionInsideInnerType() {
+            method() // expected-error {{class declaration cannot close over value 'self' defined in outer scope}}
+            self.method() // expected-error {{value of type 'InnerType' has no member 'method'}}
+          }
+      }
+    }
   }
 }
 
@@ -186,4 +241,4 @@ public class TestRebindingSelfIsDisallowed {
     let `self` = "self shouldn't become a string"
     let _: Int = count // expected-error{{cannot convert value of type 'Void' to specified type 'Int'}}
   }
-}
+}