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 additional tests for borrowed input #509

Merged
merged 1 commit into from Nov 14, 2022
Merged

Conversation

Mingun
Copy link
Collaborator

@Mingun Mingun commented Nov 13, 2022

Borrow input in attributes is working now, I suppose, after rewrite in #490. Now that is ensured by tests. Closes #328

@Mingun Mingun added the serde Issues related to mapping from Rust types to XML label Nov 13, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #509 (c9edac4) into master (9f8ec44) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #509      +/-   ##
==========================================
+ Coverage   61.33%   61.35%   +0.01%     
==========================================
  Files          33       33              
  Lines       15393    15393              
==========================================
+ Hits         9441     9444       +3     
+ Misses       5952     5949       -3     
Flag Coverage Δ
unittests 61.35% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/reader/buffered_reader.rs 85.37% <0.00%> (-0.48%) ⬇️
src/de/mod.rs 67.23% <0.00%> (+0.07%) ⬆️
src/escapei.rs 12.53% <0.00%> (+0.17%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

/// Deserialization of all XML's in that module expected to fail because
/// values requires unescaping that will lead to allocating an internal
/// buffer by deserializer, but the `Borrowed*` types couldn't take ownership
/// on it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any concept of opportunistic zero-copy with Cow<&str>? As someone not terribly familiar with serde.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I understand you correctly, Cow<str> will borrow whenever possible i. e. when deserializer can provide data of underlying source by calling Visitor::visit_borrowed_str. The opposite case when the deserializer allocates String by itself and calls Visitor::visit_string. The intermediate case when deserializer provides borrow to the data, that it owns, via Visitor::visit_str.

In our case we can either provide access to the XML source by calling Visitor::visit_borrowed_str, or allocate a new String (because we in any case need to allocate to do unescaping and/or decoding) and give it to the Visitor. If XML in UTF-8 and does not contain escaped data or mixed text/CDATA or CDATA/CDATA content (after solving #474), the data will be borrowed

@Mingun Mingun merged commit 92de42e into tafia:master Nov 14, 2022
@Mingun Mingun deleted the borrow branch November 14, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
serde Issues related to mapping from Rust types to XML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support zero-copy reading in serde deserializer
3 participants