Skip to content

Commit 25d4012

Browse files
DougGregorDougGregor
committed
[Trailing closures] Bias toward the backward scan for ambiguities.
This approach, suggested by Xiaodi Wu, provides better source compatibility for existing Swift code, by breaking ties in favor of the existing Swift semantics. Each time the backward-scan rule is needed (and differs from the forward-scan result), we will produce a warning + Fix-It to prepare for Swift 6 where the backward rule can be removed.
1 parent 1b93736 commit 25d4012

9 files changed

+32
-52
lines changed

‎lib/Sema/CSRanking.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
4848
llvm::errs() << "use of an unavailable declaration";
4949
break;
5050

51-
case SK_BackwardTrailingClosure:
52-
llvm::errs() << "backward scan when matching a trailing closure";
51+
case SK_ForwardTrailingClosure:
52+
llvm::errs() << "forward scan when matching a trailing closure";
5353
break;
5454

5555
case SK_Fix:

‎lib/Sema/CSSimplify.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,13 +1229,13 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
12291229
cs.getConstraintLocator(locator),
12301230
callArgumentMatch->trailingClosureMatching);
12311231

1232-
// If we matched with the backward rule, increase the score for backward
1233-
// matches.
1234-
// FIXME: We shouldn't want this? Wait until later so we can diagnose
1235-
// appropriately.
1236-
if (callArgumentMatch->trailingClosureMatching
1237-
== TrailingClosureMatching::Backward)
1238-
cs.increaseScore(SK_BackwardTrailingClosure);
1232+
// If there was a disjunction because both forward and backward were
1233+
// possible, increase the score for forward matches to bias toward the
1234+
// (source-compatible) backward matches. The compiler will produce a
1235+
// warning for such code.
1236+
if (trailingClosureMatching &&
1237+
*trailingClosureMatching == TrailingClosureMatching::Forward)
1238+
cs.increaseScore(SK_ForwardTrailingClosure);
12391239

12401240
// Take the parameter bindings we selected.
12411241
parameterBindings = std::move(callArgumentMatch->parameterBindings);

‎lib/Sema/ConstraintSystem.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -681,8 +681,8 @@ enum ScoreKind {
681681
SK_Hole,
682682
/// A reference to an @unavailable declaration.
683683
SK_Unavailable,
684-
/// A use of the "backward" scanning heuristic for trailing closures.
685-
SK_BackwardTrailingClosure,
684+
/// A use of the "forward" scan for trailing closures.
685+
SK_ForwardTrailingClosure,
686686
/// A use of a disfavored overload.
687687
SK_DisfavoredOverload,
688688
/// An implicit force of an implicitly unwrapped optional value.

‎test/Constraints/diagnostics.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,9 +1105,7 @@ func rdar17170728() {
11051105
// expected-error@-1{{missing argument label 'into:' in call}}
11061106
// expected-error@-2{{cannot convert value of type 'Int?' to expected argument type}}
11071107
$0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
1108-
// expected-error@-1 2 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
1109-
// expected-error@-2 {{cannot convert value of type 'Bool' to expected argument type 'Int'}}
1110-
// expected-error@-3 {{result values in '? :' expression have mismatching types 'Bool' and 'Int?'}}
1108+
// expected-error@-1 {{binary operator '+' cannot be applied to two 'Bool' operands}}
11111109
}
11121110
}
11131111

‎test/Parse/multiple_trailing_closures.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,12 @@ func bar(_ s: S) {
6868
}
6969
}
7070

71-
func multiple_trailing_with_defaults( // expected-note{{contains defaulted closure parameters 'animations' and 'completion'}}
71+
func multiple_trailing_with_defaults( // expected-note{{declared here}}
7272
duration: Int,
7373
animations: (() -> Void)? = nil,
7474
completion: (() -> Void)? = nil) {}
7575

76-
multiple_trailing_with_defaults(duration: 42) {} // expected-warning{{unlabeled trailing closure argument matches}}
77-
// expected-note@-1{{'completion' to retain}}
78-
// expected-note@-2{{'animations' to silence this}}
76+
multiple_trailing_with_defaults(duration: 42) {} // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'completion' to suppress this warning}}
7977

8078
multiple_trailing_with_defaults(duration: 42) {} completion: {}
8179

‎test/expr/closure/trailing.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ func takeFunc(_ f: (Int) -> Int) -> Int {}
55
func takeValueAndFunc(_ value: Int, _ f: (Int) -> Int) {}
66
func takeTwoFuncs(_ f: (Int) -> Int, _ g: (Int) -> Int) {}
77
func takeFuncWithDefault(f : ((Int) -> Int)? = nil) {}
8-
func takeTwoFuncsWithDefaults(f1 : ((Int) -> Int)? = nil, f2 : ((String) -> String)? = nil) {} // expected-note{{contains defaulted closure parameters 'f1' and 'f2'}}
8+
func takeTwoFuncsWithDefaults(f1 : ((Int) -> Int)? = nil, f2 : ((String) -> String)? = nil) {}
99
// expected-note@-1{{'takeTwoFuncsWithDefaults(f1:f2:)' declared here}}
1010

1111
struct X {
@@ -107,8 +107,7 @@ func labeledArgumentAndTrailingClosure() {
107107

108108
// Trailing closure binds to first parameter... unless only the second matches.
109109
takeTwoFuncsWithDefaults { "Hello, " + $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'f2' to suppress this warning}}
110-
takeTwoFuncsWithDefaults { $0 + 1 } // expected-warning{{rather than}}
111-
// expected-note@-1 2{{label the argument}}
110+
takeTwoFuncsWithDefaults { $0 + 1 }
112111
takeTwoFuncsWithDefaults(f1: {$0 + 1 })
113112
}
114113

‎test/expr/postfix/call/forward_trailing_closure.swift

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %target-typecheck-verify-swift -disable-fuzzy-forward-scan-trailing-closure-matching
22

3-
func forwardMatch1( // expected-note 5 {{contains defaulted}}
3+
func forwardMatch1(
44
a: Int = 0, b: Int = 17, closure1: (Int) -> Int = { $0 }, c: Int = 42,
55
numbers: Int..., closure2: ((Double) -> Double)? = nil,
66
closure3: (String) -> String = { $0 + "!" }
@@ -11,29 +11,24 @@ func testForwardMatch1(i: Int, d: Double, s: String) {
1111
forwardMatch1()
1212

1313
// Matching closure1
14-
forwardMatch1 { _ in i } // expected-warning{{rather than}}
15-
// expected-note@-1 2{{label the argument}}
14+
forwardMatch1 { _ in i }
1615
forwardMatch1 { _ in i } closure2: { d + $0 }
1716
forwardMatch1 { _ in i } closure2: { d + $0 } closure3: { $0 + ", " + s + "!" }
1817
forwardMatch1 { _ in i } closure3: { $0 + ", " + s + "!" }
1918

20-
forwardMatch1(a: 5) { $0 + i } // expected-warning{{rather than}}
21-
// expected-note@-1 2{{label the argument}}
19+
forwardMatch1(a: 5) { $0 + i }
2220
forwardMatch1(a: 5) { $0 + i } closure2: { d + $0 }
2321
forwardMatch1(a: 5) { $0 + i } closure2: { d + $0 } closure3: { $0 + ", " + s + "!" }
2422
forwardMatch1(a: 5) { $0 + i } closure3: { $0 + ", " + s + "!" }
2523

26-
forwardMatch1(b: 1) { $0 + i } // expected-warning{{rather than}}
27-
// expected-note@-1 2{{label the argument}}
24+
forwardMatch1(b: 1) { $0 + i }
2825
forwardMatch1(b: 1) { $0 + i } closure2: { d + $0 }
2926
forwardMatch1(b: 1) { $0 + i } closure2: { d + $0 } closure3: { $0 + ", " + s + "!" }
3027
forwardMatch1(b: 1) { $0 + i } closure3: { $0 + ", " + s + "!" }
3128

3229
// Matching closure2
33-
forwardMatch1(closure1: { $0 + i }) { d + $0 } // expected-warning{{rather than}}
34-
// expected-note@-1 2{{label the argument}}
35-
forwardMatch1(closure1: { $0 + i }, numbers: 1, 2, 3) { d + $0 } // expected-warning{{rather than}}
36-
// expected-note@-1 2{{label the argument}}
30+
forwardMatch1(closure1: { $0 + i }) { d + $0 }
31+
forwardMatch1(closure1: { $0 + i }, numbers: 1, 2, 3) { d + $0 }
3732
forwardMatch1(closure1: { $0 + i }, numbers: 1, 2, 3) { d + $0 } closure3: { $0 + ", " + s + "!" }
3833

3934
// Matching closure3

‎test/expr/postfix/call/forward_trailing_closure_ambiguity.swift

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %target-typecheck-verify-swift
22

33
// Ambiguity when calling.
4-
func ambiguous1( // expected-note 3 {{'ambiguous1(x:a:y:b:z:c:)' contains defaulted closure parameters '}}
4+
func ambiguous1( // expected-note 3 {{declared here}}
55

66
x: (Int) -> Int = { $0 },
77
a: Int = 5,
@@ -12,24 +12,18 @@ func ambiguous1( // expected-note 3 {{'ambiguous1(x:a:y:b:z:c:)' contains defaul
1212
) {}
1313

1414
func testAmbiguous1() {
15-
ambiguous1 { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'x' rather than parameter 'z'}}
16-
// expected-note@-1{{label the argument with 'z' to retain the pre-Swift 5.3 behavior}}{{13-13=(z: }}{{20-20=)}}
17-
// expected-note@-2{{label the argument with 'x' to silence this warning for Swift 5.3 and newer}}{{13-13=(x: }}{{20-20=)}}
15+
ambiguous1 { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'z' to suppress this warning}}{{13-13=(z: }}{{20-20=)}}
1816

19-
ambiguous1() { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'x' rather than parameter 'z'}}
20-
// expected-note@-1{{label the argument with 'z' to retain the pre-Swift 5.3 behavior}}{{14-15=z: }}{{22-22=)}}
21-
// expected-note@-2{{label the argument with 'x' to silence this warning for Swift 5.3 and newer}}{{14-15=x: }}{{22-22=)}}
17+
ambiguous1() { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'z' to suppress this warning}}{{14-15=z: }}{{22-22=)}}
2218

23-
ambiguous1(a: 3) { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'y' rather than parameter 'z'}}
24-
// expected-note@-1{{label the argument with 'z' to retain the pre-Swift 5.3 behavior}}{{18-19=, z: }}{{26-26=)}}
25-
// expected-note@-2{{label the argument with 'y' to silence this warning for Swift 5.3 and newer}}{{18-19=, y: }}{{26-26=)}}
19+
ambiguous1(a: 3) { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'z' to suppress this warning}}{{18-19=, z: }}{{26-26=)}}
2620

2721
// No warning; this is matching the last parameter.
2822
ambiguous1(b: 3) { $0 }
2923
}
3024

3125
// Ambiguity with two unlabeled arguments.
32-
func ambiguous2( // expected-note{{'ambiguous2(_:a:y:b:_:x:)' contains defaulted closure parameters '_' and '_'}}
26+
func ambiguous2( // expected-note{{declared here}}
3327
_: (Int) -> Int = { $0 },
3428
a: Int = 5,
3529
y: (Int) -> Int = { $0 },
@@ -39,11 +33,11 @@ func ambiguous2( // expected-note{{'ambiguous2(_:a:y:b:_:x:)' contains defaulted
3933
) {}
4034

4135
func testAmbiguous2() {
42-
ambiguous2 { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches earlier parameter '_' rather than later parameter with the same name}}
36+
ambiguous2 { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with '_' to suppress this warning}}{{15-15=(}}{{22-22=)}}
4337
}
4438

4539
// Ambiguity with one unlabeled argument.
46-
func ambiguous3( // expected-note{{'ambiguous3(x:a:y:b:_:c:)' contains defaulted closure parameters 'x' and '_'}}
40+
func ambiguous3( // expected-note{{declared here}}
4741
x: (Int) -> Int = { $0 },
4842
a: Int = 5,
4943
y: (Int) -> Int = { $0 },
@@ -53,9 +47,7 @@ func ambiguous3( // expected-note{{'ambiguous3(x:a:y:b:_:c:)' contains defaulted
5347
) {}
5448

5549
func testAmbiguous3() {
56-
ambiguous3 { $0 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'x' rather than parameter '_'}}
57-
// expected-note@-1{{label the argument with '_' to retain the pre-Swift 5.3 behavior}}{{13-13=(}}{{20-20=)}}
58-
// expected-note@-2{{label the argument with 'x' to silence this warning for Swift 5.3 and newer}}{{13-13=(x: }}{{20-20=)}}
50+
ambiguous3 { $0 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with '_' to suppress this warning}}{{13-13=(}}{{20-20=)}}
5951
}
6052

6153
// Not ambiguous because of an arity mismatch that would lead to different

‎test/expr/postfix/call/forward_trailing_closure_fuzzy.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,11 @@ func testTrailingClosureEitherDirection() {
4242
}
4343

4444
// Check that we resolve ambiguities when both directions can be matched.
45-
// expected-note@+1{{'trailingClosureBothDirections(f:g:)' contains defaulted closure parameters 'f' and 'g'}}
45+
// expected-note@+1{{declared here}}
4646
func trailingClosureBothDirections(
4747
f: (Int, Int) -> Int = { $0 + $1 }, g: (Int, Int) -> Int = { $0 - $1 }
4848
) { }
49-
trailingClosureBothDirections { $0 * $1 } // expected-warning{{since Swift 5.3, unlabeled trailing closure argument matches parameter 'f' rather than parameter 'g'}}
50-
// expected-note@-1{{label the argument with 'g' to retain the pre-Swift 5.3 behavior}}
51-
// expected-note@-2{{label the argument with 'f' to silence this warning for Swift 5.3 and newer}}
49+
trailingClosureBothDirections { $0 * $1 } // expected-warning{{backward matching of the unlabeled trailing closure is deprecated; label the argument with 'g' to suppress this warning}}
5250

5351
// Check an amusing quirk of the "backward" rule that allows the order of
5452
// arguments to be swapped.

0 commit comments

Comments
 (0)