[Pitch #2] Forward scan matching for trailing closures

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

2 Likes

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

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

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.

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

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

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

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

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

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

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

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

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

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:).

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

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.

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

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

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

Terms of Service

Privacy Policy

Cookie Policy