[Sema] Enforce we don't type-check bindings independently of closures

Bindings in closures must be type-checked together with the surrounding
closure, add an assertion to make sure we don't try this. Carve out
an exception for code completion and error cases which may still kick
lazy type-checking. We ought to eventually fix up all the error cases
cases though since they imply we're leaving bits of the AST
un-type-checked.

Author: hamishknight

lib/Sema/TypeCheckConstraints.cpp

@@ -841,6 +841,28 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
                 initializer, DC, patternType, pattern,
                 /*bindPatternVarsOneWay=*/false);
 
+  // Bindings cannot be type-checked independently from their context in a
+  // closure, make sure we don't ever attempt to do this. If we want to be able
+  // to lazily type-check these we'll need to type-check the entire surrounding
+  // expression.
+  if (auto *CE = dyn_cast<ClosureExpr>(DC)) {
+    if (!PBD || !isPlaceholderVar(PBD)) {
+      // Completion may trigger lazy type-checking as part of fallback
+      // unqualified lookup, just decline to type-check. We may also end up
+      // here for invalid code, e.g in a closure for an out-of-place ReturnStmt,
+      // which we don't currently type-check but may still kick semantic
+      // queries for. Also just decline to type-check in this case.
+      // FIXME: We shouldn't be leaving AST un-type-checked in invalid cases, we
+      // ought to fix that and then change this to only consider IDE inspection.
+      auto &ctx = CE->getASTContext();
+      if (ctx.hadError() || ctx.SourceMgr.hasIDEInspectionTargetBuffer()) {
+        target.markInvalid();
+        return true;
+      }
+      ABORT("Cannot type-check PatternBindingDecl without closure context");
+    }
+  }
+
   // Type-check the initializer.
   auto resultTarget = typeCheckExpression(target, options);
 

test/IDE/complete_unqualified_fallback.swift

@@ -1,4 +1,4 @@
-// RUN: %batch-code-completion
+// RUN: %batch-code-completion -solver-scope-threshold=50
 
 protocol P0 {}
 protocol P1 {}
@@ -47,8 +47,16 @@ S {
     S {
       S {
         S {
+          let x = 0
+          let y: Int = 0
           #^COMPLETE^#
-          // COMPLETE: Decl[Struct]/CurrModule: FooBar[#FooBar#]; name=FooBar
+          // COMPLETE-DAG: Decl[Struct]/CurrModule: FooBar[#FooBar#]; name=FooBar
+
+          // We decline to type-check a variable indepedently of the surrounding
+          // closure, so this has an ErrorType. If this changes, you either need
+          // to make this test case more complex or tweak the limits.
+          // COMPLETE-DAG: Decl[LocalVar]/Local: x[#_#]; name=x
+          // COMPLETE-DAG: Decl[LocalVar]/Local: y[#Int#]; name=y
         }
       }
     }

validation-test/compiler_crashers/ConstraintWalker-walkToExprPost-c956e0.swift renamed to validation-test/compiler_crashers_fixed/ConstraintWalker-walkToExprPost-c956e0.swift

@@ -1,5 +1,5 @@
 // {"kind":"typecheck","original":"1562769e","signature":"(anonymous namespace)::ConstraintWalker::walkToExprPost(swift::Expr*)"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 enum c
   func d()  e {
     if let