[Sema] Sink Pattern invalidation into SyntacticElementTarget::markInvalid

Rather than handling this in the callers of `typeCheckExpression`,
handle it in the constraint system's invalidation logic. This also
ensures we set a type for sub-pattern nodes.

Author: hamishknight

lib/AST/Stmt.cpp

@@ -493,7 +493,7 @@ Type DoCatchStmt::getCaughtErrorType() const {
     ->getCaseLabelItems()
     .front()
     .getPattern();
-  if (firstPattern->hasType())
+  if (firstPattern->hasType() && !firstPattern->getType()->hasError())
     return firstPattern->getType();
 
   return Type();

lib/Sema/SyntacticElementTarget.cpp

@@ -308,6 +308,11 @@ void SyntacticElementTarget::markInvalid() const {
       return Action::Continue(E);
     }
 
+    PreWalkResult<Pattern *> walkToPatternPre(Pattern *P) override {
+      P->setType(ErrorType::get(Ctx));
+      return Action::Continue(P);
+    }
+
     PreWalkAction walkToDeclPre(Decl *D) override {
       // Mark any VarDecls and PatternBindingDecls as invalid.
       if (auto *VD = dyn_cast<VarDecl>(D)) {

lib/Sema/TypeCheckConstraints.cpp

@@ -852,31 +852,8 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
     return false;
   }
 
-  auto &Context = DC->getASTContext();
   initializer = target.getAsExpr();
   pattern = target.getInitializationPattern();
-
-  if (!initializer->getType())
-    initializer->setType(ErrorType::get(Context));
-
-  // Assign error types to the pattern and its variables, to prevent it from
-  // being referenced by the constraint system.
-  if (patternType->hasUnresolvedType() ||
-      patternType->hasPlaceholder() ||
-      patternType->hasUnboundGenericType()) {
-    pattern->setType(ErrorType::get(Context));
-  }
-
-  pattern->forEachVariable([&](VarDecl *var) {
-    // Don't change the type of a variable that we've been able to
-    // compute a type for.
-    if (var->hasInterfaceType() &&
-        !var->getTypeInContext()->hasUnboundGenericType() &&
-        !var->isInvalid())
-      return;
-
-    var->setInvalid();
-  });
   return true;
 }
 
@@ -928,25 +905,14 @@ bool TypeChecker::typeCheckForEachPreamble(DeclContext *dc, ForEachStmt *stmt) {
   FrontendStatsTracer statsTracer(Context.Stats, "typecheck-for-each", stmt);
   PrettyStackTraceStmt stackTrace(Context, "type-checking-for-each", stmt);
 
-  auto failed = [&]() -> bool {
-    // Invalidate the pattern and the var decl.
-    stmt->getPattern()->setType(ErrorType::get(Context));
-    stmt->getPattern()->forEachVariable([&](VarDecl *var) {
-      if (var->hasInterfaceType() && !var->isInvalid())
-        return;
-      var->setInvalid();
-    });
-    return true;
-  };
-
   auto target = SyntacticElementTarget::forForEachPreamble(stmt, dc);
   if (!typeCheckTarget(target))
-    return failed();
+    return true;
 
   if (auto *where = stmt->getWhere()) {
     auto boolType = dc->getASTContext().getBoolType();
     if (!boolType)
-      return failed();
+      return true;
 
     SyntacticElementTarget whereClause(where, dc, {boolType, CTP_Condition},
                                        /*isDiscarded=*/false);
@@ -960,7 +926,7 @@ bool TypeChecker::typeCheckForEachPreamble(DeclContext *dc, ForEachStmt *stmt) {
   // Check to see if the sequence expr is throwing (in async context),
   // if so require the stmt to have a `try`.
   if (diagnoseUnhandledThrowsInAsyncContext(dc, stmt))
-    return failed();
+    return true;
 
   return false;
 }