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

HashMap serialization does not support other hasher's #87

Closed
Mythra opened this issue Oct 5, 2024 · 2 comments
Closed

HashMap serialization does not support other hasher's #87

Mythra opened this issue Oct 5, 2024 · 2 comments

Comments

@Mythra
Copy link

Mythra commented Oct 5, 2024

Hey,

I've been working on adopting merde into a project I've been working on, and this project makes extensive use of fnv, and it's type FnvHashMap, which is really just: pub type FnvHashMap<K, V> = HashMap<K, V, FnvBuildHasher>; because of it's performance characteristics with smaller keys. Unfortunately when trying to implement JsonSerialize with types like this, you get an error message like:

error[E0277]: the trait bound `HashMap<std::string::String, merde::Value<'args>, BuildHasherDefault<FnvHasher>>: JsonSerialize` is not satisfied
    --> src/ser.rs:1235:22
     |
1235 |         guard.pair("args", &self.args_merde);
     |                            ^^^^^^^^^^^^^^^^ the trait `JsonSerialize` is not implemented for `HashMap<std::string::String, merde::Value<'args>, BuildHasherDefault<FnvHasher>>`
     |
     = help: the trait `JsonSerialize` is implemented for `HashMap<K, V>`
     = note: required for the cast from `&HashMap<std::string::String, merde::Value<'args>, BuildHasherDefault<FnvHasher>>` to `&dyn JsonSerialize`

Changing this type to a regular hashmap allows the code to compile. I know merde isn't super focused on performance, or every case, so I'm curious would there be any possibility to support custom hashers? or is this something better left to "just use a regular hashmap, and if you want something to support custom hashers use another crate".

@fasterthanlime
Copy link
Contributor

I definitely want to support custom hashers (but not pull them in by default). I thought it was already the case, in fact.

I would accept a PR for that if you beat me to it!

@fasterthanlime
Copy link
Contributor

merde 6.1.0 is being released, which includes that feature :)

cf. #88

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

No branches or pull requests

2 participants