Merge pull request #65211 from calda/cal--weak-self-capture-inner-func

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

Author: xedin

lib/AST/UnqualifiedLookup.cpp

@@ -401,19 +401,65 @@ 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()) {
+          isSelfRebinding = subPattern->getBoundName() == Ctx.Id_self;
+        }
+      }
+
+      if (!isSelfRebinding) {
         continue;
       }
     }
 
-    if (auto selfDRE = dyn_cast<DeclRefExpr>(cond.getInitializer())) {
-      return (selfDRE->getDecl()->getName().isSimpleName(Ctx.Id_self));
+    Expr *exprToCheckForDRE = cond.getInitializer();
+    if (auto LE = dyn_cast<LoadExpr>(exprToCheckForDRE)) {
+      if (auto subexpr = LE->getSubExpr()) {
+        exprToCheckForDRE = subexpr;
+      }
     }
+
+    exprToCheckForDRE = exprToCheckForDRE->getSemanticsProvidingExpr();
+
+    DeclRefExpr *condDRE = dyn_cast<DeclRefExpr>(exprToCheckForDRE);
+    if (!condDRE || !condDRE->getDecl()->hasName()) {
+      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.
+ClosureExpr *closestParentClosure(DeclContext *dc) {
+  if (!dc) {
+    return nullptr;
+  }
+
+  if (auto closure = dyn_cast<ClosureExpr>(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 +471,7 @@ 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 closureExpr = closestParentClosure(factory->DC);
   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,160 @@ 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()
+      }
+      
+      subscript(index: Int) -> Int { // expected-error {{subscript' functions may only be declared within a type}}
+        method()
+        return index
+      }
+    }
+    
+    doVoidStuffNonEscaping { [weak self] in
+      func innerFunction1() {
+          method()
+          self?.method()
+      }
+      
+      guard let self else { return }
+      
+      func innerFunction2() {
+          method()
+          self.method()
+      }
+      
+      subscript(index: Int) -> Int { // expected-error {{subscript' functions may only be declared within a type}}
+        method()
+        return index
+      }
+    }
+    
+    doVoidStuff { [weak self] in
+      guard let self else { return }
+      
+      func innerFunction1() {
+        doVoidStuff { // expected-note {{capture 'self' explicitly to enable implicit 'self' in this closure}}
+          method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note {{reference 'self.' explicitly}}
+        }
+        
+        // This example should probably compile without an error -- seems like a bug in the impl of SE-0269
+        doVoidStuff { [self] in // expected-note {{variable other than 'self' captured here under the name 'self' does not enable implicit 'self'}}
+          method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          self.method()
+        }
+        
+        doVoidStuff { [weak self] in
+          method() // expected-error {{call to method 'method' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          self?.method()
+        }
+        
+        doVoidStuff { [weak self] in
+          guard let self else { return }
+          method()
+          
+          func innerMethod3() {
+            method()
+            self.method()
+          }
+        }
+      }
+    }
+  }
+}
+
+class NoImplicitSelfInInnerClass {
+  func method() { }
+  
+  private init() { // expected-note {{'self' declared here}} expected-note {{'self' declared here}} expected-note {{'self' declared here}} expected-note {{'self' declared here}} expected-note {{'self' declared here}} expected-note {{'self' declared here}} expected-note {{'self' declared here}}
+    doVoidStuff {
+      class InnerType { // expected-note {{type declared here}} expected-note {{type declared here}} expected-note {{type declared here}}
+        init() {
+          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'}}
+        }
+        
+        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'}}
+        }
+        
+        subscript(index: Int) -> Int {
+          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'}}
+          return index
+        }
+      }
+    }
+    
+    doVoidStuff { [weak self] in
+      guard let self else { return }
+      method()
+      
+      class InnerType { // expected-note {{type declared here}} expected-note {{type declared here}} expected-note {{type declared here}}
+        func methodOnInnerType() { }
+        
+        init() {
+          methodOnInnerType()
+          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'}}
+        }
+        
+        func functionInsideInnerType() {
+          methodOnInnerType()
+          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'}}
+        }
+        
+        subscript(index: Int) -> Int {
+          methodOnInnerType()
+          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'}}
+          return index
+        }
+      }
+    }
+    
+    doVoidStuff { [weak self] in
+      guard let self else { return }
+      
+      func innerMethod() {
+        method()
+        
+        class InnerType { // expected-note {{type declared here}}
+          func methodOnInnerType() { }
+          
+          init() {
+            methodOnInnerType()
+            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 }
+              self.method() // expected-error {{value of type 'InnerType' has no member 'method'}}
+              methodOnInnerType()
+            }
+            
+            doVoidStuff { [weak self] in
+              guard let self else { return }
+              method() // expected-error {{value of type 'InnerType' has no member 'method'}}
+              methodOnInnerType()
+            }
+          }
+        }
+      }
+    }
+    
   }
 }
 

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'}}
   }
-}
+}