SE-0286: Forward Scan for Trailing Closures

Douglas_Gregor · 2020-07-24T05:59:38.103Z

xwu:

Instead, I would suggest the following migration story:

In the next minor version of Swift, implement forward scan and backward scan; where the two "scans" do not match, backward scan "wins" (i.e., all code continues to work as it currently does) but the compiler emits a warning .

(If we had the option of labeling the first trailing closure, then the warning could then be silenced by explicitly labeling, but we don't have that option at least currently.) For now, the warning can be silenced by not using trailing closure notation . This is not necessary convenient to write, but a fix-it can help automate the task. When the fix-it is applied, the code is made future-proof for a future version of Swift.

In Swift 6 (or whatever the next major version is called), forward scan "wins" (unless the compiler is used in a Swift 5 compatibility mode).

When Swift 5 compatibility mode is dropped, so too is backward scan from the codebase.

Thank you for the thoughtful review. The migration story you lay out does indeed maintain source compatibility better than the proposal under review. I've been experimenting with doing both scan directions and warning if the backward scan was chosen. For example, this allows the compiler to handle pick between the two based on what works:

func trailingClosureEitherDirection(
  f: (Int) -> Int = { $0 }, g: (Int, Int) -> Int = { $0 + $1 }
) { }

trailingClosureEitherDirection { -$0 }      // only forward scan works, choose that
trailingClosureEitherDirection { $0 * $1 }  // only backward scan works, choose that

For the latter case, there is a warning with a Fix-It to point out that you're doing something that will break:

warning: backward matching of the unlabeled trailing closure is deprecated; label the argument with 'g' 
         to suppress this warning
trailingClosureEitherDirection { $0 * $1 }
                               ^
                              (g:         )

For functions where both scans produce meaningful results, I biased toward the forward scan (i.e., the model we want in Swift 6), producing the warnings shown in this message.

This approach reduces the source-breaking impact of SE-0286 as written (ModelAssistant and SwiftUI-related failures from the source compatibility suite pass with this change), but doesn't really change the proposal itself---it's in the realm of backward-compatibility tricks we tend to do with any Swift release. However, some programs can still break with this scheme, because some amount of code that type-checked as backward will type-check differently as forward.

Note that nearly the same implementation can be used to bias toward the backward scan, matching your migration story. If we go that route, I would like us to be certain that we won't fall into the same trap that SE-0279 did, where we provided great source compatibility but left behind a feature that didn't work as intended. All of the examples from the proposal work properly when biasing toward the backward scan, albeit for different reasons:

UIView.animate(withDuration:animations:completion:) works because only the forward scan comes up with an answer. This style of API---required closure first, followed by optional ones---fits well with the forward scan.
View.sheet(isPresented:onDismiss:content:) works because the forward scan's heuristic skips onDismiss, so the forward and backward scans produce the same result.
BlockObserver.init(startHandler:produceHandler:finishHandler:) works because the type signatures of the various closure parameters are different enough; especially important is that the first and last closure have a different number of parameters, so the type checker can decide on first vs. last. The same occurs with SwiftUI's TextField.init(_:text:onEditingChanged:onCommit:).

I'm somewhat concerned that we've been getting lucky with that third bullet. Is there a class of APIs out there that will be hard to write properly to deal with the mix of forward and backward scanning rules?

Your suggestion does (necessarily) retain the weakness that the unlabeled trailing closure can move from the "last" closure (when the backward rule applies) to an earlier closure (when one uses multiple trailing closures). However, at least the compiler will produce a warning in the former case.

[EDIT #1: I implemented @xwu's suggestion. It's going through testing now]
[EDIT #2: Toolchains for macOS and Linux are now available to implement @xwu's suggestion]

Doug

Douglas_Gregor · 2020-07-24T19:39:55.081Z

After a bit more thought and experimentation, I've concluded that Xiaodi's proposed revision to SE-0286 is a better direction. It provides better source compatibility that the proposal under review, retains the ability to write APIs that work well with multiple trailing closures, and offers a path to completely eliminating the backward scan in Swift 6. It addresses the concerns raised by @xedin about silently changing the behavior of existing code, by both not changing behavior in Swift < 6 and also warning now about a future behavior change.

To that end, I have opened a pull request with a revision to SE-0286. The forward scan and "heuristic" remain the same, but I've added the backward scan back for Swift < 6. Please take a look and tell me what you think!

Doug

masters3d · 2020-07-24T19:40:32.440Z

Douglas_Gregor:

warning: backward matching of the unlabeled trailing closure is deprecated; label the argument with 'g' 
         to suppress this warning
trailingClosureEitherDirection { $0 * $1 }
                               ^
                              (g:         )

The above fix it will not work for single trailing closures.

error: consecutive statements on a line must be separated by ';'

trailingClosureEitherDirection g:{ $0 * $1 } 
                              ^
                              ;

Douglas_Gregor · 2020-07-24T19:42:19.873Z

The Fix-It is adding parentheses as well as the label, eliminating the use of the trailing closure entirely. Did you try it with the toolchain or are you guessing from the warning output?

Doug

masters3d · 2020-07-24T19:44:17.066Z

Ah no. I didn't try the fix it directly. Just what I though the fix it was telling me to do. The update to the proposal makes this clear though. Thanks! Its great to see.

trailingClosureBothDirections(g: { $0 * $1 })

Jumhyn · 2020-07-24T19:50:16.881Z

Douglas_Gregor:

The forward scan and "heuristic" remain the same, but I've added the backward scan back for Swift < 6.

Why do we still need the heuristic? Shouldn't attempting both the forward scan and the backward scan (and preferring the backward scan in ambiguous cases) be sufficient for source compatibility?

John_McCall · 2020-07-24T19:53:30.473Z

As the SE-0286 review manager, I'd interested in hearing the community's thoughts about whether a second review would be useful here. It seems to me that Doug's revisions are broadly consistent with the feedback we've received already: while people seem to feel that the forward-scan rule is a clear improvement, there is some serious concern about the source-compatibility impact, and so a more conservative approach in the short term might be warranted. When a revision is this close to the community's apparent consensus, the core team sometimes just accepts the revision rather than running a second review. Is there anyone who would provide significantly different feedback given a second review?

Jumhyn · 2020-07-24T19:59:28.220Z

The updates to the proposal seem sufficiently targeted toward (what seems to be) the only major concerns in this thread. I personally would not be bothered by acceptance of the proposal as-amended if the Core Team believes that's what's best given the time constraints involved.

anandabits · 2020-07-24T20:27:57.988Z

I agree, this revision looks good to me.

Douglas_Gregor · 2020-07-24T20:31:15.753Z

Jumhyn:

Why do we still need the heuristic? Shouldn't attempting both the forward scan and the backward scan (and preferring the backward scan in ambiguous cases) be sufficient for source compatibility?

This proposal needs to put us at a good "resting place" both for Swift 5.3+ and Swift 6. The heuristic makes the forward scan work for enough existing APIs that I don't feel like we can simply drop it from Swift 6 without a suitable replacement. For example, I call out this SwiftUI example in the proposal:

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

The intent here is clear: the unlabeled trailing closure is supposed to be content, because the SwiftUI DSL is formulated that way. If we take away the heuristic but don't have some way to indicate SwiftUI's intent, we have a source break without a good path forward for developers. Of course, a proposal along the lines of what's mentioned in "Future directions" of the proposal could absolutely revoke the heuristic from some future Swift version (be it Swift 6 or later), but it needs to do so with full consideration of the source compatibility impact and the need to support API developers. We can't assume that such a proposal will come along and be accepted in time; SE-0286 has to leave the language in a good place for all of the language versions it touches (Swift 5.3+ and Swift 6+).

SwiftUI isn't an outlier here; I saw a number of examples when I did a source compatibility run with the forward+backward scan but without the heuristic (I had the same thought as you).

Doug

Jumhyn · 2020-07-24T20:35:24.879Z

Thanks for elaborating on that Doug. I'm very satisfied with this iteration of the proposal. Excellent work done on meeting such a tight schedule!

xwu · 2020-07-25T03:11:47.505Z

Doug, this is fantastic; thank you for taking my suggestions and experimenting with them. I really appreciate how you've tried out the design with and without the currently proposed heuristic, and that you've targeted the backward scan legacy affordances narrowly to single trailing closures.

I take it this means that the heuristic you propose would remain part of Swift for the foreseeable future; I think this is fine. In fact, I'd say that it's not an unprincipled compatibility measure, but perhaps fits into a bigger desire to align the behavior of arguments with default values with the behavior of overloads that omit the argument entirely; to the extent that the heuristic brings these two closer, it seems defensible on the basis of that principle, if we see fit to articulate such a principle as something desirable.

Douglas_Gregor · 2020-07-26T04:52:53.503Z

xwu:

I take it this means that the heuristic you propose would remain part of Swift for the foreseeable future; I think this is fine. In fact, I'd say that it's not an unprincipled compatibility measure, but perhaps fits into a bigger desire to align the behavior of arguments with default values with the behavior of overloads that omit the argument entirely; to the extent that the heuristic brings these two closer, it seems defensible on the basis of that principle, if we see fit to articulate such a principle as something desirable.

I dropped the "unprincipled" adjective from the revision. It's a reasonable heuristic that's fairly easy to describe and that seems to work quite well in practice, so yes, I'm proposing to keep the heuristic for the foreseeable future.

Doug

jawbroken · 2020-07-26T14:14:50.347Z

This change makes tactical sense, because it deals with one of the grating inconsistencies between function calls with and without trailing closures.

Strategically, I worry that patching over these inconsistencies in a piecemeal fashion is going to lead to the language being left in a local suboptimum, where the design is considered “good enough” and further progress stalls because of source compatibility and other considerations. In the first pitch, where you were staging this change in using attributes, I was going to suggest that you instead just fix this all at once using an attribute (enabling forward scan, callee control of labels, etc.) and either live with that forever or find the resolve to flip the default in future. Fortunately/unfortunately you found a clever way to avoid the attributes here, so that's a less compelling alternative.

So I agree that this is an uphill step, but I worry about where the language is going to end up as a result.

Lantua · 2020-07-28T04:34:54.895Z

The (revised) proposal makes so much sense that I just want to +1.

Though I'm a little confused as to why we need to define "structurally resembles" function types. Is it different from "anything that can use closure"?

Douglas_Gregor · 2020-07-28T18:54:52.104Z

Lantua:

Though I'm a little confused as to why we need to define "structurally resembles" function types. Is it different from "anything that can use closure" ?

A parameter of type Any can accept a closure, but does not "structurally resemble" a function type. I wanted the narrower definition.

Doug

xwu · 2020-07-28T20:07:05.636Z

Mmm, this is interesting, and I’m not sure I’d appreciated that. Currently, Any works with trailing closure syntax:

func f(_ x: Any) -> String { "Hello, \(x)!" }
print(f { "World" })
// “Hello, (Function)!”

Unless there’s a compelling reason, I don’t know that we should break that, and I’m not sure it’s touched on in the proposal.

John_McCall · 2020-07-28T20:20:18.713Z

SE-0286 has been accepted with the modifications mentioned above. Thank you all for your reviews.

John_McCall · 2020-07-28T20:20:22.151Z

Douglas_Gregor · 2020-07-29T00:05:17.252Z

xwu:

Mmm, this is interesting, and I’m not sure I’d appreciated that. Currently, Any works with trailing closure syntax:
func f(_ x: Any) -> String { "Hello, \(x)!" }
print(f { "World" })
// “Hello, (Function)!”
Unless there’s a compelling reason, I don’t know that we should break that, and I’m not sure it’s touched on in the proposal.

From the proposal:

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

so this is still well-formed.

Doug