-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Make ObjC isEqual: delegate to Equatable when Hashable isn't available #68720
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
Conversation
When a Swift struct gets bridged to Obj-C, we box it into an opaque `_SwiftValue` Obj-C object. This object previously supported the Obj-C `isEqual:` and `hash` selectors by dispatching to the Swift Hashable conformance, if present. This does not work if the Swift struct conforms to Equatable but does not conform to Hashable. This case seems to have been overlooked in PR swiftlang#4124. This PR extends the earlier work to support `isEqual:` by first checking for a Hashable conformance, then falling back on an Equatable conformance if there is no Hashable conformance. Resolves rdar://114294889
@mikeash Any ideas how we might extend the caching here to support the Equatable case? There are two pointers in the _SwiftValue header for the "base Hashable type" and the Hashable conformance. I'd like to avoid adding two more pointers for Equatable, but I'm a little unsure if it's a good idea to try reusing the existing pointers. One possibility: Use a couple of bits of those pointers as booleans to flag whether the rest of the pointer is for Hashable or Equatable. I'm not sure how best to do that while dodging all the other folks (pac, etc) that are trying to exploit those bits. |
Very nice! Those pointers don't have PAC (although that raises the question of whether they should... they serve a similar purpose to an isa/metadata pointer), but there aren't always high bits available anyway, for example on 32-bit. These pointers are aligned to the pointer size, so the low 2-3 bits (depending on pointer size) are available, and nothing else is trying to use them (yet). You can use the low bit as a flag to indicate whether you have |
@swift-ci Please test |
That's what I needed to know. Thanks! PointerUnion's magic failed me here, so I ended up flogging the low-order bit manually. Seems okay so far. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just some small comments.
@swift-ci Please test |
@swift-ci please test Linux platform |
Sourcekit-lsp failure on Linux seems unrelated? |
@swift-ci Please test Linux platform |
It's unrelated, Alex merged the fix for it last night. Sorry about that! |
If a Swift type implements Equatable and/or Hashable and we then pass that object into ObjC, we want ObjC `isEqual:` and `hashValue` to use that. This allows ObjC code to build ObjC collections of Swift objects. * Support for Hashable struct/enum types was implemented in swiftlang#4124 * Support for Equatable struct/enum types was implemented in swiftlang#68720 * This implements support for Hashable and Equatable _class_ types Caveats: 1. This does a lot of dynamic lookup work for each operation, so is inherently rather slow. Unlike the struct/enum case, there is no convenient place to cache the conformance information, so it's not clear that there is a viable way to make it significantly faster. 2. This is a behavioral change to low-level support code. There is a risk of breaking code that may be relying on the old behavior.
Update PR swiftlang#68720 with lessons learned in reviewing swiftlang#69464 Background: * SwiftValue can expose Swift value types (structs/enums) to ObjC by wrapping them in an Obj-C object on the heap * SwiftObject is the Obj-C type that Swift class objects all inherit from (when viewed from Obj-C). This allows arbitrary Swift class objects to be passed into Obj-C. History: * PR swiftlang#4124 made Obj-C `-hash` and `-isEqual:` work for SwiftValue for Hashable Swift types * PR swiftlang#68720 extended SwiftValue to also support Equatable Swift types * PR swiftlang#69464 added similar support to SwiftObject In the process of working through swiftlang#69464, we found a better way to handle an ObjC request for `-hash` for a type that is Swift Equatable but not Hashable. This PR updates SwiftValue to use the same approach. The approach considers three cases: 1. A Hashable type can forward both `-hash` and `-isEqual:` to the Swift object. 2. A type that is neither Equatable nor Hashable can implement `-isEqual:` as the identity check and `-hash` as returning the address of the object in memory. 3. A type is that Equatable but not Hashable is more complex. In this last case, we can easily forward `-isEqual:` to the Equatable conformance but ObjC also requires us to always provide a compatible `-hash` implementation. The only way to do this is to have `-hash` return a constant, but that is a very bad idea in general, so we're also including a log message whenever we see a request for `-hash` on a Swift value that is Equatable but not Hashable. To limit performance problems from the logging itself, we emit the log message only once for each type.
When a Swift struct gets bridged to Obj-C, we box it into an opaque
_SwiftValue
Obj-C object. This object previously supported the Obj-CisEqual:
andhash
selectors by dispatching to the Swift Hashable conformance, if present.This does not work if the Swift struct conforms to Equatable but does not conform to Hashable. This case seems to have been overlooked in PR #4124.
This PR extends the earlier work to support
isEqual:
by first checking for a Hashable conformance, then falling back on an Equatable conformance if there is no Hashable conformance.Resolves rdar://114294889