Make all CF types Equatable and Hashable. #4394
Conversation
|
@DougGregor and @jckarter, can you review this? I want to be very sure I didn't introduce something wonky. @swift-ci Please test |
| if (isa<ProtocolDecl>(witness->getDeclContext())) { | ||
| continue; | ||
| } | ||
| enum Attempt { |
There was a problem hiding this comment.
This feels like too much of a special case -- can we generalize this somehow? Why are operators special? I see the operator in question is inside a type anyway, why isn't it found with the usual mechanisms?
There was a problem hiding this comment.
Operator lookup is always global right now. We can take it back out if/when that's no longer true or when it becomes a fallback path.
I want this to be narrow because there shouldn't be many cases where it happens; we can broaden it later if we need to.
There was a problem hiding this comment.
Ok, I'll stop complaining. Let's tack this onto @DougGregor's todo list for his upcoming name lookup cleanup. :)
|
Build failed |
|
@swift-ci Please test OS X platform |
|
Overlay part LGTM. I defer to @slavapestov and @DougGregor on the sema parts. |
|
Build failed |
|
@swift-ci test |
|
Build failed |
|
The change to operator lookup feels a bit weird as a one-off hack, but it looks like it will do the job. The rest looks good. |
|
There's an Edit: I see that it is. |
|
And this is why we run tests first. :-) Updating. |
|
Yeah, I'm not happy about the operator change, but it's justifiable and unlikely to interfere with anything else. |
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). https://bugs.swift.org/browse/SR-2388
jrose-apple
force-pushed
the
Hashable-CF
branch
from
d33c94d to
2f367ec
Compare
|
@swift-ci Please test macOS |
|
Status not updated, but the tests are running. |
|
Tests passed, force-merging. |
|
Reverting. New test Interpreter/SDK/cf.swift fails in two places when built with --test-optimized. I don't know if it's a bug here or a miscompile of this code. Also reverting #4417 on the 3.0 branch. |
|
Well, shoot. Thanks, Greg. |
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).https://bugs.swift.org/browse/SR-2388
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.