-
Notifications
You must be signed in to change notification settings - Fork 430
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
Swift 4.1 migration #590
Swift 4.1 migration #590
Conversation
The where clauses for |
@andersio yup, done! |
Tested this on beta 2, tests still passing but the regression is still not fixed. |
c79b683
to
b2de826
Compare
I can't believe we need this workaround: 5bcb5e7 🤦♂️ |
I'm seeing an issue with swift 4.1 compability and this PR: Basically.. all of my my 'signal.map { Int($0) }` is now resolving to a SignalProducer with a value that is a closure, and when I click on 'map' it resolves to the new map method from #601 Not sure how it behaves in swift 4.0, but swift 4.1 has issues. Not sure if this is just a bug in 9.3 beta 2. |
Interesting. Any chance you can provide a small test case for that? I can take a look, and maybe add it to the test suite. |
So in swift 4.0 ... optionalFetch will be the expected type Both of the 'optionalFetch1' and 'optionalFetch2' lines compile successfully in swift 4.1 |
What happens if you do:
|
Ah... so this isn't NOT related to swift 4.1. I wrote that code above and got the same warning error on assertion line 'Cast from 'SignalProducer<(UIImage?) -> UIImage?, NoError>' to unrelated type 'SignalProducer<UIImage?, NoError>' always fails' Every line compiles in swift 4.0/swift 4.1. I could just open this as in issue against #601. Since it's not actually realted to 4.1. I just first saw it in my swift 4.1 branch, but I think it's unrelated to swift 4.1 |
Ooooh yup, totally, it's related to #601, bad overload resolution :( Can you open an issue for that? |
What's the current status now that Xcode 9.3 and Swift 4.1 are officially released? |
I'm away for a month and won't be able to continue working on this, somebody else please pick it up from here? 😊 |
The current ReactiveSwift release works with Xcode 9.3 / Swift 4.1. It just has some warnings. |
Interesting. You're right, i've just tested it. It seems to work with Xcode 9.4/Swift 4.1.1 beta, too. Used to fail with older Xcode 9.3 betas. |
…air_lock_t Note: not 100% sure this is the right change.
…l with regression See https://twitter.com/NachoSoto/status/954474333669179392. It lead to an "expression too complex to be solved..." error in `Swift 4.1`.
This seems fixed in `Swift 4.1`.
@ikesyo any plan to release Result with Swift 4.1 soon? |
@andersio Sorry for the delay, Result 4.0 has been released 🚢 |
Do we want to maintain Xcode 9.2 / Swift 4.0.1 compatibility? As-is, this breaks compatibility (from changes I made) but retains some code for Swift <4.1. I'm not sure whether we should rip out the shims or add more in. |
I am for supporting only Swift 4.1 given that we are going to bump the major version anyway. |
Last updated and tested on:
Xcode 9.3 GM
.Checklist
BindingSource
workaroundsSignal.Event
andActionError
Equatable
.flatMap
->.compactMap
Result.materialize
extensionXcode 9.3
Travis
Tag finalCan be done separately.Result
releaseWait for fix to-> Workaround is implemented, doesn't seem like they'll fix the regression..logEvents
regressionSwift 4.0.x
compatibility using#if swift(>=4.1)
UnsafeMutablePointer
deprecation changes (I'm not totally sure about them)