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

Hasher::finish() does not reset the hasher. #43763

Closed
partim opened this issue Aug 9, 2017 · 2 comments · Fixed by #43905
Closed

Hasher::finish() does not reset the hasher. #43763

partim opened this issue Aug 9, 2017 · 2 comments · Fixed by #43905
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@partim
Copy link
Contributor

partim commented Aug 9, 2017

At least for HashMap’s default hasher, a call to Hasher::finish() will not reset the hasher’s internal state. Instead, additional write()s will modify the hasher’s existing state as if finish() had never been called:

use std::hash::{Hash, Hasher};
use std::collections::hash_map::DefaultHasher;

fn main() {
    let mut hasher = DefaultHasher::new();
    0u8.hash(&mut hasher);
    let h1 = hasher.finish();
    0u8.hash(&mut hasher);
    let h2 = hasher.finish();
    println!("h1: {}\nh2: {}", h1, h2);
}

will produce

h1: 7541581120933061747
h2: 6165216591721696495

Is this intended behavior? If so, this should probably mentioned in the documentation as it is somewhat surprising given the name of the method. I am happy to create a PR but wanted to check first.

@Mark-Simulacrum Mark-Simulacrum added C-feature-request Category: A feature request, i.e: not implemented / a PR. I-needs-decision Issue: In need of a decision. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed I-needs-decision Issue: In need of a decision. labels Aug 10, 2017
@bluss
Copy link
Member

bluss commented Aug 10, 2017

Finish has a signature with &self so immutability in it is more or less enforced by the trait. The remaining decision is how this should be documented on the trait; as a guarantee or just rule of thumb. The latter seems closest since it doesn't add a contract to an existing trait.

@partim
Copy link
Contributor Author

partim commented Aug 11, 2017

Oh, fair point about the &self argument. Totally missed that.

So, I propose to modify the documentation for Hasher::finish() pointing out more clearly the function really only extracts the current hash value and that if one wishes to create the hash of a new value one needs to create a new hasher for that. I’ll submit a PR later.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Aug 17, 2017
Document that `std::hash::Hasher::finish()` does not reset the hasher.

Clarifies the fact that `finish()` doesn’t in fact end or reset the hasher. This was surprising to me …

Follows up on and fixes rust-lang#43763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants