Re-apply "Make all CF types Equatable and Hashable." #4568
Conversation
|
@swift-ci Please test |
|
@DougGregor, @parkera, this is technically a source-breaking change since someone may have manually added Hashable or Equatable to a CF type. Then again, that's both a type and protocol they don't own, which isn't really safe because anyone could do it ("what if two people did this?"). Do you think we can commit this or do we have to be more circumspect? |
|
This seems like acceptable risk to me. |
| // RUN: %target-run-simple-swift | %FileCheck %s | ||
|
|
||
| // REQUIRES: executable_test | ||
| // REQUIRES: objc_interop |
There was a problem hiding this comment.
Technically only requires a platform where CF is part of the SDK, doesn't it?
What do you think about moving/copying this test in to Foundation and running it on all platforms?
|
I've found at least one case on GitHub where someone is doing exactly this on CFString, so it's definitely a source-breaking change. May have to wait until Swift 4. |
|
Resurrecting this. It's almost certainly breaking, and we don't have a way to conditionalize it on Swift 3/4, but it is an improvement. What do people think? |
|
Even if this breaks somebody, it's really simple to fix. +1 |
|
I'm definitely +1 on doing this |
jrose-apple
force-pushed
the
Hashable-CF-take-2
branch
2 times, most recently
from
febd73e to
ba1d754
Compare
|
@swift-ci Please test source compatibility |
|
I still want to do a full test on Apple platforms but there's an error in one test right now. @swift-ci Please smoke test |
jrose-apple
force-pushed
the
Hashable-CF-take-2
branch
3 times, most recently
from
f1c1eb8 to
f2ff372
Compare
|
@swift-ci Please test |
|
Build failed |
|
@akyrtzi, we're now running into the old issue where |
jrose-apple
force-pushed
the
Hashable-CF-take-2
branch
from
f2ff372 to
e360e13
Compare
|
@swift-ci Please test macOS |
|
@swift-ci Please smoke test Linux |
|
Build failed |
|
@swift-ci Please test source compatibility |
1 similar comment
|
@swift-ci Please test source compatibility |
Like NSObject, CFType has primitive operations CFEqual and CFHash, so Swift should allow those types to show up in Hashable positions (like dictionaries). The most general way to do this was to introduce a new protocol, _CFObject, and then have the importer automatically make all CF types conform to it. This did require one additional change: the == implementation that calls through to CFEqual is in a new CoreFoundation overlay, but the conformance is in the underlying Clang module. Therefore, operator lookup for conformances has been changed to look in the overlay for an imported declaration (if there is one). This re-applies 361ab62, reverted in f50b1e7, after a /very/ long interval where we decided if it was worth breaking people who've added these conformances on their own. Since the workaround isn't too difficult--- use `#if swift(>=3.2)` to guard the extension introducing the conformance---it was deemed acceptable. https://bugs.swift.org/browse/SR-2388
jrose-apple
force-pushed
the
Hashable-CF-take-2
branch
from
e360e13 to
631250b
Compare
|
@swift-ci Please smoke test |
Like NSObject, CFType has primitive operations CFEqual and CFHash, so Swift should allow those types to show up in Hashable positions (like dictionaries). The most general way to do this was to introduce a new protocol, _CFObject, and then have the importer automatically make all CF types conform to it. This did require one additional change: the == implementation that calls through to CFEqual is in a new CoreFoundation overlay, but the conformance is in the underlying Clang module. Therefore, operator lookup for conformances has been changed to look in the overlay for an imported declaration (if there is one). This re-applies 361ab62, reverted in f50b1e7, after a /very/ long interval where we decided if it was worth breaking people who've added these conformances on their own. Since the workaround isn't too difficult--- use `#if swift(>=3.2)` to guard the extension introducing the conformance---it was deemed acceptable. https://bugs.swift.org/browse/SR-2388
not needed since swift 4 (swiftlang/swift#4568)
not needed since swift 4 (swiftlang/swift#4568)
This re-applies #4394, reverted in #4435, now that John has fixed CF casting logic in the optimizer. The round-trip casting test is also passing now.
https://bugs.swift.org/browse/SR-2388