[Sema] Catch use-before-declarations in nested closures

CHANGELOG.md

@@ -3,7 +3,69 @@
 > [!NOTE]
 > This is in reverse chronological order, so newer entries are added to the top.
 
-## Swift (next)
+## Swift 6.3
+
+* The checking for illegal forward references to local variables is now consistent regardless of
+  whether the reference appears in a closure. Previously the type-checker could incorrectly permit
+  forward references within a closure that it would reject outside of the closure, however this
+  is not something that can be supported in general in the type-checker. In most cases this should
+  have no impact since such invalid references would have already been rejected by later diagnostic
+  passes in the compiler. However there are a couple of cases that were previously legal that are
+  now illegal.
+
+  These include:
+
+  - `lazy` local variables with initializers that forward reference a local variable in a closure,
+    or local variables with attached macros that relocate the initializer into an accessor, e.g:
+
+    ```swift
+    func foo() {
+      lazy var x = { y } // error: use of local variable 'y' before its declaration
+      let y = 0
+    }
+    ```
+
+    ```swift
+    func foo() {
+      @LazyLikeMacro var x = { y } // error: use of local variable 'y' before its declaration
+      let y = 0
+    }
+    ```
+
+  - Forward references to local computed variables from a closure, e.g:
+
+    ```swift
+    func foo() {
+      var x = { y } // error: use of local variable 'y' before its declaration
+      var y: Int { 0 }
+    }
+    ```
+
+  Both cases were already invalid if there was no closure involved. These are now consistently
+  rejected.
+
+  This then allows for consistent shadowing behavior inside and outside of closures, allowing the
+  following previously illegal case to become legal:
+
+  ```swift
+  struct S {
+    var x: Int
+    func foo() {
+      // Already legal, both refer to `self.x`
+      let y = x
+      let x = x
+
+      let z = x // Refers to the local var.
+    }
+    func bar() {
+      // Both previously illegal, now both refer to `self.x`.
+      let y = { x }()
+      let x: Int = { x }()
+
+      let z = x // Still refers to the local var.
+    }
+  }
+  ```
 
 * [SE-0491][]:
   You can now use a module selector to specify which module Swift should look inside to find a named declaration. A

lib/Sema/PreCheckTarget.cpp

@@ -393,29 +393,28 @@ static bool isMemberChainTail(Expr *expr, Expr *parent, MemberChainKind kind) {
 }
 
 static bool isValidForwardReference(ValueDecl *D, DeclContext *DC,
-                                    ValueDecl **localDeclAfterUse) {
-  *localDeclAfterUse = nullptr;
-
-  // References to variables injected by lldb are always valid.
-  if (isa<VarDecl>(D) && cast<VarDecl>(D)->isDebuggerVar())
+                                    ValueDecl *&localDeclAfterUse) {
+  // Only VarDecls require declaration before use.
+  auto *VD = dyn_cast<VarDecl>(D);
+  if (!VD)
     return true;
 
-  // If we find something in the current context, it must be a forward
-  // reference, because otherwise if it was in scope, it would have
-  // been returned by the call to ASTScope::lookupLocalDecls() above.
-  if (D->getDeclContext()->isLocalContext()) {
-    do {
-      if (D->getDeclContext() == DC) {
-        *localDeclAfterUse = D;
-        return false;
-      }
+  // Non-local and variables injected by lldb are always valid.
+  auto *varDC = VD->getDeclContext();
+  if (!varDC->isLocalContext() || VD->isDebuggerVar())
+    return true;
 
-      // If we're inside of a 'defer' context, walk up to the parent
-      // and check again. We don't want 'defer' bodies to forward
-      // reference bindings in the immediate outer scope.
-    } while (isa<FuncDecl>(DC) &&
-             cast<FuncDecl>(DC)->isDeferBody() &&
-             (DC = DC->getParent()));
+  while (true) {
+    if (varDC == DC) {
+      localDeclAfterUse = VD;
+      return false;
+    }
+    if (isa<AbstractClosureExpr>(DC) ||
+        (isa<FuncDecl>(DC) && cast<FuncDecl>(DC)->isDeferBody())) {
+      DC = DC->getParent();
+      continue;
+    }
+    break;
   }
   return true;
 }
@@ -587,12 +586,11 @@ static Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC,
     Lookup = TypeChecker::lookupUnqualified(DC, LookupName, Loc, lookupOptions);
 
     ValueDecl *localDeclAfterUse = nullptr;
-    AllDeclRefs =
-        findNonMembers(Lookup.innerResults(), UDRE->getRefKind(),
-                       /*breakOnMember=*/true, ResultValues,
-                       [&](ValueDecl *D) {
-                         return isValidForwardReference(D, DC, &localDeclAfterUse);
-                       });
+    AllDeclRefs = findNonMembers(
+        Lookup.innerResults(), UDRE->getRefKind(),
+        /*breakOnMember=*/true, ResultValues, [&](ValueDecl *D) {
+          return isValidForwardReference(D, DC, localDeclAfterUse);
+        });
 
     // If local declaration after use is found, check outer results for
     // better matching candidates.
@@ -609,12 +607,11 @@ static Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC,
         Lookup.shiftDownResults();
         ResultValues.clear();
         localDeclAfterUse = nullptr;
-        AllDeclRefs =
-            findNonMembers(Lookup.innerResults(), UDRE->getRefKind(),
-                           /*breakOnMember=*/true, ResultValues,
-                           [&](ValueDecl *D) {
-                             return isValidForwardReference(D, DC, &localDeclAfterUse);
-                           });
+        AllDeclRefs = findNonMembers(
+            Lookup.innerResults(), UDRE->getRefKind(),
+            /*breakOnMember=*/true, ResultValues, [&](ValueDecl *D) {
+              return isValidForwardReference(D, DC, localDeclAfterUse);
+            });
       }
     }
   }

test/NameLookup/name_lookup.swift

@@ -643,8 +643,8 @@ struct PatternBindingWithTwoVars3 { var x = y, y = x }
 
 // https://github.com/apple/swift/issues/51518
 do {
-  let closure1 = { closure2() } // expected-error {{circular reference}} expected-note {{through reference here}}
-  let closure2 = { closure1() } // expected-note {{through reference here}} expected-note {{through reference here}}
+  let closure1 = { closure2() } // expected-error {{use of local variable 'closure2' before its declaration}}
+  let closure2 = { closure1() } // expected-note {{'closure2' declared here}}
 }
 
 func color(with value: Int) -> Int {

test/NameLookup/use_before_declaration_shadowing.swift

@@ -0,0 +1,94 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -parse-as-library
+
+func testLocal() {
+  // The first `y` here is considered the inner result.
+  do {
+    let y = ""
+    do {
+      let _: String = y
+      let y = 0
+      _ = y
+    }
+  }
+  do {
+    let y = ""
+    do {
+      _ = {
+        let _: String = y
+      }
+      let y = 0
+      _ = y
+    }
+  }
+  do {
+    let y = ""
+    _ = {
+      _ = {
+        let _: String = y
+      }
+      let y = 0
+      _ = y
+    }
+  }
+  do {
+    let y = ""
+    func bar() {
+      _ = {
+        let _: String = y
+      }
+      let y = 0
+      _ = y
+    }
+  }
+}
+
+let topLevelString = ""
+
+func testTopLevel() {
+  // Here 'topLevelString' is now an outer result.
+  do {
+    let _: String = topLevelString
+    let topLevelString = 0
+    _ = topLevelString
+  }
+  do {
+    _ = {
+      let _: String = topLevelString
+    }
+    let topLevelString = 0
+    _ = topLevelString
+  }
+  _ = {
+    _ = {
+      let _: String = topLevelString
+    }
+    let topLevelString = 0
+    _ = topLevelString
+  }
+  func bar() {
+    _ = {
+      let _: String = topLevelString
+    }
+    let topLevelString = 0
+    _ = topLevelString
+  }
+}
+
+struct TestLocalPropertyShadowing {
+  var str: String
+
+  func foo() {
+    { _ = str }()
+    let str = str
+    _ = str
+  }
+  func bar() {
+    let str = { str }
+    _ = str
+  }
+  func baz() {
+    let str = str
+    _ = str
+  }
+}