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

Support into iterator #251

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

hackaugusto
Copy link
Contributor

@hackaugusto hackaugusto commented Feb 23, 2024

Removes the Sized bound, it is no longer necessary, since #239 .
The change above allows for impl<T: Serializable> Serializable for &T.
The change above allows to change write_many to be generic and receive a IntoIterator without breaking the serialization for [T; C]

The removal of the Sized bound also makes the trait object save, and allows for type erasure.

Copy link
Collaborator

@irakliyk irakliyk left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good!

btw, one other interesting thing we can improve on serialization front: we could now implement Serializable and Deserializable for Vec, String, and maybe &[] by first serializing the number of elements using as usize and then serializing the actual elements.

For example, for Vec it would look something like this:

impl <T: Serializable> Serializable for Vec<T> {
    fn write_into<W: ByteWriter>(&self, target: &mut W) {
        target.write_usize(self.len());
        target.write_many(self);
    }
}

@hackaugusto
Copy link
Contributor Author

@irakliyk yes! The motivation for this was the BTreeMap serialization. This also allows that to be serialized with no copying (whereas now one has to copy the map to a vec and then serialize that)

@irakliyk irakliyk merged commit 7302d26 into facebook:main Feb 28, 2024
9 checks passed
@hackaugusto hackaugusto deleted the hacka-support-into-iterator branch February 28, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants