Skip to content

[5.1 04-24-2019][Foundation] Modernize hashing in Foundation's Swift-only types #24369

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

lorentey
Copy link
Member

This is cherry-picked from #23832 -- see there for an extended discussion.

This PR upgrades the Foundation overlay to use the new hashing API introduced in SE-0206. This gets rid of the deprecation warnings for hashValue implementations and brings the overlay in sync with current Swift best practices.

Reviewed by: @itaiferber and @parkera

rdar://problem/43394032

lorentey added 30 commits April 29, 2019 11:21
…*all* components

The new definition diverges from the existing hashValue: it feeds all path components to the hasher, rather than just a limited subset.

Hashing all the bits that are compared in == is necessary to ensure proper operation of the hash table.
This replaces the compiler-generated variant (based on hashValue) with a better implementation.
Note: URL does not implement Hashable.
I’m not sure why I was so convinced it wasn’t, but this certainly simplifies things.
…bilize hashing

The `ObjectIdentifier(object as AnyObject)` is not necessarily stable; this breaks reflexivity for ==, and it makes the hash encoding nondeterministic.
@lorentey lorentey requested a review from a team as a code owner April 29, 2019 18:29
@lorentey
Copy link
Member Author

@swift-ci test

@lorentey
Copy link
Member Author

cc @airspeedswift

@lorentey lorentey requested a review from airspeedswift April 30, 2019 00:14
@airspeedswift airspeedswift merged commit 2e85997 into swiftlang:swift-5.1-branch-04-24-2019 Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants