SE-0279: Multiple Trailing Closures [Amended]

What do you think of the slight variant suggested by @xwu and @jrose below (amended by me for clarity)? It also simplifies the rules, and would break less existing code:

I just made a run through Xcode doc. The first thing I observed is that multiple-closure is rare, which is expected of pre-SE-0279 era. Nonetheless, I found a few of them. The proposal already mentioned animate function, and Section init. I wanted to add that, currently, there aren't a lot of other functions outside of that. Anyhow, they all generally fall into these categories:

Action & Completion

They are functions of the form:

func doSomething(thatSomething: (Param) -> (), completionHandler: (Result) -> ())
Boring list

There's also a large subset where action is the animation blocks, which can be found in pretty much any animation-related functions in
UIView, UIViewController, UIViewPropertyAnimator, UIViewControllerTransitionCoordinator, UIFocusAnimationCoordinator, UIGraphicsRenderer. An example includes the discussed animate(withDuration:animations:completion:).

This would match @John_McCall's conjecture earlier that functions would have one main closure (action) and a few lesser closures. Note still that some functions in these category have defaulted (action) closures, so it doesn't fit the mould perfectly.

Event Handlers

They are functions that sets a few event handlers together. Most notably in Combine's Publisher functions:

Boring list

These functions fit more to @jrose's concern where none of the closures are mandatory (breakpoint, handleEvents), or all of them are (sink).

Notably, there're also a lot of functions that use one completion handler like so:

doSomething(..., completionHandler: (Succeed?, Error?) -> ())

Arguably this could be split into succeed closure and error closure, but it could also use Result type, or even async-await.

DSL

These are mostly SwiftUI functions, that has one main View closure, and a few accompanying closures. Some also have another View closure for nested contents.

Boring List

Misc

There are two functions I found so far that don't really fit into any category

So I'm inclined to agree that this is more of an exception rather than the norm.


From what I see, an overwhelming majority are animate & completion functions. All other categories become so rare that I did just listed all functions that I found. One caveat is that the API frequency doesn't translate to usage frequency.

Edit: Just added some more... Don't mind me :stuck_out_tongue:

16 Likes

Thanks for taking the time to compile this!

Cheers!

There's one thing I'm concerned with, is that the omitted label is now decided by the caller, not the author. It's actually not obvious that the callers will always start with the first available closure. The callers can do any of these

doSomething {
  // Do that something
} onComplete {
  // Complete
} onFailure {
  // Failure
}

doSomething(self.preset) {
  // Complete
} onFailure {
  // Failure
}

doSomething(self.preset, onComplete: self.genericUpdate) {
  // Failure
}

And I don't think we'd outright reject the latter two. They look like something the proposal would accept, and it can be hard for human to read.

8 Likes

Quick Answers

I like the general direction this proposal is going towards, but I'm hesitant on the details.

It is a long lasting problem that generates a large interest in fixing it among the community. However, I don't think the current proposal is good enough yet for a change to Swift.

Largely, yes, but not completely.

N/A

I did not participate in the review of the previous version. I read the revised proposal in full. I'm participating in the discussion of Renaming Trailing-closure Functions in the Standard Library


Detailed Review

First, questions: How would this deal with parameter ordering? Would it be ok if the trailing closures are rearranged, so long as the labels are correctly paired with the closures? What happens when there are labels by the same name?

Now, some disagreements:

If labelling the first trailing closure were allowed, users would have to evaluate whether to include the label on a case by case basis, which would inevitably lead to linter and style guide rules to prohibit it.

I'm not sold on this argument. Potential future linter and style guide rules shouldn't matter this much to the language itself. Swift allows optional semicolons, and there must be some projects somewhere that mandate semicolon at the end of every statement, but this doesn't mean Swift should enforce no semicolons when statements are on their own lines. Similarly, Swift is indifferent to indentation styles, but different linter and style guides' rules on "spaces vs. tabs" and brace-placements don't matter to the language itself.

Recall: API authors should be naming functions assuming that the argument label of the first trailing closure will be dropped. Swift users aren’t used to seeing function names and argument labels juxtaposed without parenthesis. Many find this spelling unsettling.

I'm not sold on this argument, either. The first part is based on the proposed change to the API design guide. The second part is contradictory to the nature of Swift Evolution itself. Many things in Swift today were not familiar to users before, this proposal included if accepted. Further, what's unsettling should be determined by new users' reaction, not existing users'. In contradiction to the proposal authors' argument, I believe consistency in spelling among trailing closures is more comfortable for both new and existing users. If some of them happen to find it unsettling to have a label for the first trailing closure, they have the option of dropping it.

> The existing trailing-closure feature requires special treatment of the trailing-closure argument by the type checker because of the special power of label omission: the trailing closure can be passed to a parameter that would ordinarily require an argument label. Currently, the special treatment is specific to the final argument. Because this proposal still has an unlabeled trailing-closure argument, we have to generalize that treatment to allow label omission at an intermediate argument.

Why not just get rid of the special treatment entirely? I prefer @michelf's solution. It's elegant and backward compatible.

In general, the forced omission of the first trailing closure label is practically reintroducing a problem that SE-0046 fixed in a slightly different context.


A Possible Digression

Can this proposal be extended to function declarations as well? For a default parameter value that's a large block of code, it should be very helpful to allow it to be trailing.

I should add to my previous post that the same point here applies the other way. If all labels are required off the bat and lived experience shows that eliding the first (or last) label is really valuable, that will also be a straightforward future extension. Moreover, if it can allows us to avoid the "backward scan rule now, but potentially a different source-breaking rule in Swift 6" situation, then that would be all the more preferable.

5 Likes

Note that there's already a backward scan rule for the single-trailing-closure case.

Right, hence the source-breaking part if it's to be changed. The proposal makes it clear that the issue of which argument is referred to by an unlabeled trailing closure is made greatly more challenging when multiple closures can be trailing, and the existing rule is clearly not what we would choose in that circumstance given a blank slate.

Could you elaborate on this point? Michel's proposal is to allow functions to opt in to requiring the use of a label on a trailing closure (and assumes that this proposal is modified to allow all trailing closures to be labeled). It doesn't change the special treatment of unlabeled trailing closures; changing that would not be backward-compatible, as would be adopting Michel's annotation on any existing functions.

Thanks for pointing it out. I was thinking it wrong. I was thinking in terms of having labels being the default, and @implicit for when the labels can be omitted. So, instead of the compiler looking for the function parameter corresponding to the first trailing closure and omitting its label requirement, the compiler can just omit whichever one that's annotated with @implicit. However, this is the opposite to what Michel proposed. Sorry for the confusion.

Please let me know if I'm still wrong.

Let's put SE-0279 aside for a second. The existing rule effectively discourages a class of APIs from being written in the most natural way — to work with trailing-closure syntax, they have to use overloading instead of default arguments. That is something many people would like to fix in the future, but it will require a source-compatibility break, which will need to be separately investigated, proposed, and reviewed.

Nothing we can do in SE-0279 will meaningfully change any of that. Most APIs with defaultable parameters of function type can be called with a single closure argument, defaulting all the others. We cannot change the rule for how such a call is type-checked without breaking source compatibility, which means that API authors will continue to have to use overloading rather than default arguments to express these APIs. We will still have a problem we need to fix.

The degree of implementation challenge is not something we ask the Evolution community for feedback on, but in this case, no, it was not particularly challenging, once we accepted that we couldn't get around the inherent limitations imposed by the existing rule.

1 Like

No worries. Michel did suggest that that would be his preference, but he acknowledged that it (also) wouldn't be backwards-compatible.

I have an additional objection to the reasoning for enforcing no labels for first trailing closures.

the new syntax rules are:

  • The first trailing closure drops its argument label (like today).
  • Subsequent trailing closures require argument labels.

These rules seem to work well in practice, because functions with multiple closure arguments tend to have one that is more primary. Often any additional closure arguments are optional (via default parameter values or overloading), in order to provide progressive disclosure

Even thought there is some truth to that some "functions with multiple closure arguments tend to have one that is more primary", it doesn't justify the rule. The primary parameter doesn't necessarily happen to be the first parameter in a function declaration. And, regardless of that, the primary parameter's value doesn't necessarily happen to be the first trailing closure. This is clearly shown in the proposal's own examples:

ipAddressPublisher
    .sink(receiveCompletion: { completion in
        // handle error
    }) { identity in
        self.hostnames.insert(identity.hostname!)
    }
UIView.animate(withDuration: 0.3, animations: {
    self.view.alpha = 0
}) { _ in
    self.view.removeFromSuperview()
}

The proposal uses the examples above to show that since the first parameters are the primary ones in the functions, the first trailing closures should have their labels omitted. However, the examples above (which will still be allowed if the proposal is accepted) clearly show that not all first trailing closures are the primary parameter values.

1 Like

++

Things like this could be "deprecated" using an @explicit attribute on the completion label:

func animate(withDuration duration: TimeInterval,
             animations: () -> (),
             @explicit completion: (Bool) -> ())

(This is what I was proposing on this other thread regarding single trailing closures.)

It'd would make the completion label mandatory. You'd only have these options for calling this function (or you get a warning):

UIView.animate(withDuration: 0.3) {
   self.view.alpha =0
} completion: {
   self.view.removeFromSuperview()
}
// or:
UIView.animate(withDuration: 0.3, animations: {
   self.view.alpha =0
}, completion: {
   self.view.removeFromSuperview()
})
// or (if we allow the first trailing closure to have a label):
UIView.animate(withDuration: 0.3) animations: {
   self.view.alpha =0
} completion: {
   self.view.removeFromSuperview()
}

So you'd be limited to all the options that actually make sense. ;-)

It'd also solve the Binding(get:set:) problem if you could make @explicit the two parameter labels.

Strictly speaking it's not source-breaking or backward-incompatible to make every label explicit by default if a failure to add the completion label only brings a warning (with a fixit). It'd just be deeply annoying because it'll happen for every call with a trailing closure where the closure parameter has a label... until the corresponding function signature is updated to make the label optional (where it makes sense).

That's probably fine for the standard library functions and I guess the Apple SDKs since they are updated in sync with the compiler. I assume Xcode's migration assistant would help too. Still, it'd be a disturbing change with a lot of trouble for everybody. Keeping the current behavior (optional label for the trailing closure) and adding a flag for when the label is required is easier and probably good enough, even though I think it's less in line with the language's principles.

I also tend to agree it'd be better if the label could not be omitted as a default behavior, but I disagree that the behavior should be different from the single trailing closure case. In particular, if we add mechanism like the @explicit attribute to prevent a label from being omitted as a single closure, it'd be quite awkward that we also had to add an @implicit attribute to allow a parameter label to be omitted in the multiple closure case.

1 Like

Yes, although I must point out that it's not just a matter of whether a label is provided; source compatibility does require the choice of which parameter to pass the closure as to not change.

I’m not referring to the degree of implementation challenge, but the challenge of correctly using and teaching the rule.

1 Like

Well, good API does take a lot of iteration. So it'd be fair to say that old API won't play well with new syntax. Still, how about we start with:

func with(
  duration: Double,
  animate: @escaping () -> Void,
  onComplete: ((Bool) -> Void)? = nil)

view.with(duration: 0.0, animate: defeatEnemy, onComplete: updateScore)
view.with(duration: 0.0, animate: defeatEnemy)
  onComplete: { ... }
view.with(duration: 0.0)
  animate: { ... }
  onComplete: { ... }

I'd say the reason I'm advocating for requiring the first label is that the API guideline doesn't need to play guessing game regarding whether the label is being dropped. I think It's a big plus from design perspective.

4 Likes

I wasn't suggesting changing overload resolution. The compiler can continue matching parameters and overloaded functions exactly as this proposal suggests. Only if it ends up matching a function where the label is not allowed to be implicit, it'd emit a warning accompanied by a fix-it that makes the label explicit.

A parallel with `@discardableResult`

@discardableResult's only effect is that you can omit using the return value and the compiler won't complain. Here we want to be able to omit a label for the first trailing closure and not have the compiler complain. The main question is whether allowing the label to be omitted is opt-in or opt-out in the function declaration. The @discardableResult precedent would say that omissions should be opt-in, but to keep "warning-less" the current behavior for single trailing closures it'd have to be opt-out.

The parallel to @discardableResult goes further since we actually flipped the default behavior for this in Swift 3 with SE-0047. The justification was "Real world situations where it makes sense from a compiler point of view to discard non-Void results are rare." We could do the same and flip to an opt-in model for when a label can be omitted. That's certainly doable and it'd be my preferred behavior, but I'll admit the justification probably isn't there.

3 Likes