SE-0279: Multiple Trailing Closures [Amended]

Evolution Proposal Reviews
1

The second review of SE-0279 — Multiple Trailing Closures begins now and runs through April 24, 2020.

This is the second review, with a modified proposal. The core team has considered the feedback from the first review and believes that:

  • the underlying motivation for the proposal does merit a change to the language to better accommodate multiple trailing closures;
  • the concerns around the original proposed syntax warranted a rethink of the proposed solution.

The proposal authors have a revised proposal that aims to address some of those concerns.

A diff between the previous and latest proposal can be found here.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via email or direct message in the Swift forums).

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?

  • Is the problem being addressed significant enough to warrant a change to Swift?

  • Does this proposal fit well with the feel and direction of Swift?

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ben Cohen
Review Manager

11 Likes
2

I understand how existing syntax encourages removing the argument label from the first trailing closure rather than the final trailing closure, but I'm worried that this puts us at odds with APIs designed for pre-Swift-5 code, which would put the "heaviest" closure last and "light" ones in the middle in order to make existing trailing closure syntax feel natural. I admit I don't have any real-world examples of such API, but there's a reason for that beyond trailing closure syntax: the "heaviest" closure is the one that likely goes on the longest, and you wouldn't want to have to scroll past it to see that there are more arguments.

Section {
  List(…) {
    HStack(…) {
      // may go on for many lines
    }
  }
} header: {
  ...
} footer: {
  ...
}

(sorry for the straw example; I don't do SwiftUI very much.)

On a more nebulous note, it also feels weird to me that all of these are the same call:

foo(bar: { provideBar() }, baz: { provideBaz() }, garply: { provideGarply() })

foo(bar: { provideBar() }, baz: { provideBaz() }) {
  provideGarply()
}

foo(bar: { provideBar() }) {
  provideBaz()
} garply: {
  provideGarply()
}

foo() {
  provideBar()
} baz: {
  provideBaz()
} garply: {
  provideGarply()
}

Which argument label is omitted is completely up to the call site. I've been a proponent of trailing closures for a long time, and pushed back against people who complain about the last argument label being omitted, but this gets to a point that makes me uneasy, though I can't quite pin down why. (Usually I'm the one who says "people can write bad code in any language".)

I'm sure this was already discussed, but I'd be much more comfortable with "all trailing closures take an argument label; if there is exactly one trailing closure, the argument label may be omitted".

foo() bar: {
  provideBar()
} baz: {
  provideBaz()
} garply: {
  provideGarply()
}

Slightly odd-looking? Perhaps. But it keeps the name omission part from being coupled to the ordering part, and I think that's potentially useful when the rule for name omission isn't just "the final argument label".

34 Likes
3

I am still -1 on this change.

When using multiple trailing closure syntax, these APIs are all clear at the point of use, without the need to label the first trailing closure.

I don't think that changing the argument label being dropped from the last trailing closure to the first trailing closure makes much of a difference. If these APIs were clear without the trailing closures' arguments labels, the library author(s) would not have included them.

I also believe that the parentheses add value by clearly designating what code is part of a function call and where it ends. Of course, trailing closures as we have them today already violate this way of thinking — I don't like those either. I want my function calls to start at a (, end at a ), and be "foldable" in between those parentheses.

14 Likes
4

+1. I think this definitely improves the ergonomics of multiple closure call-sites and I find the examples provided in the proposal to read nicely, even if in some cases APIs must be reworked. Reworking of APIs is just a fact of life in a growing language. I’ve certainly written my fair share of API surface area pre-property wrappers that I would write differently now.

Admittedly, I do agree with Jordan that it feels odd that the call site can determine which argument label to drop, but I am not immediately sure I find that to be a detriment to the feature — something that I will have to ponder further.

Yes. It’s kind of a self-made problem that single trailing closure syntax does not always come across perfectly, but, as noted in the proposal, Sinhalese trailing closure syntax is well liked (including by myself) and multiple closures are therefore a very real weakness.

This might be the only point here worth debating but I don’t have any strong reasons to say “no” so I’ll leave it at a “yes.”

N/A

Followed the thread of the first iteration of this proposal and read the revised proposal.

1 Like
5

I am +1 on this proposal, both in terms of the problem to be solved and the solution. I think this is a great progressive expansion on the existing trailing closure syntax that fits together well.

Yes, this is a long term wart on the language that should have been fixed years ago.

Yes, and I think that this version of the proposal is a huge step forward in this respect from the previous version.

N/A. I spent a lot of time on this proposal, reading it in depth and participating in the first review.

-Chris

5 Likes
6

I like this much better then the prior version (which I strongly disliked). I'd probably use this syntax (if Xcode indents it properly).

I do think @jrose has a point about it being weird that the call site decides where the trailing closures start. I think it would be fine to say that trailing closures are “all or nothing”: a call site has to use TC syntax for as many trailing closures as possible, or not at all.

14 Likes
7

+1
I would prefer something along the lines of Jordan's suggestion or even just saying that using this feature means that no labels are omitted.

8

I have a brief question about one of the examples used in the proposal.

I am a bit unclear as to what is meant here. When I look at the example, it seems to me that the code is valid and that the action closure (because it is non-optional) and the onError closure (because it is explicitly labeled) would be invoked. What does the proposal mean when it says that it "will not type-check as expected"? What is going on? Does the proposal mean to say that the example is invalid? Or is there something else I'm missing here?

1 Like
9

I'm convinced this is a problem worth solving and I'm fairly happy with the new proposed syntax, but I agree with Jordan's suggestion that if a call has multiple trailing closures, they should all be required to have argument labels.

The proposal says:

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.

If labeling the first trailing closure was required if there was more than one, the scope of this problem would be reduced somewhat. Furthermore, I'm not sure the fact that style guides might mandate a particular choice is sufficient motivation not to offer the choice at all. For example, many Swift style guides require or prohibit the use of trailing commas in collection literals, but I think most would agree the language shouldn't enforce a single approach. I'm not convinced the situation in this case is significantly different.

I also have some concerns about the future source break this proposal considers:

The behavior of this call can’t be changed without potentially breaking source compatibility. That might be worthwhile to do in order to enable these sorts of APIs and get more consistent type-checking behavior for trailing closures; however, it will need its own proposal, and it will only be feasible under a new source-compatibility mode. We recommend considering this for Swift 6. In the meantime, library designers will have to use overloading to get this effect instead of default arguments.

I'm hesitant to support a proposal which introduces confusing typechecking behavior and would encourage a nontrivial source break in a future version. Again, it seems like requiring the first trailing closure to be labeled would eliminate the need for a backwards scan when matching arguments. Instead, the arguments could be matched as if they were all parenthesized.

3 Likes
10

The existing type-checking rule for trailing closures aggressively matches the argument with the last parameter of function type (more or less). This happens even if that parameter has a default argument and an earlier parameter doesn't — and therefore the call won't type-check because that earlier parameter is left without an argument. Those of us working on the language broadly agree with you that this is a bad and unintuitive result. Unfortunately, we can't change that behavior when there's only one trailing closure without breaking source compatibility. While we could use a new and better rule when there are multiple trailing closures, it would often create a sort of "trap" where programmers write functions like that and then find they don't work when the user drops down to a single closure. For the time being, programmers will have to use overloading to get that effect. We hope to propose a change to the type-checking rule, but it will only be able to take effect in a future language compatibility mode, maybe Swift 6.

4 Likes
11

I think overall this is going in a much better direction compared to the previous iteration.

I also agree that it should require the first closure label. As is, it somewhat signifies that the first closure is more significant compared to others. This may not be true in some context, or that the most significant closure is more appropriately put elsewhere. Like the Section example, imo it would be more intuitive to do

Section()
  header: { ... }
  content: { ... }
  footer: { ...}

even though content is the most significant closure.

7 Likes
12

We may also require the parentheses if things like this:

foo a: { ... }

is of parsing concerned. Though I do not exactly know if that’s the case.

13

It's not more problematic to parse that than it is to parse labeled trailing closures in general.

1 Like
14

I do think that in practice with APIs like this there's almost always a primary closure that's natural to write without a label: the primary completion handler in contrast to the error handler, the main contents in contrast to header/footer contents, or something along those lines. For example, with that Section API, if you didn't provide header/footer contents you'd absolutely expect to just write the contents in an unlabeled trailing closure. It's a bit odd to have to go back and put a label on that closure just because a header or footer is added afterwards; we don't generally expect adding arguments to disturb earlier arguments. So I think there's an abstract appeal to labeling all the closures — certainly it was my first instinct — that might not match reality very well.

It's also worth pointing out that, if lived experience shows that labelling the first closure is really valuable for a class of APIs, that will be a very straightforward future extension.

6 Likes
15

+1

Looking forward to this. I think control flow is what trailing closures need to work best for since that is the original purpose. I think the proposal succeeds in that regard. I think it is a natural way to separate configuration from content. This worked well for XAML. I think it will be great for SwiftUI and other declarative DSLs.

There continue to be ambiguity issues with trailing closures. This solves the immediate needs while kicking the can down the road on the remaining issues. I think it is fine to hold off where possible on syntactic ambiguity until major goals of the language are completed.

Yes. It helps to flesh out the DSL features coming to Swift and is a step toward possibly closing an ambiguity in the language.

Yes. Swift aims to add reactive and declarative programming features where this is a natural fit. This feature naturally extends trailing closures without adding a third convention to the language. It addresses existing uses for trailing closures where multiple closures were awkward and therefore avoided.

I am not aware of another general purpose language that uses multiple trailing closures, however it is reminiscent of XAML which works well to separate content from configuration. It is different then XAML since strict ordering is required. However, I think this proposal is a better fit for Swift.

I read the full revised proposal and the original proposal. I have been using Combine and SwiftUI significantly since they were introduced last year. I feel this is a good solution in those domains.

16

I like that too, but I agree with the proposal that it is too big of a syntax change for Swift 5. That ordering could be added with an overload in the future. In fact you would want both versions since not all lists would have a header.

18

Question to the proposal designer/implementor & compiler devs: is the label colon really necessary in this version/design?

—
Proposal feedback:
Personally I‘m okay-ish with this iteration. It‘s better than the first one even tough still not perfect, but that‘s my personal preference. I can live with most examples from the proposal except the one from SwiftUI, it doesn‘t look much better to me. Again this is also my preference here.

3 Likes
19

Something is required or it will clash with keywords, function calls, and variables. A colon is natural because it looks like a label.

EDIT:
Everyone has their own opinion, but my take on why this might be better for SwiftUI is for two reasons. It separates content from configuration. There are similar DSLs that adopt this method including XAML and to a lesser extent HTML. It helps parentheses matching work. If your parentheses are off the screen, that is too far to easily follow the code. Potentially you could code fold parentheses, but that doesn’t feel natural to me. It feels like I’m folding an expression.

2 Likes
20

The general preference of users for trailing closures, but the lack of a syntax for multiple trailing closures has left API authors in a bind. When it came to closures, we were forced to violate an important API guideline:

  • Prefer to locate parameters with defaults toward the end of the parameter list. Parameters without defaults are usually more essential to the semantics of a method, and provide a stable initial pattern of use where methods are invoked.

The guideline says methods should have a "a stable initial pattern," but because trailing closures were liable to be unlabeled it was important for clarity to prioritize its stability. This is why (I presume) Combine's sink varies like so:

ipAddressPublisher
  .sink { identity in
    self.hostnames.insert(identity.hostname!)
  }

ipAddressPublisher
  .sink(receiveCompletion: { completion in
    // handle error
  }) { identity in
    self.hostnames.insert(identity.hostname!)
  }

However, I don't think we should continue contorting our APIs in this way. It's more consistent with API guidelines (and at least one API author's intuition :sweat_smile:) for the "optional" parameter to come last:

ipAddressPublisher
  .sink { identity in
    self.hostnames.insert(identity.hostname!)
  }

ipAddressPublisher
  .sink { identity in
    self.hostnames.insert(identity.hostname!)
  } receiveCompletion: { completion in
    // handle error
  }

I think UIView.animate(withDuration:animations:completion:) bears this out. It was designed in Objective-C in accordance with (at least the spirit of) the API guideline and with no trailing closure syntax to contort it. It translates naturally to multiple trailing closure syntax:

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

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

You're absolutely right that if we go in this direction, APIs like Combine's sink that fit themselves to a single trailing closure world won't translate well and will need to be tweaked by hand. But I think this is a cost worth paying to bring trailing closure syntax into alignment with a fundamental API guideline.

6 Likes
21

I totally agree and I don’t think this is an issue in practice. Combine could add both ways with an override and deprecate the old way.