Skip to content

Commit f2e1420

Browse files
committed
Sema: Never record argument label mismatches for unlabeled trailing closures
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.
1 parent 7c431c9 commit f2e1420

File tree

4 files changed

+49
-5
lines changed

4 files changed

+49
-5
lines changed

lib/Sema/CSSimplify.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,15 @@ static bool matchCallArgumentsImpl(
373373
auto claim = [&](Identifier expectedName, unsigned argIdx,
374374
bool ignoreNameClash = false) -> unsigned {
375375
// Make sure we can claim this argument.
376-
assert(argIdx != numArgs && "Must have a valid index to claim");
377-
assert(!claimedArgs[argIdx] && "Argument already claimed");
376+
ASSERT(argIdx != numArgs && "Must have a valid index to claim");
377+
ASSERT(!claimedArgs[argIdx] && "Argument already claimed");
378+
379+
// Prevent recording of an argument label mismatche for an unlabeled
380+
// trailing closure. An unlabeled trailing closure is necessarily the first
381+
// one and vice versa, per language syntax.
382+
if (unlabeledTrailingClosureArgIndex == argIdx) {
383+
expectedName = Identifier();
384+
}
378385

379386
if (!actualArgNames.empty()) {
380387
// We're recording argument names; record this one.

lib/Sema/MiscDiagnostics.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2701,8 +2701,10 @@ bool swift::diagnoseArgumentLabelError(ASTContext &ctx,
27012701
}
27022702
}
27032703

2704+
ASSERT((numMissing + numExtra + numWrong > 0) &&
2705+
"Should not call this function with nothing to diagnose");
2706+
27042707
// Emit the diagnostic.
2705-
assert(numMissing > 0 || numExtra > 0 || numWrong > 0);
27062708
llvm::SmallString<16> haveBuffer; // note: diagOpt has references to this
27072709
llvm::SmallString<16> expectedBuffer; // note: diagOpt has references to this
27082710

test/expr/closure/trailing.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,3 +488,39 @@ func rdar92521618() {
488488
if let _ = { foo {} }() {}
489489
guard let _ = { foo {} }() else { return }
490490
}
491+
492+
// Argument matching never binds trailing closure arguments to
493+
// defaulted/variadic parameters of non-function type.
494+
do {
495+
// Trailing closure not considered fulfilled by 'arg'.
496+
// Note: Used to crash.
497+
do {
498+
func variadic(arg: Int...) {} // expected-note@:10 {{'variadic(arg:)' declared here}}{{none}}
499+
func defaulted(arg: Int = 0) {}
500+
501+
let _ = variadic { return () }
502+
// expected-error@-1:22 {{trailing closure passed to parameter of type 'Int' that does not accept a closure}}{{none}}
503+
let _ = defaulted { return () }
504+
// expected-error@-1:23 {{extra trailing closure passed in call}}{{none}}
505+
}
506+
// Trailing closure considered fulfilled by 'x' instead of 'arg'.
507+
do {
508+
func variadic(arg: Int..., x: String) {} // expected-note@:10 {{'variadic(arg:x:)' declared here}}{{none}}
509+
func defaulted(arg: Int = 0, x: String) {} // expected-note@:10 {{'defaulted(arg:x:)' declared here}}{{none}}
510+
511+
let _ = variadic { return () }
512+
// expected-error@-1:22 {{trailing closure passed to parameter of type 'String' that does not accept a closure}}{{none}}
513+
let _ = defaulted { return () }
514+
// expected-error@-1:23 {{trailing closure passed to parameter of type 'String' that does not accept a closure}}{{none}}
515+
}
516+
// Trailing closure considered fulfilled by 'arg'; has function type.
517+
do {
518+
func variadic(arg: ((Int) -> Void)...) {}
519+
func defaulted(arg: ((Int) -> Void) = { _ in }) {}
520+
521+
let _ = variadic { return () }
522+
// expected-error@-1:22 {{contextual type for closure argument list expects 1 argument, which cannot be implicitly ignored}}{{23-23= _ in}}
523+
let _ = defaulted { return () }
524+
// expected-error@-1:23 {{contextual type for closure argument list expects 1 argument, which cannot be implicitly ignored}}{{24-24= _ in}}
525+
}
526+
}
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
// {"signature":"(anonymous namespace)::OpaqueUnderlyingTypeChecker::check()"}
2-
// RUN: not --crash %target-swift-frontend -typecheck %s
3-
// REQUIRES: asserts
2+
// RUN: not %target-swift-frontend -typecheck %s
43
Array {

0 commit comments

Comments
 (0)