Skip to content

Commit 9bce39a

Browse files
rjmccallrjmccall
committed
Implement the conservative option for typechecking multiple trailing closures.
The current approach for type-checking single trailing closures is to scan backwards from the end of the parameter list, looking for something that can be passed a closure. This generalizes that to perform label-matching in reverse on any later trailing closures, then pick up from that point when trying to place the unlabeled trailing closure. The current approach is really not a good language rule, but changing it as much as we'd really like will require evolution approval and a source break, so we have to be cautious.
1 parent a835f01 commit 9bce39a

File tree

2 files changed

+144
-126
lines changed

2 files changed

+144
-126
lines changed

‎lib/Sema/CSSimplify.cpp

Lines changed: 93 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -432,59 +432,114 @@ matchCallArguments(SmallVectorImpl<AnyFunctionType::Param> &args,
432432
haveUnfulfilledParams = true;
433433
};
434434

435-
// If we have a trailing closure, it maps to the last parameter.
436-
// TODO: generalize this correctly for the closure arg index.
437-
if (unlabeledTrailingClosureArgIndex && numParams > 0) {
438-
unsigned lastParamIdx = numParams - 1;
439-
bool lastAcceptsTrailingClosure =
440-
acceptsTrailingClosure(params[lastParamIdx]);
441-
442-
// If the last parameter is defaulted, this might be
443-
// an attempt to use a trailing closure with previous
444-
// parameter that accepts a function type e.g.
445-
//
446-
// func foo(_: () -> Int, _ x: Int = 0) {}
447-
// foo { 42 }
448-
if (!lastAcceptsTrailingClosure && numParams > 1 &&
449-
paramInfo.hasDefaultArgument(lastParamIdx)) {
450-
auto paramType = params[lastParamIdx - 1].getPlainType();
451-
// If the parameter before defaulted last accepts.
452-
if (paramType->is<AnyFunctionType>()) {
453-
lastAcceptsTrailingClosure = true;
454-
lastParamIdx -= 1;
435+
// If we have an unlabeled trailing closure, we match trailing closure
436+
// labels from the end, then match the trailing closure arg.
437+
if (unlabeledTrailingClosureArgIndex) {
438+
unsigned unlabeledArgIdx = *unlabeledTrailingClosureArgIndex;
439+
440+
// One past the next parameter index to look at.
441+
unsigned prevParamIdx = numParams;
442+
443+
// Scan backwards to match any labeled trailing closures.
444+
for (unsigned argIdx = numArgs - 1; argIdx != unlabeledArgIdx; --argIdx) {
445+
bool claimed = false;
446+
447+
// We do this scan on a copy of prevParamIdx so that, if it fails,
448+
// we'll restart at this same point for the next trailing closure.
449+
unsigned prevParamIdxForArg = prevParamIdx;
450+
while (prevParamIdxForArg > 0) {
451+
prevParamIdxForArg -= 1;
452+
unsigned paramIdx = prevParamIdxForArg;
453+
454+
// Check for an exact label match.
455+
const auto &param = params[paramIdx];
456+
if (param.getLabel() == args[argIdx].getLabel()) {
457+
parameterBindings[paramIdx].push_back(argIdx);
458+
claim(param.getLabel(), argIdx);
459+
460+
// Future arguments should search prior to this parameter.
461+
prevParamIdx = prevParamIdxForArg;
462+
claimed = true;
463+
break;
464+
}
465+
}
466+
467+
// If we ran out of parameters, there's no match for this argument.
468+
// TODO: somehow fall through to report out-of-order arguments?
469+
if (!claimed) {
470+
if (listener.extraArgument(argIdx))
471+
return true;
455472
}
456473
}
457474

458-
bool isExtraClosure = false;
459-
// If there is no suitable last parameter to accept the trailing closure,
475+
// Okay, we've matched all the labeled closures; scan backwards from
476+
// there to match the unlabeled trailing closure.
477+
Optional<unsigned> unlabeledParamIdx;
478+
if (prevParamIdx > 0) {
479+
unsigned paramIdx = prevParamIdx - 1;
480+
481+
bool lastAcceptsTrailingClosure =
482+
acceptsTrailingClosure(params[paramIdx]);
483+
484+
// If the last parameter is defaulted, this might be
485+
// an attempt to use a trailing closure with previous
486+
// parameter that accepts a function type e.g.
487+
//
488+
// func foo(_: () -> Int, _ x: Int = 0) {}
489+
// foo { 42 }
490+
//
491+
// FIXME: shouldn't this skip multiple arguments and look for
492+
// something that acceptsTrailingClosure rather than just something
493+
// with a function type?
494+
if (!lastAcceptsTrailingClosure && paramIdx > 0 &&
495+
paramInfo.hasDefaultArgument(paramIdx)) {
496+
auto paramType = params[paramIdx - 1].getPlainType();
497+
// If the parameter before defaulted last accepts.
498+
if (paramType->is<AnyFunctionType>()) {
499+
lastAcceptsTrailingClosure = true;
500+
paramIdx -= 1;
501+
}
502+
}
503+
504+
if (lastAcceptsTrailingClosure)
505+
unlabeledParamIdx = paramIdx;
506+
}
507+
508+
// If there's no suitable last parameter to accept the trailing closure,
460509
// notify the listener and bail if we need to.
461-
if (!lastAcceptsTrailingClosure) {
462-
if (numArgs > numParams) {
510+
if (!unlabeledParamIdx) {
511+
bool isExtraClosure = false;
512+
513+
if (prevParamIdx == 0) {
514+
isExtraClosure = true;
515+
} else if (unlabeledArgIdx > 0) {
463516
// Argument before the trailing closure.
464-
unsigned prevArg = numArgs - 2;
517+
unsigned prevArg = unlabeledArgIdx - 1;
465518
auto &arg = args[prevArg];
466519
// If the argument before trailing closure matches
467520
// last parameter, this is just a special case of
468521
// an extraneous argument.
469-
const auto param = params[numParams - 1];
522+
const auto param = params[prevParamIdx - 1];
470523
if (param.hasLabel() && param.getLabel() == arg.getLabel()) {
471524
isExtraClosure = true;
472-
if (listener.extraArgument(numArgs - 1))
473-
return true;
474525
}
475526
}
476527

477-
if (!isExtraClosure &&
478-
listener.trailingClosureMismatch(lastParamIdx, numArgs - 1))
479-
return true;
480-
}
528+
if (isExtraClosure) {
529+
if (listener.extraArgument(unlabeledArgIdx))
530+
return true;
531+
} else {
532+
if (listener.trailingClosureMismatch(prevParamIdx - 1,
533+
unlabeledArgIdx))
534+
return true;
535+
}
481536

482-
// Claim the parameter/argument pair.
483-
claim(params[lastParamIdx].getLabel(), numArgs - 1,
484-
/*ignoreNameClash=*/true);
485-
// Let's claim the trailing closure unless it's an extra argument.
486-
if (!isExtraClosure)
487-
parameterBindings[lastParamIdx].push_back(numArgs - 1);
537+
} else {
538+
// Claim the parameter/argument pair.
539+
claim(params[*unlabeledParamIdx].getLabel(), unlabeledArgIdx,
540+
/*ignoreNameClash=*/true);
541+
parameterBindings[*unlabeledParamIdx].push_back(unlabeledArgIdx);
542+
}
488543
}
489544

490545
{

‎test/Parse/multiple_trailing_closures.swift

Lines changed: 51 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -2,54 +2,69 @@
22

33
func foo<T, U>(a: () -> T, b: () -> U) {}
44

5-
foo {
6-
a: { 42 }
7-
b: { "" }
8-
}
9-
10-
foo { a: { 42 } b: { "" } }
5+
foo { 42 }
6+
b: { "" }
117

12-
foo {
13-
a: { 42 }, // expected-error {{unexpected ',' separator}} {{12-13=}}
14-
b: { "" }
15-
}
8+
foo { 42 } b: { "" }
169

1710
func when<T>(_ condition: @autoclosure () -> Bool,
1811
`then` trueBranch: () -> T,
1912
`else` falseBranch: () -> T) -> T {
2013
return condition() ? trueBranch() : falseBranch()
2114
}
2215

23-
let _ = when (2 < 3) {
24-
then: { 3 }
25-
else: { 4 }
26-
}
16+
let _ = when (2 < 3) { 3 } else: { 4 }
2717

2818
struct S {
2919
static func foo(a: Int = 42, b: (inout Int) -> Void) -> S {
3020
return S()
3121
}
3222

23+
static func foo(a: Int = 42, ab: () -> Void, b: (inout Int) -> Void) -> S {
24+
return S()
25+
}
26+
3327
subscript(v v: () -> Int) -> Int {
3428
get { return v() }
3529
}
3630

31+
subscript(u u: () -> Int, v v: () -> Int) -> Int {
32+
get { return u() + v() }
33+
}
34+
3735
subscript(cond: Bool, v v: () -> Int) -> Int {
3836
get { return cond ? 0 : v() }
3937
}
38+
subscript(cond: Bool, u u: () -> Int, v v: () -> Int) -> Int {
39+
get { return cond ? u() : v() }
40+
}
4041
}
4142

4243
let _: S = .foo {
43-
b: { $0 = $0 + 1 }
44+
$0 = $0 + 1
4445
}
4546

47+
let _: S = .foo {} b: { $0 = $0 + 1 }
48+
4649
func bar(_ s: S) {
4750
_ = s[] {
48-
v: { 42 }
51+
42
52+
}
53+
54+
_ = s[] {
55+
21
56+
} v: {
57+
42
58+
}
59+
60+
_ = s[true] {
61+
42
4962
}
5063

5164
_ = s[true] {
52-
v: { 42 }
65+
21
66+
} v: {
67+
42
5368
}
5469
}
5570

@@ -58,98 +73,46 @@ func multiple_trailing_with_defaults(
5873
animations: (() -> Void)? = nil,
5974
completion: (() -> Void)? = nil) {}
6075

61-
multiple_trailing_with_defaults(duration: 42) {
62-
animations: {}
63-
}
76+
multiple_trailing_with_defaults(duration: 42) {}
6477

65-
multiple_trailing_with_defaults(duration: 42) {
66-
completion: {}
67-
}
68-
69-
multiple_trailing_with_defaults(duration: 42) {
70-
animations: {}
71-
completion: {}
72-
}
78+
multiple_trailing_with_defaults(duration: 42) {} completion: {}
7379

7480
func test_multiple_trailing_syntax_without_labels() {
75-
func fn(f: () -> Void) {}
81+
func fn(f: () -> Void, g: () -> Void) {}
7682

77-
fn {
78-
_: {} // Ok
79-
}
83+
fn {} g: {} // Ok
8084

81-
fn {
82-
f: {} // Ok
83-
}
85+
fn {} _: {} // expected-error {{extra argument in call}}
8486

8587
func multiple(_: () -> Void, _: () -> Void) {}
8688

87-
multiple {
88-
_: { }
89-
_: { }
90-
}
89+
multiple {} _: { }
9190

9291
func mixed_args_1(a: () -> Void, _: () -> Void) {}
9392
func mixed_args_2(_: () -> Void, a: () -> Void, _: () -> Void) {} // expected-note 2 {{'mixed_args_2(_:a:_:)' declared here}}
9493

95-
mixed_args_1 {
96-
a: {}
94+
mixed_args_1 {}
9795
_: {}
98-
}
9996

100-
mixed_args_1 {
101-
_: {}
102-
a: {} // expected-error {{argument 'a' must precede unnamed argument #1}}
103-
}
97+
// FIXME: not a good diagnostic
98+
mixed_args_1 {} // expected-error {{extra argument in call}}
99+
a: {}
104100

105-
mixed_args_2 {
106-
_: {}
101+
mixed_args_2 {}
107102
a: {}
108103
_: {}
109-
}
110104

111-
mixed_args_2 {
112-
_: {} // expected-error {{missing argument for parameter 'a' in call}}
105+
mixed_args_2
106+
{} // expected-error {{missing argument for parameter #1 in call}}
113107
_: {}
114-
}
115108

116-
mixed_args_2 {
117-
a: {}
109+
// FIXME: not a good diagnostic
110+
mixed_args_2
111+
{} // expected-error {{extra argument in call}}
118112
_: {}
119-
_: {} // expected-error {{unnamed argument #3 must precede argument 'a'}}
120-
}
121-
122-
mixed_args_2 {
123-
a: {} // expected-error {{missing argument for parameter #1 in call}}
124113
_: {}
125-
}
126-
}
127-
128-
foo {
129-
a: { 42 }
130-
b: { 42 }
131114

132-
_ = 1 + 2
133-
// expected-error@-1 {{expected an argument label followed by a closure literal}}
134-
}
135-
136-
foo {
137-
a: { 42 }
138-
b: { 45 }
139-
c:
140-
} // expected-error {{expected a closure literal}}
141-
142-
foo {
143-
a: { 42 }
144-
b: { 45 }
145-
c: 67 // expected-error {{expected a closure literal}}
146-
// expected-error@-1 {{extra argument 'c' in call}}
115+
mixed_args_2
116+
{} // expected-error {{missing argument for parameter #1 in call}}
117+
_: {}
147118
}
148-
149-
foo { // expected-note {{to match this opening '{'}}
150-
a: { 42 }
151-
b: { "" }
152-
153-
func foo() {} // expected-error {{expected an argument label followed by a closure literal}}
154-
// expected-error@-1 {{expected '}' at the end of a trailing closures block}}
155-
} // expected-error {{extraneous '}' at top level}}

0 commit comments

Comments
 (0)