rust-lang / libs-team

The home of the library team
Apache License 2.0
115 stars 18 forks source link

ACP: Move `std::collections::hash_map::{RandomState, DefaultHasher}` to `std::hash` #267

Closed clarfonthey closed 11 months ago

clarfonthey commented 12 months ago

Proposal

Problem statement

As part of the effort to move HashMap and HashSet (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 in std::collections::hash_map and not in std::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 the std 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 of std::collections::hash_map::RandomState, in addition to RandomState and DefaultHasher being more discoverable in std::hash.

It's also worth pointing out that the only other Hasher implementation is SipHasher, 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 the std::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, only RandomState 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:

Amanieu commented 11 months ago

We discussed this in the libs-api meeting yesterday and are happy to accept this.