Fix unexpected error by simplifying outermost capture validation

Author: calda

lib/Sema/MiscDiagnostics.cpp

@@ -2135,19 +2135,19 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
         if (!DRE) {
           return false;
         }
-
+        
         auto selfDecl = dyn_cast_or_null<VarDecl>(DRE->getDecl());
         auto ty = DRE->getType();
         if (!selfDecl || !ty) {
           return false;
         }
-
+        
         // Implicit self was always allowed in structs and metadata types
         // even if the self capture was invalid.
         if (!ty->hasReferenceSemantics() || ty->is<MetatypeType>()) {
           return true;
         }
-
+        
         // If this implicit self decl is from a closure that captured self
         // weakly, then we should always emit an error, since implicit self was
         // only allowed starting in Swift 5.8 and later.
@@ -2156,21 +2156,21 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
           // captures in non-escaping closures in Swift 5.7, so in that case we
           // can only warn until Swift 6.
           if (AnyFunctionRef(const_cast<AbstractClosureExpr *>(ACE))
-                  .isKnownNoEscape()) {
-            return true;
-          }
-
+              .isKnownNoEscape()) {
+                return true;
+              }
+          
           // If this is a capture like `[weak self = somethingElse]`,
           // this unintentionally permitted implicit self in Swift 5.8 - 5.10,
           // so we have to only warn until Swift 6.
           if (!isSimpleSelfCapture(weakSelfDecl)) {
             return true;
           }
-
+          
           if (auto conditionalStmt = parentConditionalStmt(selfDecl)) {
             auto isValidSelfRebinding =
-                hasValidSelfRebinding(conditionalStmt, ACE);
-
+            hasValidSelfRebinding(conditionalStmt, ACE);
+            
             if (isValidSelfRebinding) {
               // In Swift 5.8 - 5.10 we unintentionally permitted implicit
               // self without validating any parent closures. To preserve
@@ -2179,62 +2179,45 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
                 return true;
               }
             }
-
+            
             // In Swift 5.8 - 5.10 we used `requiresLoadExpr` instead
             // of `requiresCaptureListRef` to validate self unwrap
             // conditions. For source compatibility, if the legacy check
             // succeeds but the current check fails, we should only warn
             // until Swift 6.
             auto legacyRebindsSelfResult =
-                conditionalStmt->rebindsSelf(ACE->getASTContext(),
-                                             /*requiresCaptureListRef*/ false,
-                                             /*requiresLoadExpr*/ true);
-
+            conditionalStmt->rebindsSelf(ACE->getASTContext(),
+                                         /*requiresCaptureListRef*/ false,
+                                         /*requiresLoadExpr*/ true);
+            
             if (!isValidSelfRebinding && legacyRebindsSelfResult) {
               return true;
             }
           }
         }
-
+        
         // If the self decl refers to a weak self unwrapping
         // condition in some parent closure, then there is no
         // source-compatibility requirement to avoid an error.
         if (hasValidSelfRebinding(parentConditionalStmt(selfDecl), ACE)) {
           return false;
         }
-
-        // If implicit self was specifically disallowed because of the
-        // `implicitSelfDisallowedDueToInvalidParent` check, then there
-        // is no source-compatibility requirement to avoid an error,
-        // unless this is something like a nonescaping closure that
-        // didn't have this layer of validation in Swift 5 mode.
-        // This doesn't apply to autoclosures.
-        if (!isa<AutoClosureExpr>(ACE)) {
-          if (auto invalidParentClosure =
-                  parentClosureDisallowingImplicitSelf(selfDecl, ty, ACE)) {
-            auto invalidParentClosureExcludingOutermostCapture =
-                parentClosureDisallowingImplicitSelf(
-                    selfDecl, ty, ACE, /*validateOutermostCapture:*/ false);
-
-            // In Swift 5 mode we didn't validate the outermost capture in
-            // nonescaping closures, so in that case we only warn.
-            if (!isClosureRequiringSelfQualification(ACE, Ctx) &&
-                (invalidParentClosureExcludingOutermostCapture !=
-                 invalidParentClosure)) {
+        
+        // In Swift 5 mode we didn't validate the outermost capture in
+        // nonescaping closures, so if only the outermost capture is invalid
+        // then we must only warn.
+        if (!isClosureRequiringSelfQualification(ACE, Ctx)) {
+          auto invalidParentClosure = parentClosureDisallowingImplicitSelf(selfDecl, ty, ACE);
+          
+          if (invalidParentClosure) {
+            auto excludingOutermostCapture = parentClosureDisallowingImplicitSelf(selfDecl, ty, ACE, /*validateOutermostCapture:*/ false);
+            
+            if (invalidParentClosure != excludingOutermostCapture) {
               return true;
             }
-
-            return false;
-          }
-        }
-
-        bool closureHasExplicitSelfCapture = false;
-        if (auto closureExpr = dyn_cast<ClosureExpr>(ACE)) {
-          if (closureExpr->getCapturedSelfDecl()) {
-            closureHasExplicitSelfCapture = true;
           }
         }
-
+        
         // Implicit self was accidentially allowed in examples like this
         // in Swift 5.3-5.5, so check for this case and emit a warning
         // instead of an error:
@@ -2245,7 +2228,20 @@ static void diagnoseImplicitSelfUseInClosure(const Expr *E,
         //     }
         //   }
         //
-        return !selfDecl->isSelfParameter() && !closureHasExplicitSelfCapture;
+        bool isEscapingClosureWithExplicitSelfCapture = false;
+        if (isClosureRequiringSelfQualification(ACE, Ctx)) {
+          if (auto closureExpr = dyn_cast<ClosureExpr>(ACE)) {
+            if (closureExpr->getCapturedSelfDecl()) {
+              isEscapingClosureWithExplicitSelfCapture = true;
+            }
+          }
+        }
+
+        if (!selfDecl->isSelfParameter() && !isEscapingClosureWithExplicitSelfCapture) {
+          return true;
+        }
+        
+        return false;
       };
 
       SourceLoc memberLoc = SourceLoc();

test/expr/closure/closures.swift

@@ -1234,7 +1234,7 @@ class TestGithubIssue70089 {
           self.x += 1
 
           doVoidStuff {
-            x += 1  // expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+            x += 1  // expected-warning {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
             self.x += 1
           }
 
@@ -1300,7 +1300,7 @@ class TestGithubIssue70089 {
         }
 
         doVoidStuffNonEscaping {
-          x += 1 // expected-error {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          x += 1 // expected-warning {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
           self.x += 1
         }
       }
@@ -1311,6 +1311,24 @@ class TestGithubIssue70089 {
           self.x += 1
         }
       }
+
+      doVoidStuff { [self] in
+        doVoidStuff { // expected-note {{capture 'self' explicitly to enable implicit 'self' in this closure}}
+          doVoidStuff {
+            x += 1 // expected-warning {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note {{reference 'self.' explicitly}}
+            self.x += 1
+          }
+        }
+      }
+
+      doVoidStuffNonEscaping { [self] in
+        doVoidStuffNonEscaping {
+          doVoidStuffNonEscaping {
+            x += 1
+            self.x += 1
+          }
+        }
+      }
     }
 }
 
@@ -1357,7 +1375,7 @@ class TestGithubIssue69911 {
         }
 
         doVoidStuffNonEscaping { [self = TestGithubIssue69911()] in // expected-note {{variable other than 'self' captured here under the name 'self' does not enable implicit 'self'}} expected-note {{variable other than 'self' captured here under the name 'self' does not enable implicit 'self'}} expected-note {{variable other than 'self' captured here under the name 'self' does not enable implicit 'self'}}
-           x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+           x += 1 // expected-warning {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
            self.x += 1
 
            doVoidStuff { [self] in
@@ -1366,7 +1384,7 @@ class TestGithubIssue69911 {
            }
 
            doVoidStuffNonEscaping {
-             x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+             x += 1 // expected-warning{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
              self.x += 1
            }
         }
@@ -1382,33 +1400,33 @@ class TestGithubIssue69911 {
         guard let self = self ?? TestGithubIssue69911.staticOptional else { return }
 
         doVoidStuffNonEscaping {
-          x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          x += 1 // expected-warning{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
           self.x += 1
         }
 
         doVoidStuffNonEscaping {
           doVoidStuffNonEscaping {
-            x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+            x += 1 // expected-warning{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
             self.x += 1
           }
         }
 
         doVoidStuff { [self] in
           doVoidStuffNonEscaping {
-            x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+            x += 1 // expected-warning{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
             self.x += 1
           }
         }
 
         doVoidStuffNonEscaping { [self] in
           doVoidStuffNonEscaping { [self] in
-            x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+            x += 1 // expected-warning{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
             self.x += 1
           }
         }
 
         doVoidStuffNonEscaping { [self = TestGithubIssue69911()] in // expected-note {{variable other than 'self' captured here under the name 'self' does not enable implicit 'self'}}
-           x += 1 // expected-error{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+           x += 1 // expected-warning{{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
            self.x += 1
         }
 
@@ -1417,6 +1435,13 @@ class TestGithubIssue69911 {
           self?.x += 1
         }
 
+        doVoidStuffNonEscaping { [self = TestGithubIssue69911()] in // expected-note {{variable other than 'self' captured here under the name 'self' does not enable implicit 'self'}}
+          doVoidStuffNonEscaping { [self] in
+            x += 1 // expected-warning {{reference to property 'x' in closure requires explicit use of 'self' to make capture semantics explicit}}
+            self.x += 1
+          }
+        }
+
         doVoidStuff { [self] in
           doVoidStuffNonEscaping {
             doVoidStuffNonEscaping {
@@ -1699,4 +1724,13 @@ struct TestInvalidSelfCaptureInStruct {
       self.method()
     }
   }
+}
+
+struct TestAsyncLet {
+  func bar() -> Int { 0 }
+  func foo() async {
+    let _ = {
+      async let _ = bar()
+    }
+  }
 }