[Sema] Allow falling back to outer results from closure

If we encounter a variable declared after its use within a closure,
we can fallback to using an outer result if present. This matches the
behavior outside of a closure and generally seems more consistent with
the behavior we have if we also find an inner result.

rdar://163656720

Author: hamishknight

lib/Sema/PreCheckTarget.cpp

@@ -393,8 +393,7 @@ static bool isMemberChainTail(Expr *expr, Expr *parent, MemberChainKind kind) {
 }
 
 static bool isValidForwardReference(ValueDecl *D, DeclContext *DC,
-                                    ValueDecl *&localDeclAfterUse,
-                                    bool &shouldUseOuterResults) {
+                                    ValueDecl *&localDeclAfterUse) {
   // Only VarDecls require declaration before use.
   auto *VD = dyn_cast<VarDecl>(D);
   if (!VD)
@@ -412,11 +411,6 @@ static bool isValidForwardReference(ValueDecl *D, DeclContext *DC,
     }
     if (isa<AbstractClosureExpr>(DC) ||
         (isa<FuncDecl>(DC) && cast<FuncDecl>(DC)->isDeferBody())) {
-      // If we cross a closure, don't allow falling back to an outer result if
-      // we have a forward reference. This preserves the behavior prior to
-      // diagnosing this in Sema.
-      if (isa<AbstractClosureExpr>(DC))
-        shouldUseOuterResults = false;
       DC = DC->getParent();
       continue;
     }
@@ -592,20 +586,18 @@ static Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC,
     Lookup = TypeChecker::lookupUnqualified(DC, LookupName, Loc, lookupOptions);
 
     ValueDecl *localDeclAfterUse = nullptr;
-    bool shouldUseOuterResults = true;
     AllDeclRefs = findNonMembers(
         Lookup.innerResults(), UDRE->getRefKind(),
         /*breakOnMember=*/true, ResultValues, [&](ValueDecl *D) {
-          return isValidForwardReference(D, DC, localDeclAfterUse,
-                                         shouldUseOuterResults);
+          return isValidForwardReference(D, DC, localDeclAfterUse);
         });
 
     // If local declaration after use is found, check outer results for
     // better matching candidates.
     if (ResultValues.empty() && localDeclAfterUse) {
       auto innerDecl = localDeclAfterUse;
       while (localDeclAfterUse) {
-        if (!shouldUseOuterResults || Lookup.outerResults().empty()) {
+        if (Lookup.outerResults().empty()) {
           Context.Diags.diagnose(Loc, diag::use_local_before_declaration, Name);
           Context.Diags.diagnose(innerDecl, diag::decl_declared_here,
                                  localDeclAfterUse);
@@ -618,8 +610,7 @@ static Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC,
         AllDeclRefs = findNonMembers(
             Lookup.innerResults(), UDRE->getRefKind(),
             /*breakOnMember=*/true, ResultValues, [&](ValueDecl *D) {
-              return isValidForwardReference(D, DC, localDeclAfterUse,
-                                             shouldUseOuterResults);
+              return isValidForwardReference(D, DC, localDeclAfterUse);
             });
       }
     }

test/NameLookup/use_before_declaration_shadowing.swift

@@ -54,20 +54,41 @@ func testTopLevel() {
   }
   do {
     _ = {
-      let _: String = topLevelString // expected-error {{use of local variable 'topLevelString' before its declaration}}
+      let _: String = topLevelString
     }
-    let topLevelString = 0 // expected-note {{'topLevelString' declared here}}
+    let topLevelString = 0
+    _ = topLevelString
   }
   _ = {
     _ = {
-      let _: String = topLevelString // expected-error {{use of local variable 'topLevelString' before its declaration}}
+      let _: String = topLevelString
     }
-    let topLevelString = 0 // expected-note {{'topLevelString' declared here}}
+    let topLevelString = 0
+    _ = topLevelString
   }
   func bar() {
     _ = {
-      let _: String = topLevelString // expected-error {{use of local variable 'topLevelString' before its declaration}}
+      let _: String = topLevelString
     }
-    let topLevelString = 0 // expected-note {{'topLevelString' declared here}}
+    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
   }
 }

test/SILOptimizer/closure_lifetime_fixup_undef.swift

@@ -1,14 +1,12 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend %s -sil-verify-all -c
 
-// Report the error but don't crash.
+// Make sure we don't crash.
 
 class TestUndefined {
   private var stringList: [String]!
 
   func dontCrash(strings: [String]) {
     assert(stringList.allSatisfy({ $0 == stringList.first!}))
-    // expected-error@-1 {{use of local variable 'stringList' before its declaration}}
     let stringList = strings.filter({ $0 == "a" })
-    // expected-note@-1 {{'stringList' declared here}}
   }
 }

test/expr/expressions.swift

@@ -252,12 +252,21 @@ func test_lambda1() {
 
 func test_lambda2() {
   // A recursive lambda.
-  var fib = { (n: Int) -> Int in // expected-note 2{{'fib' declared here}}
+  var fibLocal = { (n: Int) -> Int in // expected-note 2{{'fibLocal' declared here}}
     if (n < 2) {
       return n
     }
 
-    return fib(n-1)+fib(n-2) // expected-error 2{{use of local variable 'fib' before its declaration}}
+    return fibLocal(n-1)+fibLocal(n-2) // expected-error 2{{use of local variable 'fibLocal' before its declaration}}
+  }
+
+  var fib = { (n: Int) -> Int in
+    if (n < 2) {
+      return n
+    }
+
+    // These resolve to the top-level function.
+    return fib(n-1)+fib(n-2)
   }
 }