[Pitch #2] Forward scan matching for trailing closures

Evolution Pitches
1

Hi all,

This is the second version of my pitch on forward scan matching for trailing closures. I'm re-pitching based on three significant changes:

  • I've removed all of the attributes that controlled how trailing closures were matched, based on significant and well-justified push-back against adding such attributes as permanent fixtures in the language.
  • We have a lot more data about how much source breakage occurs in practice ("not much"), coupled with a new heuristic that reduces that much further
  • I'm taking a more aggressive stance toward source breakage, suggesting that we break some existing Swift code right now so that we get to the forward-scan model quicker.

Introduction

SE-0279 "Multiple Trailing Closures" 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.

I propose that we replace the backward scan used by the trailing closure matching rules with a forward scan, which is simpler and works better for APIs that support trailing closures (whether single or multiple) as well as default arguments. This is a source-breaking change that affects a small amount of code: I propose that we take the first (smallest) part of this source break immediately, to be finished by a slightly larger source break in Swift 6

To get a feel for the new behavior, it really helps to write some code against it. I implemented this proposal in the Swift compiler, and have built both a Linux and a macOS toolchain that support it. Using this toolchain, you can experiment with the new behavior. Please try it out!

The problem

Several folks noted the downsides of the "backward" matching rule. The rule itself is described in the detailed design section of SE-0279 (search for "backward"), but I'll reiterate its effect briefly here. Let's try to declare the UIView animate(withDuration:animation:completion:) method in the obvious way to make use of SE-0279:

class func animate(
    withDuration duration: TimeInterval, 
    animations: @escaping () -> Void, 
    completion: ((Bool) -> Void)? = nil
)

SE-0279's backward-matching matches the named trailing closure arguments backward, from the last function parameter, then the unnamed trailing closure argument matches the parameter before that. So given the following example (straight from SE-0279):

UIView.animate(withDuration: 0.3) {
  self.view.alpha = 0
} completion: { _ in
  self.view.removeFromSuperview()
}

The completion: trailing closure matches the last parameter, and the unnamed trailing closure matches animations:. The backward rule worked fine here.

However, things fall apart when a single (therefore unnamed) trailing closure is provided to this API:

UIView.animate(withDuration: 0.3) {
  self.view.alpha = 0
}

Now, the backward rule (which SE-0279 retained for source-compatibility reasons) matches the unnamed trailing closure to completion:. The compiler produces an error:

error: missing argument for parameter 'animations' in call
  animate(withDuration: 3.1415) {

Note that the "real" UIView API actually has two different methods---animate(withDuration:animation:completion:) and animate(withDuration:animation:)---where the latter looks like this:

class func animate(
    withDuration duration: TimeInterval, 
    animations: @escaping () -> Void
)

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.

The forward scanning rule

The idea of the forward-scanning 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 simple example, this means the following:

UIView.animate(withDuration: 0.3) {
  self.view.alpha = 0
}
// equivalent to
UIView.animate(withDuration: 0.3, animations: {
  self.view.alpha = 0
})

and

UIView.animate(withDuration: 0.3) {
  self.view.alpha = 0
} completion: { _ in
  self.view.removeFromSuperview()
}
// equivalent to
UIView.animate(withDuration: 0.3, animation: {
    self.view.alpha = 0
  }, completion: { _ in
  self.view.removeFromSuperview()
  })

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.

Note that you can still have the unlabeled trailing closure match a later parameter, by specifying earlier ones:

UIView.animate(withDuration: 0.3, animations: self.doAnimation) { _ in
  self.view.removeFromSuperview()
}
// equivalent to
UIView.animate(withDuration: 0.3, animations: self.doAnimation, completion: _ in
  self.view.removeFromSuperview()
})

Structural resemblance to a function type

When a function parameter has a default argument, the call site can skip mentioning that parameter entirely, and the default argument will be used instead. The matching of arguments to parameters tends to rely on argument labels to determine when a particular parameter has been skipped. For example:

func nameMatchingExample(x: Int = 1, y: Int = 2, z: Int = 3) { }

nameMatchingExample(x: 5) // equivalent to nameMatchingExample(x: 5, y: 2, z: 3)
nameMatchingExample(y: 4) // equivalent to nameMatchingExample(x: 1, y: 4, z: 3)
nameMatchingExample(x: -1, z: -3) // equivalent to nameMatchingExample(x: -1, y: 2, z: -3)

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:

class func animate(
    withDuration duration: TimeInterval = 1.0, 
    animations: @escaping () -> Void, 
    completion: ((Bool) -> Void)? = nil
)

Consider a call:

UIView.animate {
  self.view.alpha = 0
}

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.

I propose that the forward-scanning rule skip 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:

  • The parameter is not inout
  • The adjusted type of the parameter (defined below) is a function type

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 two adjustments:

  • If the parameter is an @autoclosure , use the result type of the parameter's declared (function) type, before performing the second adjustment.
  • Remove all outer "optional" types.

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.

Source compatibility impact

The forward-scanning rule is source-breaking. A run over Swift's source compatibility suite 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:):

func sheet(isPresented: Binding<Bool>, onDismiss: (() -> Void)? = nil, content: @escaping () -> Content) -> some View

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:

sheet(isPresented: $isPresented) { Text("Hello") }

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 proposed rule. However, see below.

Mitigating the source compatibility impact

With the View.sheet(isPresented:onDismiss:content:) API, the forward scan chooses to match the unlabeled trailing closure with onDismiss:, even though 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 we do that. We can turn this into an heuristic to accept more existing code, reducing the source breaking impact of the proposal. Specifically,

  • the parameter that would match the unlabeled trailing closure
    argument has a default argument, and
  • there are parameters after that parameter that require an argument
    (i.e., they are not variadic and do not have a default argument), up until the first parameter whose label matches that of the next trailing closure (if any)

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:, maintaining the existing Swift behavior.

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.

I propose that this heuristic be part of the semantic model for forward scan for now, then be removed in Swift 6 so that only the simpler, more predictable forward scan persists in the language. This approach should bring the immediate source breakage down to an acceptable level for an incremental language update, while leaving the greater (although still manageable) source breakage for the next major language version.

Remaining source compatibility impact

The remaining source compatibility failure in the Swift source compatibility suite, a project called ModelAssistant, is due to this API:

init(startHandler: ((AOperation) -> Void)? = nil, produceHandler: ((AOperation, Foundation.Operation) -> Void)? = nil, finishHandler: ((AOperation, [NSError]) -> Void)? = nil) {
    self.startHandler = startHandler
    self.produceHandler = produceHandler
    self.finishHandler = finishHandler
}

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.

Note that this API interacts poorly with SE-0279 multiple trailing closures, because the unlabeled trailing closure "moves" backwards:

// SE-0279 backward scan behavior
BlockObserver { (operation, errors) in
  print("finishHandler!")
}

// label finishHandler, unlabeled moves "back" to produceHandler

BlockObserver { (aOperation, foundationOperation) in
  print("produceHandler!")
} finishHandler: { (operation, errors) in
  print("finishHandler!")
}

// label produceHandler, unlabeled moves "back" to startHandler
BlockObserver { aOperation in 
  print("startHandler!") {
} produceHandler: (aOperation, foundationOperation) in
  print("produceHandler!")
} finishHandler: { (operation, errors) in
  print("finishHandler!")
}

The forward scan provides a consistent unlabeled trailing closure anchor, and later (labeled) trailing closures can be tacked on:

// Proposed forward scan
BlockObserver { aOperation in 
  print("startHandler!") {
}

// add another
BlockObserver { aOperation in 
  print("startHandler!") {
} produceHandler: (aOperation, foundationOperation) in
  print("produceHandler!")
}

// specify everything 
BlockObserver { aOperation in 
  print("startHandler!") {
} produceHandler: (aOperation, foundationOperation) in
  print("produceHandler!")
} finishHandler: { (operation, errors) in
  print("finishHandler!")
}

// skip the middle one!
BlockObserver { aOperation in 
  print("startHandler!") {
} finishHandler: { (operation, errors) in
  print("finishHandler!")
}

I contend that we cannot get to a usable model for SE-0279 multiple trailing closures without breaking source compatibility with this API.

Workaround via overload sets

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:

init(startHandler: ((AOperation) -> Void)? = nil, produceHandler: ((AOperation, Foundation.Operation) -> Void)? = nil, finishHandler: ((AOperation, [NSError]) -> Void)) {
    self.startHandler = startHandler
    self.produceHandler = produceHandler
    self.finishHandler = finishHandler
}

One can then add overloads to handle other cases, e.g., the zero-argument case:

init() {
  self.init(startHandler: nil, produceHandler: nil, finishHandler:nil)
}

For those counting, one of the other unaccounted-for failures was due to this issue as it manifest's in SwiftUI's TextField initializer:

extension TextField where Label == SwiftUI.Text {
  public init(_ titleKey: SwiftUI.LocalizedStringKey, text: SwiftUI.Binding<Swift.String>, onEditingChanged: @escaping (Swift.Bool) -> Swift.Void = { _ in }, onCommit: @escaping () -> Swift.Void = {})
}

One last bug...

The last known source-compatibility failure is due to a bug in the existing backward scan, which---in certain rare circumstances---actually allows one to get arguments passed out-of-order:

struct X {
  let content: () -> Int
  var optionalInt: Int?
}

// This...
_ = X(optionalInt: 17) { 42 }

// is currently accepted as being equivalent to
_ = X(content: { 42 }, optionalInt: 17)

This code never should have been accepted in the first place, but it breaks under the implementation of the forward scan.

[EDIT #1: Extended the heuristic to account for multiple trailing closures, based on feedback from @Jumhyn.]

Doug

28 Likes
2

Nice design and great changes.
Let forward-scanning rule them all !
Break what it should be broken sooner rather than later.

2 Likes
3

Why do we need to limit the heuristic to calls with a single unlabeled trailing closure? Would it be viable to say if the matched parameter has a default and the call site does not compile when that call parameter is chosen you apply the default and examine the next parameter?

Possible variations of this would limit the failure to compile by examining only whether the type of the first trailing closure matches the type of the parameter and / or examining whether the sequence of labels for subsequent trailing are able to match all of the remaining parameters. If the labels do not match or there would remain parameters with no argument then you would apply the default and try to match the first trailing closure with the next parameter.

So you do not want this syntax:

sheet(isPresented: $isPresented) { Text("Hello") }

to work in the long run? That seems unfortunate to me. From an API design perspective I think this syntax should work and if it doesn’t library authors will have to use overloads rather than default parameters. That would feel like a step backwards to me.

I think this pitch is a good step forward. To be honest, I don’t see the issue with keeping the heuristic and avoiding a larger source break in Swift 6. Do you have examples where it would result in surprising behavior or is the concern about it hypothetical? I don’t understand why if it is acceptable for Swift 5.x we would drop it in Swift 6 and start requiring libraries to use an overload-based design.

1 Like
4

We can do better than a heuristic, and we can improve argument matching for all call sites with default parameters values, not just those involving trailing closures.

Consider the problem of determining “Could this call-site possibly be referencing this function, and if so which arguments go to which parameters?”

Currently, Swift uses a greedy forward scan to match argument labels at the call site with those in the function declaration (and a greedy reverse scan for trailing closures).

However, this means that a function declared like:

func foo(llama l1: Int = 0, llama l2: Int, duck: Int) { ... }

Cannot be called like this:

foo(llama: 2, duck: 4)

Because the first llama argument at the call site will greedily match the l1 parameter, leaving nothing available for l2.

A general algorithm exists which can properly account for defaulted parameters, by matching less greedily.

The problem is usually described in terms of abridgment: “Is book B an abridged copy of book A, which did not remove any of the underlined words from book A?”

Here, book A is a function declaration, book B is a call site, “words” are “argument labels”, and “underlined” means “does not have a default parameter value”.

The algorithmic complexity is proportional to both the number of arguments supplied and the number thereby required to be using default values.

So it is technically quadratic, but in practice functions have very few parameters (eg. 10 is a lot), even fewer with default values, and almost none which repeat the same argument label.

It would be very rare indeed for the matching algorithm to be worse than linear on call sites for any realistic function.

There are a couple of edge cases involving unlabeled trailing closures (which can match any label), and variadic parameters (which can match subsequent empty labels), but those are easily handled.

The point remains, that if we want to eliminate the issue, we can replace the greedy matching with perfect matching, at essentially no cost to compilation speed.

1 Like
5

I'm quite happy with this iteration of the pitch, and am glad we're approaching this from a direction that doesn't end up polluting function signatures with tons of attributes.

When you talk about the "matched parameter" here, are you referring to the matched parameter of the unlabeled/first trailing closure only?

If we decide that the heuristic should live on past Swift 5, I think the rule should definitely take the form you suggest, since we'd otherwise end up in the same situation where adding additional trailing closures can change the parameter matched to a closure:

func trailingClosures(arg1: () -> Void = {}, arg2: () -> Void, arg3: () -> Void = {}) {}
trailingClosures { print("Hello!") } // By the heuristic, this matches arg2
trailingClosures { print("Hello,") } arg3: { print("world!") } // Now, an error!

This inconsistency seems not-ideal (but perhaps acceptable) if it's a temporary mitigation of source breakage. However, if we do want the heuristic to stick around (and I think you've given a compelling argument for why we might, @anandabits) then we should apply it to multiple trailing closures as well.

I think the position of the pitch makes sense if we assume that the heuristic is unacceptable as a long-term citizen due to the complexity it adds to the resulting forward-scan rule. For Swift 5, we have to maintain source compatibility as much as possible, so a more-complex-than-ideal rule is viewed as the lesser of two evils when compared to a more extensive source break. When we move to Swift 6, though, we are no longer constrained by the same compatibility requirements so we are free to have the forward scan procedure adopt the 'ideal' rule.

6

Your suggestion doesn't improve anything for trailing closures, the subject under discussion. It only seems to affect cases where there are repeated argument labels (which is rare) and the defaulted or variadic parameters become before the ones that require an argument, which is against the API design guidelines.

I understand that you're trying to generalize my heuristic to something perfect, but you need to demonstrate use cases when adding complexity.

Humans need to understand the algorithm, too, and a greedy forward-scan algorithm is a lot more direct than a "perfect" one.

Doug

2 Likes
7

I don't think we want this heuristic long-term; the forward-scan rule (without the heuristic) is fairly straightforward to understand. The heuristic is meant to help existing Swift code continue to compile when switching to forward scanning. That's why I'm proposing to remove the heuristic in Swift 6: APIs that don't work with forward scan will need to change or adapt.

I do not consider this path to be technically feasible. If you consider the API from ModelAssistant that I mentioned, we would have to ask "does the call site compile?" up to 3 times for each call site, and "does the call site compiler?" isn't locally computable (for example, you might have to check the closure body to figure it out). This would add another axis of exponentially to the type-check problem, because it's effectively doing the same thing as having multiple overloads, but with a single declaration.

I don't think that API, as declared today, should work in Swift 6: the heuristic is a compatibility hack that complicates a simple model. I do think it would be valuable for it to be possible to declare an API like that one in Swift 6, without needing overloads. Whether the solution is my @noTrailingClosure or your @elidedWhenFirstTrailingClosure or something else, I don't know and don't really want to commit to just yet.

My proposal's approach here might be wrong from an evolution standpoint: I'm saying we should commit ourselves to not having the heuristic in Swift 6, but leave space for the discussion of what should replace it. Alternatively, I could state the heuristic as part of the proposal, with future directions arguing that we should replace it with something else. Perhaps that better conveys that we have an acceptable "resting" place after this proposal, and leave it to later proposals to argue for more source breakage in this area.

Doug

5 Likes
8

I think we can agree that this pitch is a better "resting" place than the one we have right now. IIUC, your desire is to get this change in before Swift 5.3 ships, right? I would support that. We can discuss the longer term changes when there is more time.

2 Likes
9

This is a good point. The extension of the heuristic to cope with multiple trailing closures is straightforward; it basically stops looking for a parameter that "needs" the trailing parameter when it matches the label for the next trailing closure. I went ahead and implemented it here, and borrowed your test case. It puts the proposal at a better resting point.

Yes.

Doug

2 Likes
10

Agree, this is a nice enhancement that should solve the cases I was thinking of in my reply.

Excellent, thank you for continuing to work on this feature. I know it has been a tough one.

3 Likes
11

Yeah, I'm totally on board with this. Thank you for all your effort on working out the details of this feature!

12

Of course it improves things for trailing closures.

We’re talking about an API that takes multiple closure parameters, say with the first having a default value and the second not, being called with a single function argument as a trailing closure.

A pure greedy approach would match the trailing closure to the first parameter, leaving nothing for the second, required, parameter.

Your heuristic would say “Skip the defaulted parameter and use the trailing closure for the required parameter.”

My strategy generalizes that to all parameters, “Make sure all parameters without a default value are matched, and subject to that constraint match as greedily as possible.”

Yes, that is exactly my motivation.

In my experience, humans will look at such an API and say, “Well obviously the argument being supplied must match the parameter with no default value.”

It’s pretty clear to a human that if a function takes N parameters but only M arguments are provided, then those M must cover all of the parameters without default values.

The greedy approach is, therefore, surprising and counterintuitive to humans.

An API like this:

foo(_ x: Int = 0, _ y: Int) { ... }

Being called like this:

foo(3)

Is, to a human reader, obviously correct, with the first parameter using its default value and the second taking the argument provided.

• • •

If you want a simple way to explain it to people, try this:

“Imagine if, instead of one function with some parameters having default values, there were actually several functions, one for every possible way to omit some or all of those defaulted parameters.”

The rule I am proposing works exactly like that. If our foo were actually 2 functions:

foo(_ x: Int, _ y: Int) { ... }

foo(_ y: Int) { foo(0, y) }

Then it’s obvious what calling it with only one argument would do. I am saying the language should work exactly like that, for every function call, without needing the overloads to actually exist.

7 Likes
13

For what one non-SwiftUI engineer's opinion is worth, I think the two closure arguments are in the wrong order for Swift 5.3 anyway—that is, the current design makes you write this:

sheet(isPresented: $isPresented) {
  doThing()
} content: {
  Text("Hello")
}

But it would be more natural to write this:

sheet(isPresented: $isPresented) {
  Text("Hello")
} onDismiss: {
  doThing()
}

And, if the API was redesigned to do so, then omitting the onDismiss: closure would also get the behavior you want.

I suspect that the current API was designed specifically for the limitations of Swift 5.1's trailing closure support; with multiple trailing closures and this proposed change, the rules are now different, and the SwiftUI folks might want to reconsider the parameter order in that light. (If they leave the old methods but remove the default value for onDismiss, this won't cause a source or ABI break, and it could probably be backwards-deployed.) But an Evolution proposal can't change SwiftUI, so this is drifting off-topic.

5 Likes
14

It's "obvious" from the perspective of "this is the only way the call succeeds", but it is not obvious from the call site---foo(3)---that 3 will end up being passed to y. Just because we can make a thing compile doesn't mean it is good to make it compile, because clarity of the result matters.

In my experience, explaining something in terms of "figure out what happens when you make a call to this overload set" is not effective.

Swift API design encourages argument labels. It encourages defaulted parameters to be at the end. Your examples thus far have gone against those guidelines. Why should I want to make more calls to a poorly-designed API succeed in the type checker?

Doug

2 Likes
15

Is this matching done in one pass, with the trailing closure scan picking up where the normal parameter scan left off, or two scans, with the trailing closure scan skipping parameters already matched by the normal parameter scan?

That is, suppose you write:

UIView.animate(withDuration: 0.3, completion: view.removeFromSuperview) {
  self.view.alpha = 0
}

Does the trailing closure match to animations:, or does the compiler treat it as definitely coming after completion: and therefore reject it?

I think your wording here implies that it does the second, but I wonder if the first might be more useful. The little example above seems like a plausible way to use animate(withDuration:animations:completion:).

16

The latter: the compiler rejects it, because Swift long-ago rejected the notion that arguments could come in a different order from the parameters they match.

Doug

3 Likes
17

This scenario is literally the reason for your proposed heuristic.

If the scenario is not worthy of a change, then no change is needed.

If a change is desired, then I propose that treating default parameters as if they represented overloads is the optimal, intuitive, and preferable solution.

18

The scenario I am describing is worthy of a targeted change to address a demonstrating source-compatibility problem. Your generalization lacks additional use cases and has the potential to break existing overload sets by loosening up the matching rules. If you believe strongly in it, you're welcome to come up with a separate proposal.

Doug

19

I like this. Forward scanning is needed to put the most important arguments first which is generally considered Swift best practices. If it is less impactful to do this now than after SwiftUI becomes heavily used I think it makes sense to do this now.

2 Likes
22

I have moved by comments to SE-0286: Forward Scan for Trailing Closures - #9 by xedin