Add draft proposal for ownership keyword removal in protocols. #707
Conversation
|
|
|
@DevAndArtist Thanks, fixed 👍 |
tkremenek
added
the
workgroup: needs implementation
|
The Core Team briefly discussed this proposal and believe it has a lot of traction. An implementation is needed for this proposal to schedule it for active review. Since this is source-breaking, a desired implementation would warn in the existing Swift modes (with a fixit to remove |
|
Please assign this back to me when there is an implementation. |
|
Thanks @tkremenek! I've got the start of an implementation. I originally added the change with fixits for warning in Swift 3 mode, and an error in Swift 4. I've rebased and will look at changing that to warn for Swift 4 mode and error in Swift 5 as you suggest. As this might be my first contribution I'll open a PR on the Swift repo as I might need a bit of help and guidance along the way. When I've got it ready I'll update this proposal with the link. Thanks again. |
|
Fantastic, please ask on the swift-dev mailing list if you get stuck and need help. |
|
I've added a pull request for the implementation of this proposal apple/swift#11744. @tkremenek I don't have permission to set the assignees but it's ready if you want to assign yourself back to it. Thanks. |
tkremenek
added
workgroup: ready
|
Implementation looks like it is coming along nicely. I've noticed some comments from @jrose-apple and @slavapestov in the implementation as it looks like they are actively reviewing the implementation. |
|
This is ready for review. |
### Fixed - Fixed memory leak relating to capturing CFNotify - Fixed black screen after search ### Added - Introduced `ManagedObjectContextUsing` protocol with default `context` implemented in `AppDelegate` ### Changed - Made `context` static in `AppDelegate` - Removed some unnecessary force unwrapping ### Suggested - Integrate imported characters’ `id`s to `uuid`s in core data to avoid duplicate? - Report the black screen as a bug to apple? - Remove unused `ManagedObjectContextSettable` protocol? - Upgrade `InAppNotify` to use Swift 4 instead of 3.2?
See Swift Evolution: swiftlang/swift-evolution#707
ghost
mentioned this pull request
commit 75f4a15 Author: Aaron McTavish <[email protected]> Date: Tue Apr 3 09:17:19 2018 +0100 Weak is no longer allowed on protocol properties See Swift Evolution: swiftlang/swift-evolution#707 commit 31a0279 Author: Aaron McTavish <[email protected]> Date: Tue Apr 3 08:51:02 2018 +0100 Update to Xcode 9.3 Project Settings commit faa9ef1 Author: Aaron McTavish <[email protected]> Date: Tue Apr 3 08:48:42 2018 +0100 Update gem dependencies
* Update gem dependencies * Update to Xcode 9.3 Project Settings * Weak is no longer allowed on protocol properties See Swift Evolution: swiftlang/swift-evolution#707
This proposal removes support for the keywords
weakandunownedfor property declarations in a protocol.This would fix SR-479.
There's was discussion on the Swift-evolution thread: Ownership on protocol property requirements. Most comments were that
weakandownedfor a property in a protocol don't have any meaning currently, are misleading to users, and should be removed.Although no functionality is actually being removed or changed it is technically a source breaking change.
There were comments around introducing meaning to these keywords in protocols but that it would probably be better given time constraints to get this inconsistency fixed for Swift 4 and bring up adding meaning as a separate topic.
Update, I've added a pull request for the implementation of this proposal apple/swift#11744.