Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-2907] 3.0.1p2: @escaping of closure arrays #45501

Closed
swift-ci opened this issue Oct 10, 2016 · 16 comments
Closed

[SR-2907] 3.0.1p2: @escaping of closure arrays #45501

swift-ci opened this issue Oct 10, 2016 · 16 comments

Comments

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Oct 10, 2016

Previous ID SR-2907
Radar rdar://problem/28701872
Original Reporter helge (JIRA User)
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Linux TrustySwift 4.2.0-27-generic #32~14.04.1-Ubuntu SMP Fri Jan 22 15:32:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
Welcome to Swift version 3.0.1 (swift-3.0.1-PREVIEW-2). Type :help for assistance.

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, 3.0Regression
Assignee @milseman
Priority Medium

md5: e9fb703f13cd396fc9d9df95e592e039

Issue Description:

Swift 3.0.1 Preview 2 Regression
The compiler throws an error when arrays or varargs of closures are marked as @escaping:

helge@TrustySwift:~/dev/Swift/tmp$ swift TestIt.swift
TestIt.swift:8:28: error: @escaping attribute may only be used in function parameter position
  func add(callbacks cbs: @escaping Callback...) {
                          ~^~~~~~~~~~~~~~~~

Example which works with Swift 3.0.0:

    public typealias Middleware =
                   ( IncomingMessage, ServerResponse, @escaping Next ) -> Void
    
    public func connect(middleware: @escaping Middleware...) -> Connect {
      let app = Connect()
      for m in middleware { _ = app.use(m) }
      return app
    }

I can remove the @escaping and it compiles just fine with 3.0.1p2, which I think is wrong. And as a matter of fact it breaks compilation with Swift 3.0.0:

/Users/helge/dev/Swift/Noze.io/Sources/connect/Module.swift:21:17: error: invalid conversion from non-escaping function of type '(IncomingMessage, ServerResponse, @escaping (Any...) -> ()) -> ()' to potentially escaping function type 'Middleware' (aka '(IncomingMessage, ServerResponse, @escaping (Any...) -> ()) -> ()')
    _ = app.use(m)
                ^
                  as! Middleware

Summary: @escaping needs to be allowed again on array parameters or vararg parameters.

P.S.: This is made worse by the fact that you can't even workaround it via:

#if swift(>=3.0.1)

but that may be another bug...

@belkadan
Copy link
Contributor

@belkadan belkadan commented Oct 10, 2016

"escaping" is a property of parameters, but in these cases the closure type isn't being used as a parameter—it's a generic argument. Just like you can't put escaping in Box<() -> Void>, it's also not accepted for Optional or Array. So 3.0.1 is the expected behavior.)

(We probably want to consider making an exception for Optional, but that would be a much more drastic source-breaking change, because it would force everyone to re-audit their optional closure parameters.)

@belkadan
Copy link
Contributor

@belkadan belkadan commented Oct 10, 2016

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Oct 10, 2016

Comment by Helge Heß (JIRA)

I think it is quite easy to argue that it is in fact a property of parameters in this case:

func doIt(cbs: @escaping MyCallbackType...)

I can kinda see your point for this:

func doIt(cbs: @escaping [ MyCallbackType ])

This implies @escaping because the types are already part of a collection? Fine for me.

Neither changes the fact that the 3.0.1change is a major source breaking change to Swift 3.0. And that should not be :-)
Particularly because source code can't even safeguard against the issue, see SR-2908.

P.S.: Maybe a solution is to change the hard error into a fix-it-note in this case? (just ignore the @escaping if it is implied ...)

@milseman
Copy link
Mannequin

@milseman milseman mannequin commented Oct 10, 2016

@belkadan
Copy link
Contributor

@belkadan belkadan commented Oct 10, 2016

Actually, reopening because of the source-breaking aspect.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Oct 10, 2016

I don't think we can leave around warnings on backwards-compatibility code either, not without any way to silence warnings. If we care about this case being compatible with 3.0.0, we probably just need to accept both until Swift 4.

@slavapestov
Copy link
Member

@slavapestov slavapestov commented Oct 10, 2016

Hi +helge, we consider the 3.0.0 behavior a bug – the proposal was not implemented correctly. While it is unfortunate that the behavior changed in 3.0.1, the new behavior is more correct and in the spirit of the feature we wanted to implement. Why is it important to maintain source compatibility between 3.0.0 and 3.0.1? Can you just not use 3.0.0?

@milseman
Copy link
Mannequin

@milseman milseman mannequin commented Oct 10, 2016

The problem here is that they can't. They can't support both language versions in a library simultaneously, as neither is valid syntax for the other.

@milseman
Copy link
Mannequin

@milseman milseman mannequin commented Oct 10, 2016

I'm working on a potential fix.

@slavapestov
Copy link
Member

@slavapestov slavapestov commented Oct 10, 2016

Why not just say the library requires 3.0.1?

@milseman
Copy link
Mannequin

@milseman milseman mannequin commented Oct 10, 2016

Because maybe they want to be compatible with 3.0 and have 3.0 users who can migrate to 3.0.1 at their convenience. If it's a library that has users, this would require a coordinated change of the library and all users at the same time, otherwise there's breakage.

I agree that fixing this makes it worse for 3.0.1 and later code, as it gives the false impression that having `@escaping` on a var arg closure changes semantics. This has to be carefully weighed.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Oct 10, 2016

Comment by Myke Olson (JIRA)

@swift-ci create

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Oct 10, 2016

Comment by Helge Heß (JIRA)

I don't want to troll, but I find it pretty hard to discuss the core issue :-> Do you really consider it OK to have source code incompatibilities between a 3.0.0 and 3.0.1 version. I mean there is a 3 in front and the thing being bumped is the sub minor.

It doesn't really matter that much, but my specific issue is that:
a) I want macOS people use the standard Xcode and not some preview release. So that is 3.0.0.
b) but tuxOS people need to use 3.0.1pr2 because libdispatch is badly b0rked in 3.0.0
c) a+b would still be OK for me, but due to SR-2908 I can't even hack around the language difference (because quite frankly `#if swift(=>maj.min)` anticipated that there would be no language differences in subminors :-)

Personally I can also wait for Xcode 8.0.1 and the 3.0.1 Linux release. Though I (and presumably everyone else) was really hoping that this 'you have to use this specific Swift 3 version' madness would end with Swift 3.0.
Right now I'm still telling newcomers to stick to Swift 2.3 because I can't make a 3.0 release yet, even accepting that Linux itself would require a preview version :-)

@slavapestov
Copy link
Member

@slavapestov slavapestov commented Oct 10, 2016

helge (JIRA User) I think you're right, and we should try to maintain compatibility here. @milseman is working on a fix.

@milseman
Copy link
Mannequin

@milseman milseman mannequin commented Oct 11, 2016

#5217 is the PR for the 3.0 release family. There's no guarantee that it will be taken, as that will need to be carefully determined, as it does slightly pessimize the developer experience with 3.0.1 and later.

@milseman
Copy link
Mannequin

@milseman milseman mannequin commented Oct 12, 2016

This got merged into the swift-3 branch.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants