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

Make hasher usable #14

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Make hasher usable #14

merged 2 commits into from
Jul 23, 2024

Conversation

carols10cents
Copy link
Contributor

Currently, the only public way to construct a BloomFilterBuilder
is by using BloomFilterBuilder::default(), which only constructs
a BloomFilterBuilder<RandomState, CompressedBitmap>.

Then the hasher function takes a hasher of type H, which can
only ever be called with a RandomState as that's the only kind of
BloomFilterBuilder you can have.

So you can have a BloomFilterBuilder of any hasher you want, as long as
it's RandomState!

Thus, this test fails to compile:

error[E0308]: mismatched types
   --> src/bloom.rs:446:21
    |
446 |             .hasher(BuildHasherDefault::<twox_hash::XxHash64>::default())
    |              ------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `RandomState`, found `BuildHasherDefault<XxHash64>`
    |              |
    |              arguments to this method are incorrect
    |
    = note: expected struct `std::hash::RandomState`
               found struct `BuildHasherDefault<XxHash64>`

To fix this, I changed hasher into a constructor that otherwise uses the same values as in default-- let me know if you'd rather have a different solution :)

Currently, the only public way to construct a `BloomFilterBuilder`
is by using `BloomFilterBuilder::default()`, which only constructs
a `BloomFilterBuilder<RandomState, CompressedBitmap>`.

Then the `hasher` function takes a `hasher` of type `H`, which can
only ever be called with a `RandomState` as that's the only kind of
`BloomFilterBuilder` you can have.

So you can have a BloomFilterBuilder of any hasher you want, as long as
it's RandomState!

Thus, this test fails to compile:

```
error[E0308]: mismatched types
   --> src/bloom.rs:446:21
    |
446 |             .hasher(BuildHasherDefault::<twox_hash::XxHash64>::default())
    |              ------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `RandomState`, found `BuildHasherDefault<XxHash64>`
    |              |
    |              arguments to this method are incorrect
    |
    = note: expected struct `std::hash::RandomState`
               found struct `BuildHasherDefault<XxHash64>`
```
So that it's actually possible to change the hasher :P
Copy link
Owner

@domodwyer domodwyer left a comment

Choose a reason for hiding this comment

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

Fail 🥔

Thank you! 🙏

@domodwyer domodwyer merged commit 0d72876 into domodwyer:master Jul 23, 2024
2 checks passed
@carols10cents carols10cents deleted the wtf branch July 23, 2024 14:21
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