Skip to content
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

Improve std::hash::Hasher to finish with an associated type #65744

Closed
afranchuk opened this issue Oct 23, 2019 · 4 comments
Closed

Improve std::hash::Hasher to finish with an associated type #65744

afranchuk opened this issue Oct 23, 2019 · 4 comments
Labels
rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@afranchuk
Copy link

std::hash::Hasher currently defines a finish() method that returns a fixed type (u64). This trait could be improved by changing the return type to some associated Output type. I don't see a particular reason this wouldn't work, since, for instance, std::collections::HashMap can simply provide a Hasher which produces the type it expects. This would be nice as then types implementing std::hash::Hash could be used with, for example, cryptographic hash functions. I am aware of the digest crate intended for that use case, but I don't see a reason not to improve the existing trait.

@Mark-Simulacrum
Copy link
Member

Hasher is a stable trait so this change cannot be made backwards compatibly, so we cannot make it at this point. As such, closing.

In theory an extension that provides a default could work here, but I imagine that it would run into lots of issues really quickly (e.g., inference, in particular for Hash trait impls, would be problematic).

@Mark-Simulacrum Mark-Simulacrum added the rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 label Oct 23, 2019
@afranchuk
Copy link
Author

I just want to remark that if RFC 2532 (associated type defaults) gets in then this change could be made in a backwards compatible way.

@Mark-Simulacrum
Copy link
Member

I don't think that's entirely true (or, at least, not in general) - Hash::hash takes a Hasher generically, and that can't change to also take a type param which would mean that the extension to this trait is largely useless.

@afranchuk
Copy link
Author

Good point, you'd have to move the finish() method from Hasher to a separate trait. The only thing Hash should care about is the write methods of Hasher, I think? So yeah, a breaking change...

@dtolnay dtolnay added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants