Skip to content

Conversation

hoxxep
Copy link
Contributor

@hoxxep hoxxep commented Aug 26, 2025

Following up from the rapidhash PR #633, this bumps foldhash to 0.2.0 for a large improvement in foldhash's string hashing.

Both rapidhash and foldhash are equal in the hashbrown benchmark suite as it only tests the hashing of usize objects.

The reason foldhash 0.2.0 required a major version bump is because FoldHasher now has a lifetime parameter. I don't know how users are expected to use DefaultHashBuilder, but noting that it likely makes this a breaking change for hashbrown?

cc. @orlp @Amanieu from #633

@orlp
Copy link

orlp commented Aug 26, 2025

I'm not sure if it's a breaking release, the DefaultHashBuilder still implements the HashBuilder contract all the same, and that lifetime is just 'static on an associated type.

@CryZe
Copy link
Contributor

CryZe commented Aug 26, 2025

I believe it technically is breaking because you could pass DefaultHashBuilder to a function expecting foldhash-0.1::RandomState whereas that breaks with this. DefaultHashBuilder probably should be using the newtype pattern.

@jqnatividad
Copy link

FYI... I also got this compilation error when I attempted to use this PR on [patch.crates-io] of my project's Cargo.toml

error[E0204]: the trait `core::marker::Copy` cannot be implemented for this type
  --> /Users/jdoe/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/hashlink-0.10.0/src/lib.rs:19:17
   |
19 | #[derive(Clone, Copy, Default, Debug)]
   |                 ^^^^
20 | pub struct DefaultHashBuilder(hashbrown::DefaultHashBuilder);
   |                               ----------------------------- this field does not implement `core::marker::Copy`

@cuviper
Copy link
Member

cuviper commented Aug 27, 2025

DefaultHashBuilder probably should be using the newtype pattern.

Yes -- this might be a good time to do that and bump to 0.16.

@orlp
Copy link

orlp commented Aug 28, 2025

FYI... I also got this compilation error when I attempted to use this PR on [patch.crates-io] of my project's Cargo.toml

error[E0204]: the trait `core::marker::Copy` cannot be implemented for this type
  --> /Users/jdoe/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/hashlink-0.10.0/src/lib.rs:19:17
   |
19 | #[derive(Clone, Copy, Default, Debug)]
   |                 ^^^^
20 | pub struct DefaultHashBuilder(hashbrown::DefaultHashBuilder);
   |                               ----------------------------- this field does not implement `core::marker::Copy`

See here for context: orlp/foldhash#28.

But yes, the lack of a newtype pattern meant that this breaking change would leak into hashbrown as it directly passes through any traits that were implemented.

@cuviper
Copy link
Member

cuviper commented Aug 28, 2025

See #643 for a newtype.

@Amanieu Amanieu added this pull request to the merge queue Aug 28, 2025
Merged via the queue into rust-lang:master with commit bf35a11 Aug 28, 2025
26 checks passed
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.

6 participants