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

Add and derive serialization and deserialization traits (serde) #14

Closed

Conversation

InnovativeInventor
Copy link

I added serialization and deserialization using Serde's derive. I also ran fmt on it to make it look pretty (lmk if you want me to remove that).

I could expose the serialization and deserialization to end-users, but I feel like the decision to serialize to JSON, cbor, msgpack, etc. should be up to the user rather than a decision we impose.

Should fix #1.

@InnovativeInventor InnovativeInventor marked this pull request as draft January 19, 2021 00:47
@InnovativeInventor
Copy link
Author

Currently the trait Serialize is not implemented for std::collections::hash_map::RandomState -- I believe that the correct place for such an implementation is in the serde repo (opened an issue to check serde-rs/serde#1954). Once that is done, this should be ready to merge.

@mainrs
Copy link

mainrs commented Mar 24, 2021

This should be put behind a feature flag, making the dependency optional and putting a guard on the derives would work:

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]

On that note: I am not sure about the role of the random state in the algorithm. If everything but the random state is serialized, would it work if the struct gets deserialized again and a new random state gets created using RandomState::new?

@prataprc
Copy link
Owner

prataprc commented Mar 25, 2021

@InnovativeInventor @sirwindfield

If RandomState is used as BuildHasher, std has got this to say

A particular instance RandomState will create the same instances
of Hasher, but the hashers created by two different RandomState
instances are unlikely to produce the same result for the same values.

If DefaultHasher is used as BuildHasher, std has got this to say,

The internal algorithm is not specified, and so it and its hashes
should not be relied upon over releases.

The default type for parameter H might change when we get a reliable
and commonly used BuildHasher type is available.

I am also adding the above note as part of Xor8 type-doc.

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.

Serialize / Deserialize Xor8 type.
3 participants