-
Notifications
You must be signed in to change notification settings - Fork 19
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
ACP: Move std::collections::hash_map::{RandomState, DefaultHasher}
to std::hash
#267
Labels
ACP-accepted
API Change Proposal is accepted (seconded with no objections)
api-change-proposal
A proposal to add or alter unstable APIs in the standard libraries
T-libs-api
Comments
clarfonthey
added
api-change-proposal
A proposal to add or alter unstable APIs in the standard libraries
T-libs-api
labels
Sep 9, 2023
Amanieu
added
the
ACP-accepted
API Change Proposal is accepted (seconded with no objections)
label
Oct 4, 2023
We discussed this in the libs-api meeting yesterday and are happy to accept this. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Nov 10, 2023
Add `std::hash::{DefaultHasher, RandomState}` exports (needs FCP) This implements rust-lang/libs-team#267 to move the libstd hasher types to `std::hash` where they belong, instead of `std::collections::hash_map`. <details><summary>The below no longer applies, but is kept for clarity.</summary> This is a small refactor for rust-lang#27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API. I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of `std::hash`, but for now, they're at least separated out from the collections code in a way that will make moving that around easier. I decided to simply copy the rustdoc for `std::hash` from `core::hash` since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred. </details>
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Nov 11, 2023
Add `std::hash::{DefaultHasher, RandomState}` exports (needs FCP) This implements rust-lang/libs-team#267 to move the libstd hasher types to `std::hash` where they belong, instead of `std::collections::hash_map`. <details><summary>The below no longer applies, but is kept for clarity.</summary> This is a small refactor for rust-lang#27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API. I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of `std::hash`, but for now, they're at least separated out from the collections code in a way that will make moving that around easier. I decided to simply copy the rustdoc for `std::hash` from `core::hash` since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred. </details>
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this issue
Nov 15, 2023
Add `std::hash::{DefaultHasher, RandomState}` exports (needs FCP) This implements rust-lang/libs-team#267 to move the libstd hasher types to `std::hash` where they belong, instead of `std::collections::hash_map`. <details><summary>The below no longer applies, but is kept for clarity.</summary> This is a small refactor for #27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API. I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of `std::hash`, but for now, they're at least separated out from the collections code in a way that will make moving that around easier. I decided to simply copy the rustdoc for `std::hash` from `core::hash` since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred. </details>
lnicola
pushed a commit
to lnicola/rust-analyzer
that referenced
this issue
Apr 7, 2024
Add `std::hash::{DefaultHasher, RandomState}` exports (needs FCP) This implements rust-lang/libs-team#267 to move the libstd hasher types to `std::hash` where they belong, instead of `std::collections::hash_map`. <details><summary>The below no longer applies, but is kept for clarity.</summary> This is a small refactor for #27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API. I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of `std::hash`, but for now, they're at least separated out from the collections code in a way that will make moving that around easier. I decided to simply copy the rustdoc for `std::hash` from `core::hash` since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred. </details>
RalfJung
pushed a commit
to RalfJung/rust-analyzer
that referenced
this issue
Apr 27, 2024
Add `std::hash::{DefaultHasher, RandomState}` exports (needs FCP) This implements rust-lang/libs-team#267 to move the libstd hasher types to `std::hash` where they belong, instead of `std::collections::hash_map`. <details><summary>The below no longer applies, but is kept for clarity.</summary> This is a small refactor for #27242, which moves the definitions of `RandomState` and `DefaultHasher` into `std::hash`, but in a way that won't be noticed in the public API. I've opened rust-lang/libs-team#267 as a formal ACP to move these directly into the root of `std::hash`, but for now, they're at least separated out from the collections code in a way that will make moving that around easier. I decided to simply copy the rustdoc for `std::hash` from `core::hash` since I think it would be ideal for the two to diverge longer-term, especially if the ACP is accepted. However, I would be willing to factor them out into a common markdown document if that's preferred. </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
ACP-accepted
API Change Proposal is accepted (seconded with no objections)
api-change-proposal
A proposal to add or alter unstable APIs in the standard libraries
T-libs-api
Proposal
Problem statement
As part of the effort to move
HashMap
andHashSet
(in some form) to liballoc, the actual libstd-specific implementation needs to be separated out from the version that can be added to liballoc.In general, it's very weird that these are exported in
std::collections::hash_map
anyway, and to a user who wishes to use them outside these collections, they're not exactly easy to find. Additionally, these are only exported instd::collections::hash_map
and not instd::collections::hash_set
.Ideally, these would be front and center in the
std::hash
module to indicate that they're the recommended way to perform hashing when thestd
crate is available, i.e. in all cases except those where#[no_std]
is enabled.Motivating examples or use cases
Mostly covered by above. The main use case is simply importing
std::hash::RandomState
instead ofstd::collections::hash_map::RandomState
, in addition toRandomState
andDefaultHasher
being more discoverable instd::hash
.It's also worth pointing out that the only other
Hasher
implementation isSipHasher
, which is deprecated. This would offer a genuine "default" choice alongside that one in the same module, rather than being located in a different one.Solution sketch
std::collections::hash_map::{RandomState, DefaultHasher}
should be re-exported in thestd::hash
module, making this module officially different from the one offered in libcore. In some future version, after a suitable amount of time, the original location can be deprecated.Eventually, there could potentially be more methods for
DefaultHasher
that let the user seed the hasher with their own source of randomness in libcore, bridging the gap between the core and std implementations. This would require guaranteeing that the hasher itself doesn't depend on libstd, onlyRandomState
does, although I don't think that would be too controversial.Alternatives
The only real alternative is not doing this.
Links and related work
"Move HashMap to liballoc" tracking issue: rust-lang/rust#27242
What happens now?
This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
Second, if there's a concrete solution:
The text was updated successfully, but these errors were encountered: