[CS] Invalidate VarDecls in targets that fail to type-check

This avoids a crash that could occur when
attempting to query their interface type later,
which could cause us to attempt to type-check
the Decl separately from its enclosing closure.

Eventually we also ought to use this to fill in
ErrorTypes for expressions (since type-checking
ought to only produce typed AST), but I'm leaving
that as future work for now.

rdar://120012473

Author: hamishknight

include/swift/Sema/SyntacticElementTarget.h

@@ -887,6 +887,10 @@ class SyntacticElementTarget {
     llvm_unreachable("invalid target type");
   }
 
+  /// Marks the target as invalid, filling in ErrorTypes for the AST it
+  /// represents.
+  void markInvalid() const;
+
   /// Walk the contents of the application target.
   std::optional<SyntacticElementTarget> walk(ASTWalker &walker) const;
 };

lib/Sema/SyntacticElementTarget.cpp

@@ -301,6 +301,30 @@ bool SyntacticElementTarget::contextualTypeIsOnlyAHint() const {
   llvm_unreachable("invalid contextual type");
 }
 
+void SyntacticElementTarget::markInvalid() const {
+  class InvalidationWalker : public ASTWalker {
+    PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
+      // TODO: We ought to fill in ErrorTypes for expressions here; ultimately
+      // type-checking should always produce typed AST.
+      return Action::Continue(E);
+    }
+
+    PreWalkAction walkToDeclPre(Decl *D) override {
+      // Mark any VarDecls and PatternBindingDecls as invalid.
+      if (auto *VD = dyn_cast<VarDecl>(D)) {
+        // Only set invalid if we don't already have an interface type computed.
+        if (!VD->hasInterfaceType())
+          D->setInvalid();
+      } else if (isa<PatternBindingDecl>(D)) {
+        D->setInvalid();
+      }
+      return Action::VisitNodeIf(isa<PatternBindingDecl>(D));
+    }
+  };
+  InvalidationWalker walker;
+  walk(walker);
+}
+
 std::optional<SyntacticElementTarget>
 SyntacticElementTarget::walk(ASTWalker &walker) const {
   SyntacticElementTarget result = *this;

lib/Sema/TypeCheckConstraints.cpp

@@ -466,6 +466,13 @@ TypeChecker::typeCheckExpression(SyntacticElementTarget &target,
 std::optional<SyntacticElementTarget>
 TypeChecker::typeCheckTarget(SyntacticElementTarget &target,
                              TypeCheckExprOptions options) {
+  auto errorResult = [&]() -> std::optional<SyntacticElementTarget> {
+    // Fill in ErrorTypes for the target if we can.
+    if (!options.contains(TypeCheckExprFlags::AvoidInvalidatingAST))
+      target.markInvalid();
+
+    return std::nullopt;
+  };
   DeclContext *dc = target.getDeclContext();
   auto &Context = dc->getASTContext();
   PrettyStackTraceLocation stackTrace(Context, "type-checking-target",
@@ -475,11 +482,12 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target,
   // expression and folding sequence expressions.
   if (ConstraintSystem::preCheckTarget(
           target, /*replaceInvalidRefsWithErrors=*/true)) {
-    return std::nullopt;
+    return errorResult();
   }
 
   // Check whether given target has a code completion token which requires
-  // special handling.
+  // special handling. Returns true if handled, in which case we've already
+  // type-checked it for completion, and don't need the solution applied.
   if (Context.CompletionCallback &&
       typeCheckForCodeCompletion(target, /*needsPrecheck*/ false,
                                  [&](const constraints::Solution &S) {
@@ -517,7 +525,7 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target,
   // Attempt to solve the constraint system.
   auto viable = cs.solve(target, allowFreeTypeVariables);
   if (!viable)
-    return std::nullopt;
+    return errorResult();
 
   // Apply this solution to the constraint system.
   // FIXME: This shouldn't be necessary.
@@ -528,7 +536,7 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target,
   auto resultTarget = cs.applySolution(solution, target);
   if (!resultTarget) {
     // Failure already diagnosed, above, as part of applying the solution.
-    return std::nullopt;
+    return errorResult();
   }
 
   // Unless the client has disabled them, perform syntactic checks on the
@@ -571,6 +579,13 @@ Type TypeChecker::typeCheckParameterDefault(Expr *&defaultValue,
     DiagnosticTransaction diagnostics(ctx.Diags);
 
     TypeCheckExprOptions options;
+
+    // Avoid invalidating the AST since we'll fall through and try to type-check
+    // with an archetype contextual type opened. Note we also don't need to call
+    // `markInvalid` for the target below since we'll replace the expression
+    // with an ErrorExpr on failure anyway.
+    options |= TypeCheckExprFlags::AvoidInvalidatingAST;
+
     // Expand macro expansion expression at caller side only
     if (!atCallerSide && isa<MacroExpansionExpr>(defaultValue)) {
       options |= TypeCheckExprFlags::DisableMacroExpansions;

lib/Sema/TypeChecker.h

@@ -136,6 +136,10 @@ enum class TypeCheckExprFlags {
 
   /// Don't expand macros.
   DisableMacroExpansions = 0x08,
+
+  /// If set, typeCheckExpression will avoid invalidating the AST if
+  /// type-checking fails. Do not add new uses of this.
+  AvoidInvalidatingAST = 0x10,
 };
 
 using TypeCheckExprOptions = OptionSet<TypeCheckExprFlags>;

test/Constraints/type_inference_from_default_closure.swift

@@ -0,0 +1,6 @@
+// NOTE: This intentionally doesn't use %target-typecheck-verify, this should
+// compile without any errors since we're testing the ASTVerifier is happy.
+// RUN: %target-swift-frontend -typecheck %s
+
+// Make sure we don't prematurely mark 'x' as invalid.
+func testInferenceFromClosureVar<T>(x: T = { var x: Int = 0; return x }()) {}

test/Constraints/type_inference_from_default_exprs.swift

@@ -266,3 +266,6 @@ do {
   // expected-warning@-1 {{dictionary literal of type '[String : Int]' has duplicate entries for string literal key 'a'}}
   // expected-note@-2 2 {{duplicate key declared here}}
 }
+
+func testInferenceFromClosureVarInvalid<T>(x: T = { let x = "" as Int; return x }()) {}
+// expected-error@-1 {{cannot convert value of type 'String' to type 'Int' in coercion}}

test/Index/rdar120012473.swift

@@ -0,0 +1,50 @@
+// RUN: %empty-directory(%t)
+// RUN: not %target-swift-frontend -typecheck %s -index-store-path %t
+
+let k = ""
+
+// Make sure we don't crash here (rdar://120012473).
+func foo() {
+  _ = {
+    let x = switch 1 {
+    case k:
+      return false
+    }
+    return true
+  }
+
+  for x in [0] where ({
+    let x = switch 1 {
+    case k:
+      return false
+    }
+    return true
+  }()) {}
+}
+
+@resultBuilder
+struct Builder {
+  static func buildBlock<T>(_ components: T...) -> T {
+    fatalError()
+  }
+}
+
+@Builder
+func bar() -> Bool {
+  let fn = {
+    let x = switch 1 {
+    case k:
+      return false
+    }
+    return true
+  }
+  fn()
+}
+
+func baz(x: () -> Bool = {
+  let x = switch 1 {
+  case k:
+    return false
+  }
+  return true
+}) {}