Skip to content

Commit 1d75c33

Browse files
authored
Merge pull request #1165 from DougGregor/forward-scan-trailing-closures
Forward-scan matching for trailing closures
2 parents c34e2cf + b442290 commit 1d75c33

File tree

1 file changed

+311
-0
lines changed

1 file changed

+311
-0
lines changed
Lines changed: 311 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
1+
# Forward-scan matching for trailing closures
2+
3+
* Proposal: [SE-0286](0286-forward-scan-trailing-closures.md)
4+
* Authors: [Doug Gregor](https://github.com/DougGregor)
5+
* Review Manager: [John McCall](https://github.com/rjmccall)
6+
* Status: **Active Review (July 17th...July 27th, 2020)**
7+
* Implementation: [apple/swift#32891](https://github.com/apple/swift/pull/32891)
8+
* Toolchains: [Linux](https://ci.swift.org/job/swift-PR-toolchain-Linux/389//artifact/branch-master/swift-PR-32891-389-ubuntu16.04.tar.gz), [macOS](https://ci.swift.org/job/swift-PR-toolchain-osx/558//artifact/branch-master/swift-PR-32891-558-osx.tar.gz)
9+
* Discussion: [Pitch #2](https://forums.swift.org/t/pitch-2-forward-scan-matching-for-trailing-closures/38491), [Pitch #1](https://forums.swift.org/t/pitch-1-forward-scan-matching-for-trailing-closures-source-breaking/38162)
10+
11+
## Introduction
12+
13+
[SE-0279 "Multiple Trailing Closures"](https://github.com/apple/swift-evolution/blob/master/proposals/0279-multiple-trailing-closures.md) threaded the needle between getting the syntax we wanted for multiple trailing closures without breaking source compatibility. One aspect of that compromise was to extend (rather than replace) the existing rule for matching a trailing closure to a parameter by scanning *backward* from the end of the parameter list.
14+
15+
However, the backward-scan matching rule makes it hard to write good API that uses trailing closures, especially multiple trailing closures. This proposal replaces the backward scan with a forward scan, which is simpler, more in line with normal argument matching in a call, and works better for APIs that support trailing closures (whether single or multiple) and default arguments. This change introduces a *minor source break* for code involving multiple, defaulted closure parameters, but the practical impacts of this source break are low, and in its current state SE-0279 multiple trailing closures aren't very useful.
16+
17+
## Motivation
18+
19+
Several folks noted the downsides of the "backward" matching rule. The rule itself is described in the [detailed design](https://github.com/apple/swift-evolution/blob/master/proposals/0279-multiple-trailing-closures.md#detailed-design) section of SE-0279 (search for "backward"). To understand the problem with the backward rule, let's try to declare the UIView [`animate(withDuration:animations:completion:)`](https://developer.apple.com/documentation/uikit/uiview/1622515-animate) method in the obvious way to make use of SE-0279:
20+
21+
```swift
22+
class func animate(
23+
withDuration duration: TimeInterval,
24+
animations: @escaping () -> Void,
25+
completion: ((Bool) -> Void)? = nil
26+
)
27+
```
28+
29+
SE-0279 matches the named trailing closure arguments backward, matching the last (labeled) trailing closure argument from the back of the parameter list, then proceeding to move to earlier trailing closures and function parameters. Consider the following example (straight from SE-0279):
30+
31+
```swift
32+
UIView.animate(withDuration: 0.3) {
33+
self.view.alpha = 0
34+
} completion: { _ in
35+
self.view.removeFromSuperview()
36+
}
37+
```
38+
39+
The `completion:` trailing closure matches the last parameter, and the unnamed trailing closure matches `animations:`. The backward rule worked fine here.
40+
41+
However, things fall apart when a single (therefore unnamed) trailing closure is provided to this API:
42+
43+
```swift
44+
UIView.animate(withDuration: 0.3) {
45+
self.view.alpha = 0
46+
}
47+
```
48+
49+
Now, the backward rule matches the unnamed trailing closure to `completion:`. The compiler produces an error:
50+
51+
```
52+
error: missing argument for parameter 'animations' in call
53+
animate(withDuration: 0.3) {
54+
```
55+
56+
Note that the "real" UIView API actually has two different methods---`animate(withDuration:animations:completion:)` and `animate(withDuration:animations:)`---where the latter looks like this:
57+
58+
```swift
59+
class func animate(
60+
withDuration duration: TimeInterval,
61+
animations: @escaping () -> Void
62+
)
63+
```
64+
65+
This second overload only has a closure argument, so the backward-matching rule handles the single-trailing-closure case. These overloads exist because these UIView APIs were imported from Objective-C, which does not have default arguments. A new Swift API would not be written this way---except that SE-0279 forces it due to the backward-matching rule.
66+
67+
## Proposed solution
68+
69+
The idea of the forward-scan matching rule is to match trailing closure arguments to parameters in the same forward, left-to-right manner that other arguments are matched to parameters. The unlabeled trailing closure will be matched to the next parameter that is either unlabeled or has a declared type that structurally resembles a function type (defined below). For the example above, this means the following:
70+
71+
```swift
72+
UIView.animate(withDuration: 0.3) {
73+
self.view.alpha = 0
74+
}
75+
// equivalent to
76+
UIView.animate(withDuration: 0.3, animations: {
77+
self.view.alpha = 0
78+
})
79+
```
80+
81+
and
82+
83+
```swift
84+
UIView.animate(withDuration: 0.3) {
85+
self.view.alpha = 0
86+
} completion: { _ in
87+
self.view.removeFromSuperview()
88+
}
89+
// equivalent to
90+
UIView.animate(withDuration: 0.3, animations: {
91+
self.view.alpha = 0
92+
}, completion: { _ in
93+
self.view.removeFromSuperview()
94+
})
95+
```
96+
97+
Note that the unlabeled trailing closure matches `animations:` in both cases; specifying additional trailing closures fills out later parameters but cannot shift the unlabeled trailing closure to an earlier parameter.
98+
99+
Note that you can still have the unlabeled trailing closure match a later parameter, by specifying earlier ones:
100+
101+
```swift
102+
UIView.animate(withDuration: 0.3, animations: self.doAnimation) { _ in
103+
self.view.removeFromSuperview()
104+
}
105+
// equivalent to
106+
UIView.animate(withDuration: 0.3, animations: self.doAnimation, completion: { _ in
107+
self.view.removeFromSuperview()
108+
})
109+
```
110+
111+
This is both a consequence of forward matching and also a necessity for source compatibility.
112+
113+
### Structural resemblance to a function type
114+
115+
When a function parameter does not require an argument (e.g., because it is variadic or has a default argument), the call site can skip mentioning that parameter entirely, and the default will be used instead (e.g., an empty variadic argument or the specified default argument). The matching of arguments to parameters tends to rely on argument labels to determine when a particular parameter has been skipped. For example:
116+
117+
```swift
118+
func nameMatchingExample(x: Int = 1, y: Int = 2, z: Int = 3) { }
119+
120+
nameMatchingExample(x: 5) // equivalent to nameMatchingExample(x: 5, y: 2, z: 3)
121+
nameMatchingExample(y: 4) // equivalent to nameMatchingExample(x: 1, y: 4, z: 3)
122+
nameMatchingExample(x: -1, z: -3) // equivalent to nameMatchingExample(x: -1, y: 2, z: -3)
123+
```
124+
125+
The unlabeled trailing closure ignores the (otherwise required) argument label, which would prevent the use of argument labels for deciding which parameter should be matched with the unlabeled trailing closure. Let's bring that back to the UIView example by adding a default argument to `withDuration:`
126+
127+
```swift
128+
class func animate(
129+
withDuration duration: TimeInterval = 1.0,
130+
animations: @escaping () -> Void,
131+
completion: ((Bool) -> Void)? = nil
132+
)
133+
```
134+
135+
Consider a call:
136+
137+
```swift
138+
UIView.animate {
139+
self.view.alpha = 0
140+
}
141+
```
142+
143+
The first parameter is `withDuration`, but there is no argument in parentheses. Unlabeled trailing closures ignore the parameter name, so without some additional rule, the unlabeled trailing closure would try to match `withDuration:` and this call would be ill-formed.
144+
145+
The forward-scan matching rule skips over any parameters that do not "structurally resemble" a function type. A parameter structurally resembles a function type if both of the following are true:
146+
147+
* The parameter is not `inout`
148+
* The adjusted type of the parameter (defined below) is a function type
149+
150+
The adjusted type of the parameter is the parameter's type as declared in the function, looking through any type aliases whenever they appear, and performing three adjustments:
151+
152+
* If the parameter is an `@autoclosure` , use the result type of the parameter's declared (function) type, before performing the second adjustment.
153+
* If the parameter is variadic, looking at the element type of the (implied) array type.
154+
* Remove all outer "optional" types.
155+
156+
Following this rule, the `withDuration` parameter (a `TimeInterval`) does not resemble a function type. However, `@escaping () -> Void` does, so the unlabeled trailing closure matches `animations`. `@autoclosure () -> ((Int) -> Int)` and `((Int) -> Int)?` would also resemble a function type.
157+
158+
### Mitigating the source compatibility impact
159+
160+
The forward-scanning rule, as described above, is source-breaking. A run over Swift's [source compatibility suite](https://swift.org/source-compatibility/) with this change enabled in all language modes turned up source compatibility breaks in three projects. The first problem occurs with a SwiftUI API [`View.sheet(isPresented:onDismiss:content:)`](https://developer.apple.com/documentation/swiftui/view/sheet(ispresented:ondismiss:content:)):
161+
162+
```swift
163+
func sheet(
164+
isPresented: Binding<Bool>,
165+
onDismiss: (() -> Void)? = nil,
166+
content: @escaping () -> Content
167+
) -> some View
168+
```
169+
170+
Note that `onDismiss` and `content` both structurally resemble a function type. This API fits well with the backward-matching rule, because the unlabeled trailing closure in the following example is always ascribed to `content:`. The `onDismiss:` argument gets the default argument `nil`:
171+
172+
```swift
173+
sheet(isPresented: $isPresented) { Text("Hello") }
174+
```
175+
176+
With the forward-scanning rule, the unlabeled trailing closure matches the `onDismiss:` parameter, and there is no suitable argument for `content:`. Therefore, the well-formed code above would be rejected by the rule as proposed above.
177+
178+
However, it is clear from the function signature that (1) `onDismiss:` could have used the default argument, and (2) `content:` therefore won't have an argument if it is not paired with the unlabeled trailing closure. We can turn this into an heuristic to accept more existing code, reducing the source breaking impact of the proposal. Specifically, if
179+
180+
* the parameter that would match the unlabeled trailing closure
181+
argument does not require an argument (because it is variadic or has a default argument), and
182+
* there are parameters *after* that parameter that require an argument, up until the first parameter whose label matches that of the *next* trailing closure (if any)
183+
184+
then do not match the unlabeled trailing closure to that parameter. Instead, skip it and examine the next parameter to see if that should be matched against the unlabeled trailing closure. For the `View.sheet(isPresented:onDismiss:content:)` API, this means that `onDismiss`, which has a default argument, will be skipped in the forward match so that the unlabeled trailing closure will match `content:`, allowing this code to continue to compile correctly.
185+
186+
This heuristic is remarkably effective: in addition to fixing 2 of the 3 failures from the Swift source compatibility suite (the remaining failure will be discussed below), it resolved 17 of the 19 failures we observed in a separate (larger) testsuite comprising a couple of million lines of Swift.
187+
188+
## Source compatibility
189+
190+
Even with the heuristic, the forward-scan matching rule will still fail to compile some existing code, and can change the meaning of some code, when there are multiple, defaulted parameters of closure type. As an example, the remaining source compatibility failure in the Swift source compatibility suite, a project called [ModelAssistant](https://github.com/ssamadgh/ModelAssistant), is due to [this API](https://github.com/ssamadgh/ModelAssistant/blob/c96335280a3aba5f8e14955ecaf38dc25a0872b6/Source/Libraries/AOperation/Observers/BlockObserver.swift#L22-L26):
191+
192+
```swift
193+
init(
194+
startHandler: ((AOperation) -> Void)? = nil,
195+
produceHandler: ((AOperation, Foundation.Operation) -> Void)? = nil,
196+
finishHandler: ((AOperation, [NSError]) -> Void)? = nil
197+
) {
198+
self.startHandler = startHandler
199+
self.produceHandler = produceHandler
200+
self.finishHandler = finishHandler
201+
}
202+
```
203+
204+
Note that this API takes three closure parameters. The (existing) backward scan will match `finishHandler:`, while the forward scan will match `startHandler:`. The heuristic described in the previous section does not apply, because all of the closure parameters have default arguments. Existing code that uses trailing closures with this API will break.
205+
206+
Note that this API interacts poorly with SE-0279 multiple trailing closures, because the unlabeled trailing closure "moves" backwards as additional trailing closures are provided at the call site:
207+
208+
```swift
209+
// SE-0279 backward scan behavior
210+
BlockObserver { (operation, errors) in
211+
print("finishHandler!")
212+
}
213+
214+
// label finishHandler, unlabeled moves "back" to produceHandler
215+
216+
BlockObserver { (aOperation, foundationOperation) in
217+
print("produceHandler!")
218+
} finishHandler: { (operation, errors) in
219+
print("finishHandler!")
220+
}
221+
222+
// label produceHandler, unlabeled moves "back" to startHandler
223+
BlockObserver { aOperation in
224+
print("startHandler!")
225+
} produceHandler: { (aOperation, foundationOperation) in
226+
print("produceHandler!")
227+
} finishHandler: { (operation, errors) in
228+
print("finishHandler!")
229+
}
230+
```
231+
232+
The forward scan provides a consistent unlabeled trailing closure anchor, and later (labeled) trailing closures can be tacked on:
233+
234+
```swift
235+
// Proposed forward scan
236+
BlockObserver { aOperation in
237+
print("startHandler!") {
238+
}
239+
240+
// add another
241+
BlockObserver { aOperation in
242+
print("startHandler!")
243+
} produceHandler: { (aOperation, foundationOperation) in
244+
print("produceHandler!")
245+
}
246+
247+
// specify everything
248+
BlockObserver { aOperation in
249+
print("startHandler!")
250+
} produceHandler: { (aOperation, foundationOperation) in
251+
print("produceHandler!")
252+
} finishHandler: { (operation, errors) in
253+
print("finishHandler!")
254+
}
255+
256+
// skip the middle one!
257+
BlockObserver { aOperation in
258+
print("startHandler!")
259+
} finishHandler: { (operation, errors) in
260+
print("finishHandler!")
261+
}
262+
```
263+
264+
The forward-scan matching rule provides more predictable results, making it easier to understand how to use this API properly.
265+
266+
### Workaround via overload sets
267+
268+
APIs like the above that will be broken by the forward scan can be "fixed" by removing the default arguments, then adding additional overloads to create the same effect. For example, drop the default argument of `finishHandler` so that the heuristic will kick in to fix calls with a single unlabeled trailing closure:
269+
270+
```swift
271+
init(
272+
startHandler: ((AOperation) -> Void)? = nil,
273+
produceHandler: ((AOperation, Foundation.Operation) -> Void)? = nil,
274+
finishHandler: ((AOperation, [NSError]) -> Void)?
275+
) {
276+
self.startHandler = startHandler
277+
self.produceHandler = produceHandler
278+
self.finishHandler = finishHandler
279+
}
280+
```
281+
282+
One can then add overloads to handle other cases, e.g., the zero-argument case:
283+
284+
```swift
285+
init() {
286+
self.init(startHandler: nil, produceHandler: nil, finishHandler: nil)
287+
}
288+
```
289+
290+
An investigation into APIs with multiple trailing closures uncovered one SwiftUI API that will break in this manner, because both closure parameters have defaults:
291+
292+
```swift
293+
extension TextField where Label == Text {
294+
public init(
295+
_ titleKey: LocalizedStringKey,
296+
text: Binding<String>,
297+
onEditingChanged: @escaping (Bool) -> Void = { _ in },
298+
onCommit: @escaping () -> Void = {}
299+
)
300+
}
301+
```
302+
303+
## Future directions
304+
305+
The heuristic that skips matching the unnamed trailing closure argument to a parameter that doesn't require an argument when the unnamed trailing closure is needed to match a later parameter is an accommodation for source compatibility, not a principled design. For a future language version that can accept more source breakage, we could remove this heuristic---leaving the forward scan in place---and find a better way to express APIs such as `View.sheet(isPresented:onDismiss:content:)` in the language. Possibilities include (but are not limited to):
306+
307+
* A parameter attribute `@noTrailingClosure` that prevents the use of trailing closure syntax for a given parameter entirely.
308+
* Eliminating the allowance for matching the first (unlabeled) trailing closure to a parameter that has an argument label, so normal argument matching rules would apply.
309+
* Allowing an argument label on the first trailing closure to let the caller select which parameter to match explicitly.
310+
311+
This proposal leaves open all of these possibilities, and makes their changes less drastic because each of the ideas involves moving to a forward-scan matching rule. As such, this proposal makes SE-0279's multiple trailing closures immediately useful with minimal source breakage, paving the way for more significant changes if we so choose in the future.

0 commit comments

Comments
 (0)