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

SipHasher::new_with_keys deprecation message is unhelpful/misleading #37070

Closed
SimonSapin opened this issue Oct 10, 2016 · 4 comments
Closed
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@SimonSapin
Copy link
Contributor

SimonSapin commented Oct 10, 2016

Since #36815, using SipHasher::new_with_keys gives a warning:

warning: use of deprecated item: use `DefaultHasher` instead, #[warn(deprecated)] on by default

But DefaultHasher does not have a new_with_keys method or anything similar. It looks like this functionality was deprecated without replacement. Assuming the libs team does not want to revisit this decision, the deprecation message should be changed to… something.

Right now, the easiest "fix" for code using this method is to use #[allow(deprecated)]. Since the method is marked #[stable] it’s not going away any time soon, so this is quite safe. But of course, recommending #[allow(deprecated)] in a deprecation message would defeat the point of deprecation.

Perhaps a "proper" fix would be to maintain a (non-deprecated) copy of SipHasher on crates.io?

One notable user of this method is phf. CC @sfackler

@aturon
Copy link
Member

aturon commented Oct 10, 2016

cc @rust-lang/libs

@aturon aturon added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 10, 2016
@sfackler
Copy link
Member

Seems like we should copy the impls out to an external crate.

@alexcrichton
Copy link
Member

Splitting out seems fine to me, but I believe the siphash crate was already taken last I checked.

@Mark-Simulacrum
Copy link
Member

The siphasher crate now exposes this API, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

5 participants