Add default Hashable implementations for RawRepresentable types #20705
Conversation
|
@swift-ci test |
lorentey
changed the title
|
@nkcsgexi This wasn't flagged by the api-digester tests as an ABI-impacting change, but I think it is one. It adds protocol extensions that prevent the compiler from synthesizing Hashable members directly under conforming types. |
|
@lorentey @jrose-apple The ABI digester only flags changes which would break backward compatibility with existing code. It does not flag changes that prevent backward deployment. |
|
Actually, I see what you mean. A downstream dependency could call a synthesized member, and it will break once a new build removes the synthesized member. However, the ABI digester won't flag that. @jrose-apple We already had a similar issue with synthesized == because of @_implements. Maybe we can re-consider calling synthesized members via witness method dispatch? @lorentey @nkcsgexi I would have expected the ABI digester to flag the removal of the synthesized members themselves. Does it skip implicit decls for some reason? Or are there no types with synthesized hashValue in the stdlib? |
|
Note that the checker correctly flagged I supplied explicit definitions in 545fa5e. (Then I promptly forgot I did so.) |
|
@lorentey , If i understand the problem correctly, you're suggesting we should flag adding decls as potential ABI breakages if they can prevent compiler's synthesizing members for types in down-streaming modules. This can be a problem when we deploy ABI checker for module A but not for module B, right? |
|
@nkcsgexi That’s exactly right! If module B isn’t covered by the ABI checker (or if a check is only triggered when there is a change in module B itself), then these additions may cause silent breakage. |
|
Thanks for clarification! The good news is we have deployed ABI checker for both stdlib and all SDK overlays, and they happen to be the entire set of modules whose ABI we care about now. So i don't think in practice this will be a problem. |
I'd argue that this is a problem for module B to deal with, we mainly need to be able to flag it for module B, not flag it for module A. |
…out implementing `hashValue` Before this change, `RawRepresentable`'s custom `hashValue` implementation used to forward to `rawValue.hashValue`. (See swiftlang#20705.) This sort of made sense at the time (`hash(into:)` was very new and `hashValue` was still in heavy use): it allowed raw representable values to return the exact same hash value as their `rawValue`, which some code used to (mistakenly) rely on. The big drawback of this is that to customize the Hashable implementation of a RawRepresentable type, people need to implement both `hashValue` and `hash(into:)` -- the former does not otherwise pick up customizations to the latter. This change makes the default `RawRepresentable.hashValue` implementation call `self.hash(into:)`, just like it would on non-RawRepresentable types. rdar://82651116
RawRepresentablecurrently provides a custom generic definition of==that gets picked up as theEquatable.==implementation whenever a concreteRawRepresentabletype conforms toEquatablewithout providing its own implementation. The custom==compares therawValue.Unfortunately, there are no such definitions provided for hashing, so
Hashableraw-representable types get compiler-synthesized hashing, which is based directly on the components of the type. For types with a computedrawValue, this can lead to a mismatch between equality and hashing, leading to broken hashes.Add the missing hashing members as extension methods on
RawRepresentable.This is a surprisingly far-reaching change: it replaces compiler-generated members synthesized directly under the type with stdlib-provided protocol extensions.
rdar://problem/45308741