Skip to content

BuildHasherDefault: remove #30

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

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

newAM
Copy link
Contributor

@newAM newAM commented Mar 29, 2025

This type existed because core::hash::BuildHasherDefault did not have a const constructor.

As of 1.85 core::hash::BuildHasherDefault has a const constructor.

This type existed because core::hash::BuildHasherDefault did not have a
const constructor.

As of 1.85 core::hash::BuildHasherDefault has a const constructor.
@newAM newAM force-pushed the rm-build-hasher-default branch from 90adb88 to 619a16d Compare March 29, 2025 00:11
Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
Is there a workaround for anyone using Rust < 1.85.0 ?
Ferrocene is on 1.83.0, IIRC.

@newAM
Copy link
Contributor Author

newAM commented Mar 30, 2025

Ferrocene is indeed on 1.83.0.

Ferrocene is about 3 months behind the latest stable, and I don't see heapless 1.0 happening within that time (though I expect hash32 would happen within that time).

There are some options here, I'm happy to implement any of these.

  1. Proceed with this PR, and have a gap in ferrocene support for const constructors
  2. Keep the type for 1.0, but mark it as deprecated. I need to check if deprecation warnings can be suppressed by users of the crate, otherwise it's similar enough to removing it.
  3. Keep the type for 1.0, document core::hash::BuildHasherDefault is preferred on newer toolchains. This does mean maintaining the type for the foreseeable future, but it is a small amount of code.

What do you think?

Copy link
Member

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks.
Let's go with option 1. We can add it back later on if somebody really needs it.

@eldruin eldruin merged commit 9fa28e7 into rust-embedded-community:main Mar 31, 2025
5 checks passed
@newAM newAM deleted the rm-build-hasher-default branch April 5, 2025 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants