Simplify diagnostics

Author: calda

lib/Sema/MiscDiagnostics.cpp

@@ -1849,16 +1849,14 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
       return selfDeclAllowsImplicitSelf510(DRE, ty, inClosure);
 
     return selfDeclAllowsImplicitSelf(DRE->getDecl(), ty, inClosure,
-                                      /*validateParentClosures:*/ true,
-                                      /*validatingNestedClosure:*/ false);
+                                      /*isValidatingParentClosures:*/ false);
   }
 
   /// Whether or not implicit self is allowed for this implicit self decl
   static bool selfDeclAllowsImplicitSelf(const ValueDecl *selfDecl,
                                          const Type captureType,
                                          const AbstractClosureExpr *inClosure,
-                                         bool validateParentClosures,
-                                         bool validatingNestedClosure) {
+                                         bool isValidatingParentClosures) {
     ASTContext &ctx = inClosure->getASTContext();
 
     auto requiresSelfQualification =
@@ -1919,7 +1917,14 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
     //    isn't unwrapped correctly in the nested closure, we would have
     //    already noticed when validating that closure.
     if (closureHasWeakSelfCapture(inClosure) &&
-        !selfDeclDefinedInConditionalStmt && !validatingNestedClosure) {
+        !selfDeclDefinedInConditionalStmt && !isValidatingParentClosures) {
+      return false;
+    }
+
+    // If the self decl refers to a weak capture in a parent closure,
+    // then implicit self is not allowed.
+    if (selfDecl->getInterfaceType()->is<WeakStorageType>() &&
+        !closureHasWeakSelfCapture(inClosure)) {
       return false;
     }
 
@@ -1945,7 +1950,7 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
     //  - We have to do this for all closures, even closures that typically
     //    don't require self qualificationm since an invalid self capture in
     //    a parent closure still disallows implicit self in a nested closure.
-    if (validateParentClosures) {
+    if (!isValidatingParentClosures) {
       return !implicitSelfDisallowedDueToInvalidParent(selfDecl, captureType,
                                                        inClosure);
     } else {
@@ -2043,25 +2048,6 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
         return nullptr;
       }
 
-      // If this closure has a non-weak capture, the parent closure
-      // has a weak capture, the implicit self decl refers directly
-      // to this closure's self capture, and that capture directly
-      // refers to the parent closure's weak self capture, then
-      // this is invalid.
-      //  - Normally this is rejected by the type checker because
-      //    self is optional here, but that doesn't happen when calling
-      //    a method defined on `Optional<Self>`.
-      if (!closureHasWeakSelfCapture(inClosure)) {
-        if (auto closureSelfCapture = selfCapture(inClosure)) {
-          if (auto parentWeakSelfCapture = weakSelfCapture(outerClosure)) {
-            if (selfDecl == closureSelfCapture &&
-                parentWeakSelfCapture == outerSelfDecl) {
-              return outerClosure;
-            }
-          }
-        }
-      }
-
       // Check if this closure contains the self decl.
       //  - If the self decl is defined in the closure's body, its
       //    decl context will be the closure itself.
@@ -2092,8 +2078,7 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
         // is sufficient to enable implicit self.
         if (!selfDeclAllowsImplicitSelf(outerSelfDecl, captureType,
                                         outerClosure,
-                                        /*validateParentClosures:*/ false,
-                                        /*validatingNestedClosure:*/ true)) {
+                                        /*isValidatingParentClosures:*/ true)) {
           return outerClosure;
         }
 
@@ -2106,8 +2091,7 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
       // parent closures, we don't need to do that separate for this closure.
       if (validateIntermediateParents) {
         if (!selfDeclAllowsImplicitSelf(selfDecl, captureType, outerClosure,
-                                        /*validateParentClosures*/ false,
-                                        /*validatingNestedClosure*/ true)) {
+                                        /*isValidatingParentClosures*/ true)) {
           return outerClosure;
         }
       }

test/expr/closure/closures_swift6.swift

@@ -501,14 +501,14 @@ class TestGithubIssue70089 {
       }
 
       doVoidStuff { [weak self] in
-        doVoidStuff { [self] in
-          x += 1 // expected-error@-1 {{value of optional type 'TestGithubIssue70089?' must be unwrapped to a value of type 'TestGithubIssue70089'}} expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} expected-note@-1 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+        doVoidStuff { [self] in // expected-error {{value of optional type 'TestGithubIssue70089?' must be unwrapped to a value of type 'TestGithubIssue70089'}} expected-note {{coalesce using '??' to provide a default when the optional value contains 'nil'}} expected-note {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+          x += 1
         }
       }
 
       doVoidStuff { [weak self] in
-        doVoidStuff { [self] in // 
-          self.x += 1 // expected-error@-1 {{value of optional type 'TestGithubIssue70089?' must be unwrapped to a value of type 'TestGithubIssue70089'}} expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} expected-note@-1 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+        doVoidStuff { [self] in // expected-error {{value of optional type 'TestGithubIssue70089?' must be unwrapped to a value of type 'TestGithubIssue70089'}} expected-note {{coalesce using '??' to provide a default when the optional value contains 'nil'}} expected-note {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+          self.x += 1
         }
       }
 
@@ -939,8 +939,6 @@ extension TestExtensionOnOptionalSelf? {
   func foo() {
     _ = { [weak self] in
       foo() // expected-error {{implicit use of 'self' in closure; use 'self.' to make capture semantics explicit}}
-      self.foo()
-      self?.bar()
     }
 
     _ = {
@@ -951,15 +949,15 @@ extension TestExtensionOnOptionalSelf? {
 
     _ = { [weak self] in
       _ = {
-        foo()
+        foo() // expected-error {{implicit use of 'self' in closure; use 'self.' to make capture semantics explicit}}
         self.foo()
         self?.bar()
       }
     }
 
     _ = { [weak self] in
       _ = { [self] in
-        foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
+        foo()
         self.foo()
         self?.bar()
       }
@@ -983,22 +981,22 @@ extension TestExtensionOnOptionalSelf {
 
     _ = { [weak self] in
       _ = {
-        foo()
+        foo() // expected-error {{implicit use of 'self' in closure; use 'self.' to make capture semantics explicit}}
         self.foo()
       }
     }
 
     _ = { [weak self] in
       _ = { [self] in
-        foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
+        foo()
         self.foo()
       }
     }
 
     _ = { [weak self] in
       _ = { [self] in
         _ = { [self] in
-          foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          foo()
           self.foo()
         }
       }
@@ -1007,11 +1005,18 @@ extension TestExtensionOnOptionalSelf {
     _ = { [weak self] in
       doVoidStuffNonEscaping {
         _ = { [self] in
-          foo() // expected-error {{call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit}}
+          foo()
           self.foo()
         }
       }
     }
+
+    _ = { [weak self] in
+      guard case let self = self else { return }
+      _ = { [self] in
+        foo()
+      }
+    }
   }
 }