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

struct deserialization should offer field-names to the passed visitor #48

Closed
oli-obk opened this issue Nov 10, 2015 · 5 comments
Closed

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Nov 10, 2015

Right now you treat structs like tuples, because they are laid out the same way in bincode's format. During serialization that works mostly fine (since serializers usually pass struct fields in the order in which they are declared). During deserialization you deserialize the struct through visit_seq. Structs are meant to be deserialized through visit_map.

See some context here: serde-rs/serde#177

I think there could be room for improvement in serde wrt struct serialization to make sure that the serializer can choose the order of the elements it receives instead of the struct's Deserialize implementation.

cc @erickt

@TyOverby
Copy link
Collaborator

Would "offering" field names during deserialization require serializing them?

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 10, 2015

nope, You get a static array of the names. So basically it'd just be iterating through that array and using them as keys to pass to a MapVisitor.

@TyOverby
Copy link
Collaborator

I'll look into it today, thanks!

@TyOverby
Copy link
Collaborator

@erickt Could you take a look at this? I tried playing with it, but I'm still really unsure about the serde interface. Since you wrote the original implementation, do you think this would be a reasonable thing to add?

@dtolnay
Copy link
Contributor

dtolnay commented Jan 31, 2017

We no longer need this. Serde has embraced visit_seq as a way to deserialize braced structs.

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

3 participants