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

Implement traits when SparseSecondaryMap uses custom hasher #21

Merged
merged 1 commit into from
Aug 25, 2019

Conversation

michael-p
Copy link
Contributor

This is an addition to #20, making various trait impls (like Index, Serialize, ...) available even when SparseSecondaryMap is used with a custom hasher (previously they were not implemented in that case meaning something like sec_sparse_map[my_key] = 3 would fail to compile.

@orlp
Copy link
Owner

orlp commented Aug 24, 2019

Hey, sorry for not getting back to you earlier. This is definitely a wanted patch, but a couple things need changes.

I'd really rather not introduce a (dev) dependency on fxhash when this isn't necessary. Could you write a test case that doesn't use a dependency?

Finally, a very mild inconsistency,

impl<K, V, H> Serialize for SparseSecondaryMap<K, V, H>
uses H instead of S.

@orlp
Copy link
Owner

orlp commented Aug 25, 2019

Oh well, I just ran the tests and realized serde already pulls in so much stuff, fxhash really doesn't hurt.

@orlp orlp merged commit 9fad143 into orlp:master Aug 25, 2019
@orlp
Copy link
Owner

orlp commented Aug 25, 2019

And I just realized I'm a moron - the 'mild inconsistency' is a necessity because of a name clash.

@michael-p
Copy link
Contributor Author

Thanks for merging! I'm really glad to see that you're back working on slotmap, it's a very useful and well-written crate!

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