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

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

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

Comments

@oli-obk
Copy link
Contributor

@oli-obk 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

@TyOverby TyOverby commented Nov 10, 2015

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

@oli-obk
Copy link
Contributor Author

@oli-obk 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

@TyOverby TyOverby commented Nov 10, 2015

I'll look into it today, thanks!

@TyOverby
Copy link
Collaborator

@TyOverby TyOverby commented Dec 11, 2015

@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
Collaborator

@dtolnay dtolnay commented Jan 31, 2017

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

@TyOverby TyOverby closed this Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.