[CS] Don’t fail constraint generation for ErrorExpr or if type fails to resolve

Instead of failing constraint generation by returning `nullptr` for an `ErrorExpr` or returning a null type when a type fails to be resolved, return a fresh type variable. This allows the constraint solver to continue further and produce more meaningful diagnostics.

Most importantly, it allows us to produce a solution where previously constraint generation for a syntactic element had failed, which is required to type check multi-statement closures in result builders inside the constraint system.

Author: ahoppen

include/swift/Sema/CSFix.h

@@ -295,6 +295,9 @@ enum class FixKind : uint8_t {
   /// Ignore result builder body if it has `return` statements.
   IgnoreResultBuilderWithReturnStmts,
 
+  /// Ignore `ErrorExpr` or `ErrorType` during pre-check.
+  IgnoreInvalidASTNode,
+
   /// Resolve type of `nil` by providing a contextual type.
   SpecifyContextualTypeForNil,
 
@@ -707,6 +710,26 @@ class ContextualMismatch : public ConstraintFix {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
+  bool coalesceAndDiagnose(const Solution &solution,
+                           ArrayRef<ConstraintFix *> secondaryFixes,
+                           bool asNote = false) const override {
+    // If the from type or to type is a placeholer type that corresponds to an
+    // ErrorExpr, the issue has already been diagnosed. There's no need to
+    // produce another diagnostic for the contextual mismatch complainting that
+    // a type is not convertible to a placeholder type.
+    if (auto fromPlaceholder = getFromType()->getAs<PlaceholderType>()) {
+      if (fromPlaceholder->getOriginator().is<ErrorExpr *>()) {
+        return true;
+      }
+    }
+    if (auto toPlaceholder = getToType()->getAs<PlaceholderType>()) {
+      if (toPlaceholder->getOriginator().is<ErrorExpr *>()) {
+        return true;
+      }
+    }
+    return ConstraintFix::coalesceAndDiagnose(solution, secondaryFixes, asNote);
+  }
+
   bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;
 
   static ContextualMismatch *create(ConstraintSystem &cs, Type lhs, Type rhs,
@@ -2693,6 +2716,32 @@ class IgnoreResultBuilderWithReturnStmts final
   }
 };
 
+class IgnoreInvalidASTNode final : public ConstraintFix {
+  IgnoreInvalidASTNode(ConstraintSystem &cs, ConstraintLocator *locator)
+      : IgnoreInvalidASTNode(cs, FixKind::IgnoreInvalidASTNode, locator) {}
+
+protected:
+  IgnoreInvalidASTNode(ConstraintSystem &cs, FixKind kind,
+                       ConstraintLocator *locator)
+      : ConstraintFix(cs, kind, locator) {}
+
+public:
+  std::string getName() const override { return "ignore invalid AST node"; }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static IgnoreInvalidASTNode *create(ConstraintSystem &cs,
+                                      ConstraintLocator *locator);
+
+  static bool classof(ConstraintFix *fix) {
+    return fix->getKind() == FixKind::IgnoreInvalidASTNode;
+  }
+};
+
 class SpecifyContextualTypeForNil final : public ConstraintFix {
   SpecifyContextualTypeForNil(ConstraintSystem &cs,
                               ConstraintLocator *locator)

lib/Sema/CSFix.cpp

@@ -1945,6 +1945,16 @@ IgnoreInvalidResultBuilderBody::create(ConstraintSystem &cs,
   return new (cs.getAllocator()) IgnoreInvalidResultBuilderBody(cs, locator);
 }
 
+bool IgnoreInvalidASTNode::diagnose(const Solution &solution,
+                                    bool asNote) const {
+  return true; // Already diagnosed by the producer of ErrorExpr or ErrorType.
+}
+
+IgnoreInvalidASTNode *IgnoreInvalidASTNode::create(ConstraintSystem &cs,
+                                                   ConstraintLocator *locator) {
+  return new (cs.getAllocator()) IgnoreInvalidASTNode(cs, locator);
+}
+
 bool SpecifyContextualTypeForNil::diagnose(const Solution &solution,
                                            bool asNote) const {
   MissingContextualTypeForNil failure(solution, getLocator());

lib/Sema/CSGen.cpp

@@ -890,6 +890,11 @@ namespace {
       // that member through the base returns a value convertible to the type
       // of this expression.
       auto baseTy = CS.getType(base);
+      if (isa<ErrorExpr>(base)) {
+        return CS.createTypeVariable(
+            CS.getConstraintLocator(expr, ConstraintLocator::Member),
+            TVO_CanBindToHole);
+      }
       auto tv = CS.createTypeVariable(
                   CS.getConstraintLocator(expr, ConstraintLocator::Member),
                   TVO_CanBindToLValue | TVO_CanBindToNoEscape);
@@ -1068,19 +1073,11 @@ namespace {
     ConstraintSystem &getConstraintSystem() const { return CS; }
 
     virtual Type visitErrorExpr(ErrorExpr *E) {
-      if (!CS.isForCodeCompletion())
-        return nullptr;
-
-      // For code completion, treat error expressions that don't contain
-      // the completion location itself as holes. If an ErrorExpr contains the
-      // code completion location, a fallback typecheck is called on the
-      // ErrorExpr's OriginalExpr (valid sub-expression) if it had one,
-      // independent of the wider expression containing the ErrorExpr, so
-      // there's no point attempting to produce a solution for it.
-      if (CS.containsCodeCompletionLoc(E))
-        return nullptr;
+      CS.recordFix(
+          IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(E)));
 
-      return PlaceholderType::get(CS.getASTContext(), E);
+      return CS.createTypeVariable(CS.getConstraintLocator(E),
+                                   TVO_CanBindToHole);
     }
 
     virtual Type visitCodeCompletionExpr(CodeCompletionExpr *E) {
@@ -1374,7 +1371,11 @@ namespace {
       const auto result = TypeResolution::resolveContextualType(
           repr, CS.DC, resCtx, genericOpener, placeholderHandler);
       if (result->hasError()) {
-        return Type();
+        CS.recordFix(
+            IgnoreInvalidASTNode::create(CS, CS.getConstraintLocator(locator)));
+
+        return CS.createTypeVariable(CS.getConstraintLocator(repr),
+                                     TVO_CanBindToHole);
       }
       // Diagnose top-level usages of placeholder types.
       if (isa<PlaceholderTypeRepr>(repr->getWithoutParens())) {
@@ -1600,7 +1601,11 @@ namespace {
 
     Type visitUnresolvedSpecializeExpr(UnresolvedSpecializeExpr *expr) {
       auto baseTy = CS.getType(expr->getSubExpr());
-      
+
+      if (baseTy->isTypeVariableOrMember()) {
+        return baseTy;
+      }
+
       // We currently only support explicit specialization of generic types.
       // FIXME: We could support explicit function specialization.
       auto &de = CS.getASTContext().Diags;
@@ -2322,8 +2327,9 @@ namespace {
         }
 
         if (!varType) {
-          varType = CS.createTypeVariable(CS.getConstraintLocator(locator),
-                                          TVO_CanBindToNoEscape);
+          varType =
+              CS.createTypeVariable(CS.getConstraintLocator(locator),
+                                    TVO_CanBindToNoEscape | TVO_CanBindToHole);
 
           // If this is either a `weak` declaration or capture e.g.
           // `weak var ...` or `[weak self]`. Let's wrap type variable

lib/Sema/CSSimplify.cpp

@@ -11211,18 +11211,6 @@ ConstraintSystem::simplifyApplicableFnConstraint(
   // following: $T1 -> $T2.
   auto func1 = type1->castTo<FunctionType>();
 
-  // If a type variable representing "function type" is a hole
-  // or it could be bound to some concrete type with a help of
-  // a fix, let's propagate holes to the "input" type. Doing so
-  // provides more information to upcoming argument and result matching.
-  if (shouldAttemptFixes()) {
-    if (auto *typeVar = type2->getAs<TypeVariableType>()) {
-      auto *locator = typeVar->getImpl().getLocator();
-      if (typeVar->isPlaceholder() || hasFixFor(locator))
-        recordAnyTypeVarAsPotentialHole(func1);
-    }
-  }
-
   // Before stripping lvalue-ness and optional types, save the original second
   // type for handling `func callAsFunction` and `@dynamicCallable`
   // applications. This supports the following cases:
@@ -11237,6 +11225,29 @@ ConstraintSystem::simplifyApplicableFnConstraint(
   type2 = getFixedTypeRecursive(type2, flags, /*wantRValue=*/true);
   auto desugar2 = type2->getDesugaredType();
 
+  // If a type variable representing "function type" is a hole
+  // or it could be bound to some concrete type with a help of
+  // a fix, let's propagate holes to the "input" type. Doing so
+  // provides more information to upcoming argument and result matching.
+  if (shouldAttemptFixes()) {
+    if (auto *typeVar = type2->getAs<TypeVariableType>()) {
+      auto *locator = typeVar->getImpl().getLocator();
+      if (hasFixFor(locator)) {
+        recordAnyTypeVarAsPotentialHole(func1);
+      }
+    }
+    Type underlyingType = desugar2;
+    while (auto *MT = underlyingType->getAs<AnyMetatypeType>()) {
+      underlyingType = MT->getInstanceType();
+    }
+    underlyingType =
+        getFixedTypeRecursive(underlyingType, flags, /*wantRValue=*/true);
+    if (underlyingType->isPlaceholder()) {
+      recordAnyTypeVarAsPotentialHole(func1);
+      return SolutionKind::Solved;
+    }
+  }
+
   TypeMatchOptions subflags = getDefaultDecompositionOptions(flags);
 
   SmallVector<LocatorPathElt, 2> parts;
@@ -12927,6 +12938,9 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::RenameConflictingPatternVariables: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
+  case FixKind::IgnoreInvalidASTNode: {
+    return recordFix(fix, 10) ? SolutionKind::Error : SolutionKind::Solved;
+  }
 
   case FixKind::ExplicitlyConstructRawRepresentable: {
     // Let's increase impact of this fix for binary operators because

test/ClangImporter/MixedSource/can_import_objc_idempotent.swift

@@ -30,7 +30,7 @@
   // expected-error@-1 {{cannot find 'CGRect' in scope}}
 
   let (r, s) = square.divided(atDistance: 50, from: .minXEdge)
-  // expected-error@-1 {{type of expression is ambiguous without more context}}
+  // expected-error@-1 {{cannot infer contextual base in reference to member 'minXEdge'}}
 #endif
 
 #if canImport(MixedWithHeader)

test/Constraints/diagnostics.swift

@@ -1137,7 +1137,7 @@ func badTypes() {
 // rdar://34357545
 func unresolvedTypeExistential() -> Bool {
   return (Int.self==_{})
-  // expected-error@-1 {{type of expression is ambiguous without more context}}
+  // expected-error@-1 {{could not infer type for placeholder}}
   // expected-error@-2 {{type placeholder not allowed here}}
 }
 

test/IDE/complete_repl_identifier_prefix_1.swift

@@ -1,6 +1,7 @@
 // RUN: %target-swift-ide-test -repl-code-completion -source-filename %s | %FileCheck %s
 
 // CHECK: Begin completions
+// CHECK-NEXT: .self: _
 // CHECK-NEXT: {{^}}true: Bool{{$}}
 // CHECK-NEXT: End completions
 

test/Parse/confusables.swift

@@ -11,6 +11,7 @@ let number⁚ Int // expected-note {{operator '⁚' (Two Dot Punctuation) looks
 // expected-error @+1 {{consecutive statements on a line must be separated by ';'}}
 5 ‒ 5 // expected-note {{unicode character '‒' (Figure Dash) looks similar to '-' (Hyphen Minus); did you mean to use '-' (Hyphen Minus)?}} {{3-6=-}}
 
+// expected-error @+3 {{cannot convert value of type '(Bool, _)' to expected condition type 'Bool'}}
 // expected-error @+2 {{cannot find 'ꝸꝸꝸ' in scope}}
 // expected-error @+1 {{expected ',' separator}}
 if (true ꝸꝸꝸ false) {} // expected-note {{identifier 'ꝸꝸꝸ' contains possibly confused characters; did you mean to use '&&&'?}} {{10-19=&&&}}

test/Parse/invalid.swift

@@ -25,7 +25,10 @@ func runAction() {} // expected-note {{'runAction' declared here}}
 
 // rdar://16601779
 func foo() {
-  runAction(SKAction.sequence() // expected-error {{cannot find 'SKAction' in scope; did you mean 'runAction'?}} {{13-21=runAction}} expected-error {{expected ',' separator}} {{32-32=,}}
+  // expected-error@+3 {{argument passed to call that takes no arguments}}
+  // expected-error@+2 {{cannot find 'SKAction' in scope; did you mean 'runAction'?}} {{13-21=runAction}}
+  // expected-error@+1 {{expected ',' separator}} {{32-32=,}}
+  runAction(SKAction.sequence()
 
     skview!
     // expected-error @-1 {{cannot find 'skview' in scope}}

test/Parse/matching_patterns.swift

@@ -329,4 +329,4 @@ case (_?)?: break // expected-warning {{case is already handled by previous patt
 let (responseObject: Int?) = op1
 // expected-error @-1 {{expected ',' separator}} {{25-25=,}}
 // expected-error @-2 {{expected pattern}}
-// expected-error @-3 {{type of expression is ambiguous without more context}}
+// expected-error @-3 {{cannot convert value of type 'Int?' to specified type '(responseObject: _)'}}