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

Borrow field identifiers from deserializer, instead of input #185

Merged
merged 1 commit into from
Jul 10, 2022

Conversation

dtolnay
Copy link
Contributor

@dtolnay dtolnay commented Jul 4, 2022

Fixes #167.

Bitvec's handwritten Deserialize impls tried to borrow a bunch of strings directly from the deserializer's input, including the field names "order", "head", "bits", "data", "width", "index" and the value of the "order" field for comparing against type_name::<O>().

This sometimes works, for example when deserializing from a JSON &str. However, this is not how derived Deserialize impls work and a lot of formats won't be able to hand out borrowed data from the input like that. For example, when deserializing any data format from a R: std::io::Read. (You can tell because https://doc.rust-lang.org/std/io/trait.Read.html literally has no methods for borrowing data. Everything that this trait does is defined in terms of copying data into a caller-provided buffer, not borrowing it from the underlying i/o object.)

This PR changes the Deserialize impls to borrow from the Deserializer instead, as opposed to the input data. This works efficiently in all formats. Deserializers that operate on i/o resources will let you borrow from the same buffer that they use for obtaining data from the underlying Read object. Deserializers that wrap an in-memory data structure like serde_json::Value will borrow from there without copying or allocation.

@myrrlyn
Copy link
Collaborator

myrrlyn commented Jul 10, 2022

This is wonderful, thank you!!

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.

Deserializing with serde yields invalid type error
2 participants