From 63d63d8c604fa717aa788b3daeed74eca0a5831c Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 14 Sep 2025 11:33:29 +0100 Subject: [PATCH] [CS] Bail from conjunction for non-zero `SK_Hole` Previously we would only do this for `SK_Fix`, do the same for `SK_Hole` since otherwise the score clearing logic for the conjunction could result in losing the hole score. --- lib/Sema/CSStep.cpp | 23 ++++++++++--------- .../2d5e95ea862946ed.swift | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/2d5e95ea862946ed.swift (80%) diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index a13787aa522..d91ed67e180 100644 --- a/lib/Sema/CSStep.cpp +++ b/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 diff --git a/validation-test/compiler_crashers_2/2d5e95ea862946ed.swift b/validation-test/compiler_crashers_2_fixed/2d5e95ea862946ed.swift similarity index 80% rename from validation-test/compiler_crashers_2/2d5e95ea862946ed.swift rename to validation-test/compiler_crashers_2_fixed/2d5e95ea862946ed.swift index f5857b88ddd..cf239d1280d 100644 --- a/validation-test/compiler_crashers_2/2d5e95ea862946ed.swift +++ b/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