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

Correctly forward number of methods to the originals in Compact / Readable wrappers #35

Merged
merged 8 commits into from
Aug 5, 2024

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Jul 28, 2024

Fixes a numerous of bugs:

  • forward serialize_i128 / serialize_u128
  • forward deserialize_i128 / deserialize_u128
  • forward visit_i128 / visit_u128
  • wrap key in SerializeMap::serialize_entry, fixes Readable/Compact still give "must use Configure" error when serializing Map #34
  • add wraps in deserialize_in_place
  • add wraps in collect_seq / collect_map
  • forwards skip_field and collect_str to the originals
  • the last commit uses private serde API. Unfortunately, it should be overridden in order to work correctly. As last resort, this commit can be dropped if you think that is will be a stopper to merge this fix
  • another private API, Deserializer::__deserialize_content also should be reimplemented to work correctly, but unfortunately this is impossible because of the efforts that have been made to ban that

… deserializer in Compact / Readable wrappers

For historical reasons Deserializer trait has default implementation for 128-bit integers
which returns error, so it is important to not forgot to re-implement those methods.
…ializer in Compact / Readable wrappers

For historical reasons Serializer trait has default implementation for 128-bit integers
which returns error, so it is important to not forgot to re-implement those methods.
…h wrapped deserializer

Although this is private serde API, not forwarding it would be error.
Unfortunately, another private API, Deserializer::__deserialize_content,
cannot be implemented because it accepts private type from serde
@Mingun Mingun changed the title Correctly forward deserialize_i128 / deserialize_u128 to the original… Correctly forward number of methods to the originals in Compact / Readable wrappers Jul 28, 2024
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

I am merging this without the 8th commit (__private_visit_untagged_option). That is not provided by serde as a public API, and using it from this crate would expose the risk of serde_test failing to compile if the signature of that function changes in any future version of serde. I am open to other options for serving that use case, including potentially stabilizing an API in serde if there is one that makes sense to provide publicly.

@dtolnay dtolnay merged commit dd1a6b4 into serde-rs:master Aug 5, 2024
8 checks passed
@Mingun Mingun deleted the forward128 branch August 5, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Readable/Compact still give "must use Configure" error when serializing Map
2 participants