From b7519fabe639b6fb9810120c908c3201058f5ebd Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 14 Sep 2025 11:33:29 +0100 Subject: [PATCH 1/3] [CS] Add `IgnoreInvalidASTNode` fix for invalid generic argument Make sure we record a fix here to ensure we don't try to do CSApply with holes. --- lib/Sema/CSGen.cpp | 2 ++ .../d14a9abe37cc3bca.swift | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/d14a9abe37cc3bca.swift (81%) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 13634185ec393..898e2f4761019 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1981,6 +1981,8 @@ namespace { /*preparedOverload*/ nullptr)); if (result->hasError()) { auto &ctxt = CS.getASTContext(); + CS.recordFix(IgnoreInvalidASTNode::create( + CS, CS.getConstraintLocator(argLocator))); result = PlaceholderType::get(ctxt, specializationArg); ctxt.Diags.diagnose(lAngleLoc, diag::while_parsing_as_left_angle_bracket); diff --git a/validation-test/compiler_crashers_2/d14a9abe37cc3bca.swift b/validation-test/compiler_crashers_2_fixed/d14a9abe37cc3bca.swift similarity index 81% rename from validation-test/compiler_crashers_2/d14a9abe37cc3bca.swift rename to validation-test/compiler_crashers_2_fixed/d14a9abe37cc3bca.swift index 4f82d22e2bf3a..97ea4b9192190 100644 --- a/validation-test/compiler_crashers_2/d14a9abe37cc3bca.swift +++ b/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 From be1f82e36bebe025d17ba47a9c941e3f1e4f4c08 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 14 Sep 2025 11:33:29 +0100 Subject: [PATCH 2/3] [CS] Avoid emitting duplicate note for generic argument diagnostic Since we can run CSGen multiple times, we need to guard the emission of the note on whether the TypeRepr was already marked invalid (i.e whether we already emitted a diagnostic for it). --- lib/Sema/CSGen.cpp | 14 +++++++++----- test/Constraints/generics.swift | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 898e2f4761019..0d1b17a189db8 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1957,6 +1957,8 @@ namespace { void addSpecializationConstraint(ConstraintLocator *locator, Type boundType, SourceLoc lAngleLoc, ArrayRef specializationArgs) { + auto &ctx = CS.getASTContext(); + // Resolve each type. SmallVector 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,19 +1983,20 @@ namespace { OpenGenericTypeRequirements(CS, locator, /*preparedOverload*/ nullptr)); if (result->hasError()) { - auto &ctxt = CS.getASTContext(); + if (!alreadyInvalid) { + ctx.Diags.diagnose(lAngleLoc, + diag::while_parsing_as_left_angle_bracket); + } CS.recordFix(IgnoreInvalidASTNode::create( CS, CS.getConstraintLocator(argLocator))); - result = PlaceholderType::get(ctxt, specializationArg); - ctxt.Diags.diagnose(lAngleLoc, - diag::while_parsing_as_left_angle_bracket); + 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); } diff --git a/test/Constraints/generics.swift b/test/Constraints/generics.swift index 7a9db67ba588d..17e5aa24d879f 100644 --- a/test/Constraints/generics.swift +++ b/test/Constraints/generics.swift @@ -1113,3 +1113,18 @@ func testHolePropagation() { _ = { () -> (S, Int) in return (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}} _ = { () -> (S, Int) in (); return (makeT(), 0) } // expected-error {{type 'R' does not conform to protocol 'P'}} } + +@freestanding(expression) macro overloadedMacro() -> String = #file +@freestanding(expression) macro overloadedMacro(_ 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}} + } +} From 63d63d8c604fa717aa788b3daeed74eca0a5831c Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 14 Sep 2025 11:33:29 +0100 Subject: [PATCH 3/3] [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 a13787aa5227b..d91ed67e180ad 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 f5857b88ddd50..cf239d1280ded 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