Implement with_hasher adaptors#1007
Conversation
jswrenn
left a comment
There was a problem hiding this comment.
One small nit, but otherwise this LGTM.
|
Two nits:
@jswrenn Feel free to proceed as you deem appropriate. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1007 +/- ##
==========================================
+ Coverage 94.38% 94.67% +0.28%
==========================================
Files 48 50 +2
Lines 6665 7062 +397
==========================================
+ Hits 6291 6686 +395
- Misses 374 376 +2 ☔ View full report in Codecov by Sentry. |
It would be a breaking change for Rust to change its default hasher, so we're fine to rely on |
| // A Hasher which forwards it's calls to RandomState to make sure different hashers | ||
| // are accepted in the various *_with_hasher methods. | ||
| #[derive(Default)] | ||
| struct TestHasher(RandomState); | ||
|
|
||
| impl TestHasher { | ||
| fn new() -> Self { | ||
| TestHasher(RandomState::new()) | ||
| } | ||
| } | ||
|
|
||
| impl BuildHasher for TestHasher { | ||
| type Hasher = <RandomState as BuildHasher>::Hasher; | ||
|
|
||
| fn build_hasher(&self) -> Self::Hasher { | ||
| self.0.build_hasher() | ||
| } | ||
| } |
There was a problem hiding this comment.
Does this buy us much? If it behaves identically to RandomState, then what's the advantage of newtyping it? I'm not worried that we've defined our with_hasher methods to literally only accept RandomState.
There was a problem hiding this comment.
Because RandomState is the default for the type parameters, it's possible to implement the types only for S = RandomState. This is just the easiest way I thought of to catch that.
There was a problem hiding this comment.
Gotcha. We can test that more directly by doing:
let _: HashMap<_, _, TestHasher> = empty::<u8>().into_group_map_with_hasher(TestHasher::new());No need to then repeat the assertions on the output value.
New calls to the *_with_hasher methods are added to make sure they are callable with non-default hashers and provide the correct types.
Closes #998
Implementations for the following methods: