From 7c431c9a9a3ffb5cf5021b4996195cd791055b01 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Thu, 5 Jun 2025 05:28:53 +0100 Subject: [PATCH 1/2] [NFC] Make a function static --- include/swift/Sema/ConstraintSystem.h | 6 ------ lib/Sema/CSSimplify.cpp | 7 +++---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index d43ceabc98e50..3b519cd1a9b06 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -6665,12 +6665,6 @@ bool isSIMDOperator(ValueDecl *value); std::string describeGenericType(ValueDecl *GP, bool includeName = false); -/// Whether the given parameter requires an argument. -bool parameterRequiresArgument( - ArrayRef params, - const ParameterListInfo ¶mInfo, - unsigned paramIdx); - } // end namespace swift namespace llvm { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 8aee857dfbd26..d0911f84b3abf 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -279,10 +279,9 @@ static ConstraintSystem::TypeMatchOptions getDefaultDecompositionOptions( } /// Whether the given parameter requires an argument. -bool swift::parameterRequiresArgument( - ArrayRef params, - const ParameterListInfo ¶mInfo, - unsigned paramIdx) { +static bool parameterRequiresArgument(ArrayRef params, + const ParameterListInfo ¶mInfo, + unsigned paramIdx) { return !paramInfo.hasDefaultArgument(paramIdx) && !params[paramIdx].isVariadic(); } From f2e1420a90e72e66509178c4ffd03c72e0ca4a25 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 4 Jun 2025 21:33:26 +0100 Subject: [PATCH 2/2] Sema: Never record argument label mismatches for unlabeled trailing closures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a crash on invalid. The previous logic was causing a label mismatch constraint fix to be recorded for an unlabeled trailing closure argument matching a variadic paramater after a late recovery argument claim in `matchCallArgumentsImpl`, because the recovery claiming skips arguments matching defaulted parameters, but not variadic ones. We may want to reconsider that last part, but currently it regresses the quality of some diagnostics, and this is a targeted fix. The previous behavior is fine because the diagnosis routine associate with the constraint fix (`diagnoseArgumentLabelError`) skips unlabeled trailing closures when tallying labeling issues — *unless* there are no other issues and the tally is zero, which we assert it is not. Fixes rdar://152313388. --- lib/Sema/CSSimplify.cpp | 11 ++++-- lib/Sema/MiscDiagnostics.cpp | 4 ++- test/expr/closure/trailing.swift | 36 +++++++++++++++++++ .../34a73a7299992de3.swift | 3 +- 4 files changed, 49 insertions(+), 5 deletions(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/34a73a7299992de3.swift (52%) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index d0911f84b3abf..454c78c7beade 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -373,8 +373,15 @@ static bool matchCallArgumentsImpl( auto claim = [&](Identifier expectedName, unsigned argIdx, bool ignoreNameClash = false) -> unsigned { // Make sure we can claim this argument. - assert(argIdx != numArgs && "Must have a valid index to claim"); - assert(!claimedArgs[argIdx] && "Argument already claimed"); + ASSERT(argIdx != numArgs && "Must have a valid index to claim"); + ASSERT(!claimedArgs[argIdx] && "Argument already claimed"); + + // Prevent recording of an argument label mismatche for an unlabeled + // trailing closure. An unlabeled trailing closure is necessarily the first + // one and vice versa, per language syntax. + if (unlabeledTrailingClosureArgIndex == argIdx) { + expectedName = Identifier(); + } if (!actualArgNames.empty()) { // We're recording argument names; record this one. diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 95f27ea0984eb..547092d7033ec 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -2701,8 +2701,10 @@ bool swift::diagnoseArgumentLabelError(ASTContext &ctx, } } + ASSERT((numMissing + numExtra + numWrong > 0) && + "Should not call this function with nothing to diagnose"); + // Emit the diagnostic. - assert(numMissing > 0 || numExtra > 0 || numWrong > 0); llvm::SmallString<16> haveBuffer; // note: diagOpt has references to this llvm::SmallString<16> expectedBuffer; // note: diagOpt has references to this diff --git a/test/expr/closure/trailing.swift b/test/expr/closure/trailing.swift index 58b43d5ee4239..4f8591acd2c2c 100644 --- a/test/expr/closure/trailing.swift +++ b/test/expr/closure/trailing.swift @@ -488,3 +488,39 @@ func rdar92521618() { if let _ = { foo {} }() {} guard let _ = { foo {} }() else { return } } + +// Argument matching never binds trailing closure arguments to +// defaulted/variadic parameters of non-function type. +do { + // Trailing closure not considered fulfilled by 'arg'. + // Note: Used to crash. + do { + func variadic(arg: Int...) {} // expected-note@:10 {{'variadic(arg:)' declared here}}{{none}} + func defaulted(arg: Int = 0) {} + + let _ = variadic { return () } + // expected-error@-1:22 {{trailing closure passed to parameter of type 'Int' that does not accept a closure}}{{none}} + let _ = defaulted { return () } + // expected-error@-1:23 {{extra trailing closure passed in call}}{{none}} + } + // Trailing closure considered fulfilled by 'x' instead of 'arg'. + do { + func variadic(arg: Int..., x: String) {} // expected-note@:10 {{'variadic(arg:x:)' declared here}}{{none}} + func defaulted(arg: Int = 0, x: String) {} // expected-note@:10 {{'defaulted(arg:x:)' declared here}}{{none}} + + let _ = variadic { return () } + // expected-error@-1:22 {{trailing closure passed to parameter of type 'String' that does not accept a closure}}{{none}} + let _ = defaulted { return () } + // expected-error@-1:23 {{trailing closure passed to parameter of type 'String' that does not accept a closure}}{{none}} + } + // Trailing closure considered fulfilled by 'arg'; has function type. + do { + func variadic(arg: ((Int) -> Void)...) {} + func defaulted(arg: ((Int) -> Void) = { _ in }) {} + + let _ = variadic { return () } + // expected-error@-1:22 {{contextual type for closure argument list expects 1 argument, which cannot be implicitly ignored}}{{23-23= _ in}} + let _ = defaulted { return () } + // expected-error@-1:23 {{contextual type for closure argument list expects 1 argument, which cannot be implicitly ignored}}{{24-24= _ in}} + } +} diff --git a/validation-test/compiler_crashers_2/34a73a7299992de3.swift b/validation-test/compiler_crashers_2_fixed/34a73a7299992de3.swift similarity index 52% rename from validation-test/compiler_crashers_2/34a73a7299992de3.swift rename to validation-test/compiler_crashers_2_fixed/34a73a7299992de3.swift index 101b8480a0977..562609e28092a 100644 --- a/validation-test/compiler_crashers_2/34a73a7299992de3.swift +++ b/validation-test/compiler_crashers_2_fixed/34a73a7299992de3.swift @@ -1,4 +1,3 @@ // {"signature":"(anonymous namespace)::OpaqueUnderlyingTypeChecker::check()"} -// RUN: not --crash %target-swift-frontend -typecheck %s -// REQUIRES: asserts +// RUN: not %target-swift-frontend -typecheck %s Array {