Merge pull request #16967 from dingobye/sr7660

[Sema] Fix spurious use_local_before_declaration errors.

Author: DougGregor

lib/Sema/TypeCheckConstraints.cpp

@@ -569,6 +569,60 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
 
   // FIXME: Need to refactor the way we build an AST node from a lookup result!
 
+  SmallVector<ValueDecl*, 4> ResultValues;
+  ValueDecl *localDeclAfterUse = nullptr;
+  auto isValid = [&](ValueDecl *D) {
+    // FIXME: The source-location checks won't make sense once
+    // EnableASTScopeLookup is the default.
+    if (Loc.isValid() && D->getLoc().isValid() &&
+        D->getDeclContext()->isLocalContext() &&
+        D->getDeclContext() == DC &&
+        Context.SourceMgr.isBeforeInBuffer(Loc, D->getLoc())) {
+      localDeclAfterUse = D;
+      return false;
+    }
+    return true;
+  };
+  bool AllDeclRefs = findNonMembers(
+      *this, Lookup.innerResults(), UDRE->getRefKind(), /*breakOnMember=*/true,
+      ResultValues, isValid);
+
+  // If local declaration after use is found, check outer results for
+  // better matching candidates.
+  if (localDeclAfterUse) {
+    auto innerDecl = localDeclAfterUse;
+
+    // Perform a thorough lookup if outer results was not included before.
+    if (!lookupOptions.contains(NameLookupFlags::IncludeOuterResults)) {
+      auto option = lookupOptions;
+      option |= NameLookupFlags::IncludeOuterResults;
+      Lookup = lookupUnqualified(DC, Name, Loc, option);
+    }
+
+    while (localDeclAfterUse) {
+      if (Lookup.outerResults().empty()) {
+        diagnose(Loc, diag::use_local_before_declaration, Name);
+        diagnose(innerDecl, diag::decl_declared_here, Name);
+        Expr *error = new (Context) ErrorExpr(UDRE->getSourceRange());
+        return error;
+      }
+
+      Lookup.shiftDownResults();
+      ResultValues.clear();
+      localDeclAfterUse = nullptr;
+      AllDeclRefs = findNonMembers(
+          *this, Lookup.innerResults(), UDRE->getRefKind(), /*breakOnMember=*/true,
+          ResultValues, isValid);
+    }
+
+    // Drop outer results if they are not supposed to be included.
+    if (!lookupOptions.contains(NameLookupFlags::IncludeOuterResults)) {
+      Lookup.filter([&](LookupResultEntry Result, bool isOuter) {
+          return !isOuter;
+      });
+    }
+  }
+
   // If we have an unambiguous reference to a type decl, form a TypeExpr.
   if (Lookup.size() == 1 && UDRE->getRefKind() == DeclRefKind::Ordinary &&
       isa<TypeDecl>(Lookup[0].getValueDecl())) {
@@ -586,27 +640,6 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
                                    UDRE->isImplicit());
   }
 
-  SmallVector<ValueDecl*, 4> ResultValues;
-  Expr *error = nullptr;
-  bool AllDeclRefs = findNonMembers(
-      *this, Lookup.innerResults(), UDRE->getRefKind(), /*breakOnMember=*/true,
-      ResultValues, [&](ValueDecl *D) {
-        // FIXME: The source-location checks won't make sense once
-        // EnableASTScopeLookup is the default.
-        if (Loc.isValid() && D->getLoc().isValid() &&
-            D->getDeclContext()->isLocalContext() &&
-            D->getDeclContext() == DC &&
-            Context.SourceMgr.isBeforeInBuffer(Loc, D->getLoc())) {
-          diagnose(Loc, diag::use_local_before_declaration, Name);
-          diagnose(D, diag::decl_declared_here, Name);
-          error = new (Context) ErrorExpr(UDRE->getSourceRange());
-          return false;
-        }
-        return true;
-      });
-  if (error)
-    return error;
-
   if (AllDeclRefs) {
     // Diagnose uses of operators that found no matching candidates.
     if (ResultValues.empty()) {

lib/Sema/TypeCheckNameLookup.cpp

@@ -47,6 +47,26 @@ void LookupResult::filter(
                 Results.end());
 }
 
+void LookupResult::shiftDownResults() {
+  // Remove inner results.
+  Results.erase(Results.begin(), Results.begin() + IndexOfFirstOuterResult);
+  IndexOfFirstOuterResult = 0;
+
+  if (Results.empty())
+    return;
+
+  // Compute IndexOfFirstOuterResult.
+  const DeclContext *dcInner = Results.front().getValueDecl()->getDeclContext();
+  for (auto &&result : Results) {
+    const DeclContext *dc = result.getValueDecl()->getDeclContext();
+    if (dc == dcInner ||
+        (dc->isModuleScopeContext() && dcInner->isModuleScopeContext()))
+      ++IndexOfFirstOuterResult;
+    else
+      break;
+  }
+}
+
 namespace {
   /// Builder that helps construct a lookup result from the raw lookup
   /// data.

lib/Sema/TypeChecker.h

@@ -148,6 +148,11 @@ class LookupResult {
   /// Filter out any results that aren't accepted by the given predicate.
   void
   filter(llvm::function_ref<bool(LookupResultEntry, /*isOuter*/ bool)> pred);
+
+  /// Shift down results by dropping inner results while keeping outer
+  /// results (if any), the innermost of which are recogized as inner
+  /// results afterwards.
+  void shiftDownResults();
 };
 
 /// An individual result of a name lookup for a type.

test/NameBinding/name-binding.swift

@@ -195,16 +195,16 @@ test+++
 //===----------------------------------------------------------------------===//
 
 func forwardReference() {
-  x = 0 // expected-error{{use of local variable 'x' before its declaration}}
-  var x: Float = 0.0 // expected-note{{'x' declared here}}
+  v = 0 // expected-error{{use of local variable 'v' before its declaration}}
+  var v: Float = 0.0 // expected-note{{'v' declared here}}
 }
 
 class ForwardReference {
   var x: Int = 0
 
   func test() {
-    x = 0 // expected-error{{use of local variable 'x' before its declaration}}
-    var x: Float = 0.0 // expected-note{{'x' declared here}}
+    x = 0
+    var x: Float = 0.0 // expected-warning{{variable 'x' was never used; consider replacing with '_' or removing it}}
   }
 }
 

test/Sema/diag_use_before_declaration.swift

@@ -15,8 +15,97 @@ func sr5163() {
 var foo: Int?
 
 func test() {
-  guard let bar = foo else { // expected-error {{use of local variable 'foo' before its declaration}}
+  guard let bar = foo else {
     return
   }
-  let foo = String(bar) // expected-note {{'foo' declared here}}
+  let foo = String(bar)
+}
+
+// SR-7660
+class C {
+  var variable: Int?
+  func f() {
+    guard let _ = variable else { return }
+    let variable = 1 // expected-warning {{initialization of immutable value 'variable' was never used; consider replacing with assignment to '_' or removing it}}
+  }
+}
+
+//===----------------------------------------------------------------------===//
+// Nested scope
+//===----------------------------------------------------------------------===//
+
+func nested_scope_1() {
+  do {
+    do {
+      let _ = x // expected-error {{use of local variable 'x' before its declaration}}
+      let x = 111 // expected-note {{'x' declared here}}
+    }
+    let x = 11
+  }
+  let x = 1
+}
+
+func nested_scope_2() {
+  do {
+    let x = 11
+    do {
+      let _ = x
+      let x = 111 // expected-warning {{initialization of immutable value 'x' was never used; consider replacing with assignment to '_' or removing it}}
+    }
+  }
+  let x = 1  // expected-warning {{initialization of immutable value 'x' was never used; consider replacing with assignment to '_' or removing it}}
+}
+
+func nested_scope_3() {
+  let x = 1
+  do {
+    do {
+      let _ = x
+      let x = 111 // expected-warning {{initialization of immutable value 'x' was never used; consider replacing with assignment to '_' or removing it}}
+    }
+    let x = 11 // expected-warning {{initialization of immutable value 'x' was never used; consider replacing with assignment to '_' or removing it}}
+  }
+}
+
+//===----------------------------------------------------------------------===//
+// Type scope
+//===----------------------------------------------------------------------===//
+
+class Ty {
+  var v : Int?
+
+  func fn() {
+    let _ = v
+    let v = 1 // expected-warning {{initialization of immutable value 'v' was never used; consider replacing with assignment to '_' or removing it}}
+  }
+}
+
+//===----------------------------------------------------------------------===//
+// File scope
+//===----------------------------------------------------------------------===//
+
+let g = 0
+
+func file_scope_1() {
+  let _ = g
+  let g = 1 // expected-warning {{initialization of immutable value 'g' was never used; consider replacing with assignment to '_' or removing it}}
+}
+
+func file_scope_2() {
+  let _ = gg // expected-error {{use of local variable 'gg' before its declaration}}
+  let gg = 1 // expected-note {{'gg' declared here}}
+}
+
+//===----------------------------------------------------------------------===//
+// Module scope
+//===----------------------------------------------------------------------===//
+
+func module_scope_1() {
+  let _ = print // Legal use of func print declared in Swift Standard Library
+  let print = "something" // expected-warning {{initialization of immutable value 'print' was never used; consider replacing with assignment to '_' or removing it}}
+}
+
+func module_scope_2() {
+  let _ = another_print // expected-error {{use of local variable 'another_print' before its declaration}}
+  let another_print = "something" // expected-note {{'another_print' declared here}}
 }