Merge pull request #84279 from hamishknight/preserve-fatal-score

[CS] Bail from conjunction for non-zero `SK_Hole`

Author: hamishknight

lib/Sema/CSGen.cpp

@@ -1957,6 +1957,8 @@ namespace {
     void addSpecializationConstraint(ConstraintLocator *locator, Type boundType,
                                      SourceLoc lAngleLoc,
                                      ArrayRef<TypeRepr *> specializationArgs) {
+      auto &ctx = CS.getASTContext();
+
       // Resolve each type.
       SmallVector<Type, 2> specializationArgTypes;
       auto options =
@@ -1971,6 +1973,7 @@ namespace {
           options |= TypeResolutionFlags::AllowPackReferences;
           elementEnv = OuterExpansions.back();
         }
+        auto alreadyInvalid = specializationArg->isInvalid();
         auto result = TypeResolution::resolveContextualType(
             specializationArg, CurDC, options,
             // Introduce type variables for unbound generics.
@@ -1980,17 +1983,20 @@ namespace {
             OpenGenericTypeRequirements(CS, locator,
                                         /*preparedOverload*/ nullptr));
         if (result->hasError()) {
-          auto &ctxt = CS.getASTContext();
-          result = PlaceholderType::get(ctxt, specializationArg);
-          ctxt.Diags.diagnose(lAngleLoc,
-                              diag::while_parsing_as_left_angle_bracket);
+          if (!alreadyInvalid) {
+            ctx.Diags.diagnose(lAngleLoc,
+                               diag::while_parsing_as_left_angle_bracket);
+          }
+          CS.recordFix(IgnoreInvalidASTNode::create(
+              CS, CS.getConstraintLocator(argLocator)));
+          result = PlaceholderType::get(ctx, specializationArg);
         }
         specializationArgTypes.push_back(result);
       }
 
       auto constraint = Constraint::create(
           CS, ConstraintKind::ExplicitGenericArguments, boundType,
-          PackType::get(CS.getASTContext(), specializationArgTypes), locator);
+          PackType::get(ctx, specializationArgTypes), locator);
       CS.addUnsolvedConstraint(constraint);
       CS.activateConstraint(constraint);
     }

lib/Sema/CSStep.cpp

@@ -887,23 +887,24 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
     if (Solutions.size() > 1)
       filterSolutions(Solutions, /*minimize=*/true);
 
-    // In diagnostic mode we need to stop a conjunction
-    // but consider it successful if there are:
+    // In diagnostic mode we need to stop a conjunction but consider it
+    // successful if there are:
     //
-    // - More than one solution for this element. Ambiguity
-    //   needs to get propagated back to the outer context
-    //   to be diagnosed.
-    // - A single solution that requires one or more fixes,
-    //   continuing would result in more errors associated
-    //   with the failed element.
+    // - More than one solution for this element. Ambiguity needs to get
+    //   propagated back to the outer context to be diagnosed.
+    // - A single solution that requires one or more fixes or holes, since
+    //   continuing would result in more errors associated with the failed
+    //   element, and we don't preserve scores across elements.
     if (CS.shouldAttemptFixes()) {
       if (Solutions.size() > 1)
         Producer.markExhausted();
 
       if (Solutions.size() == 1) {
         auto score = Solutions.front().getFixedScore();
-        if (score.Data[SK_Fix] > 0 && !CS.isForCodeCompletion())
-          Producer.markExhausted();
+        if (!CS.isForCodeCompletion()) {
+          if (score.Data[SK_Fix] > 0 || score.Data[SK_Hole] > 0)
+            Producer.markExhausted();
+        }
       }
     } else if (Solutions.size() != 1) {
       return failConjunction();
@@ -1053,7 +1054,7 @@ void ConjunctionStep::SolverSnapshot::replaySolution(const Solution &solution) {
 
   // If inference succeeded, we are done.
   auto score = solution.getFixedScore();
-  if (score.Data[SK_Fix] == 0)
+  if (score.Data[SK_Fix] == 0 && score.Data[SK_Hole] == 0)
     return;
 
   // If this conjunction represents a closure and inference

test/Constraints/generics.swift

@@ -1113,3 +1113,18 @@ func testHolePropagation() {
   _ = { () -> (S<R>, Int) in return (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}}
   _ = { () -> (S<R>, Int) in (); return (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}}
 }
+
+@freestanding(expression) macro overloadedMacro<T>() -> String = #file
+@freestanding(expression) macro overloadedMacro<T>(_ x: T = 0) -> String = #file
+
+do {
+  func foo(_ fn: () -> Int) {}
+  func foo(_ fn: () -> String) {}
+
+  // Make sure we only emit a single note here.
+  foo {
+    #overloadedMacro < Undefined >
+    // expected-error@-1 {{cannot find type 'Undefined' in scope}}
+    // expected-note@-2 {{while parsing this '<' as a type parameter bracket}}
+  }
+}

validation-test/compiler_crashers_2/2d5e95ea862946ed.swift renamed to validation-test/compiler_crashers_2_fixed/2d5e95ea862946ed.swift

@@ -1,5 +1,5 @@
 // {"kind":"typecheck","signature":"(anonymous namespace)::ExprWalker::walkToExprPost(swift::Expr*)","signatureAssert":"Assertion failed: (Ptr && \"Cannot dereference a null Type!\"), function operator->"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 func a {
   {
     \ b() a

validation-test/compiler_crashers_2/d14a9abe37cc3bca.swift renamed to validation-test/compiler_crashers_2_fixed/d14a9abe37cc3bca.swift

@@ -1,4 +1,4 @@
 // {"kind":"typecheck","signature":"swift::constraints::SolverTrail::~SolverTrail()","signatureAssert":"Assertion failed: (Changes.empty() && \"Trail corrupted\"), function ~SolverTrail"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 enum a< b { case c(}
          func d< b >(b->a< b >) d(a< e >.c let a