Swift 3 Support #62
Swift 3 Support #62
Conversation
All tests pass except for "should launch a task with standard input", which currently crashes.
| @@ -73,6 +73,7 @@ | |||
| useCustomWorkingDirectory = "NO" | |||
| ignoresPersistentStateOnLaunch = "NO" | |||
| debugDocumentVersioning = "YES" | |||
| enableThreadSanitizer = "YES" | |||
There was a problem hiding this comment.
Did you mean to leave this on?
…e Swift 3 migrator moving `@noescape` attributes. Abbreviate `String.Encoding.utf8` to `.utf8`. Replace one remaining use of `NSString(data:encoding:)` with `String(data:encoding:)`. Disable Thread Sanitizer.
|
Thanks for the feedback! All comments should be covered by the latest commit. |
|
Is this still a WIP? |
|
One of the tests is still crashing somewhere deep inside ReactiveCocoa, so I probably need to figure that out before it's ready to merge. |
| @@ -215,7 +215,7 @@ private final class Pipe { | |||
| let bytes = UnsafeMutablePointer<UInt8>(allocatingCapacity: data.count) | |||
| data.copyBytes(to: bytes, count: data.count) | |||
| let buffer = UnsafeBufferPointer(start: bytes, count: data.count) | |||
| let dispatchData = DispatchData(bytes: buffer) | |||
| let dispatchData = DispatchData(bytesNoCopy: buffer, deallocator: .custom(nil, {})) | |||
There was a problem hiding this comment.
This fixes the crashing test, but it feels to me like a non-ideal thing to have to do. I mean it seems to work, I guess because dispatchData only gets used while data (which owns the bytes) exists.
It would be preferable to create dispatchData by copying the bytes in data, but doing so causes an EXC_BAD_ACCESS when dispatchData gets deallocated.
|
I managed to get the final test to pass, though I don't feel great about the fix (see line note here). A more experienced set of eyes might be able to come up with something better. |
| CURRENT_PROJECT_VERSION = 1; | ||
| ENABLE_TESTABILITY = YES; | ||
| GCC_NO_COMMON_BLOCKS = YES; | ||
| MACOSX_DEPLOYMENT_TARGET = 10.9; | ||
| MACOSX_DEPLOYMENT_TARGET = 10.10; |
There was a problem hiding this comment.
The RAC target was switched back to 10.9. Can we keep this at 10.9 too?
|
Just a couple changes, then this looks good. 👍🏻 Also, the Xcode 7 build should be dropped from |
Replace manual pointer creation with NSData.withUnsafeBytes. Remove Xcode 7 build from .travis.yml.
|
Thanks for the feedback! I've made the changes as request. I've also updated the Cartfile to point at ReactiveCocoa's master branch since that's where RAC5 development seems to be now. |
| dispatch_io_write(channel, 0, dispatchData, self.queue) { (done, data, error) in | ||
| let dispatchData = data.withUnsafeBytes { (bytes: UnsafePointer<UInt8>) -> DispatchData in | ||
| let buffer = UnsafeBufferPointer(start: bytes, count: data.count) | ||
| return DispatchData(bytesNoCopy: buffer, deallocator: .custom(nil, {})) |
There was a problem hiding this comment.
I'd much prefer to not use bytesNoCopy, but using DispatchData(bytes: buffer) crashes with EXC_BAD_ACCESS when the DispatchData deallocates.
There was a problem hiding this comment.
I looked into this case and the other one below. I'm not sure what's going on. 😕 Maybe we should wait for the next Xcode build?
There was a problem hiding this comment.
This seems to be fixed in Xcode 8 beta 6 so I've made the change and pushed it through.
Handle recent API changes in ReactiveCocoa.
Tests currently don't run; waiting for a beta 6 compatible version of Quick.
Tests now run again.
…ta(bytes:). This no longer causes a crash as of Xcode 8 beta 6.
| // return Data(bytes: bytes, count: dispatchData.count) | ||
| // } | ||
| // } | ||
| //} |
Fix memory leak in use of UnsafeMutablePointer.
|
Looks good. Thanks! |
|
🎉 |
Moves ReactiveTask to Swift 3, targeting Xcode 8 beta 1. I'll keep the PR updated as each new beta appears.
NSDatawithData.