Use TryInto for more permissive deserialization for integers #353
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Attempt to convert between integer types using `TryInto`-based conversions rather than blanket failing for some source and destination types. * Use `into_uint` instead of `into_int` in `Value` Deserialize implementations for unsigned integer types. Previously, we were converting from signed types to unsigned types using `as`, which can lead to surprise integer values conversions (rust-cli#93). Fixes rust-cli#352 and rust-cli#93
kesyog
force-pushed
the
kyogeswaran/deserialize-uints
branch
from
a1dd5c7
to
7db2e8b
Compare
|
@matthiasbeyer, I've cleaned up my draft implementation and added a few tests. Your review and CI workflow approval would be appreciated. |
matthiasbeyer
approved these changes
Jun 29, 2022
|
No problem! It looks like all checks have passed and it's ready for merge whenever you're ready. It'd be great to get the new tests in before #338 to ensure that the functionality doesn't regress. |
|
Yes! #338 has to be rebased onto latest master now, of course 😆 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TryInto-basedconversions rather than blanket failing for some source and
destination types.
into_uintinstead ofinto_intinValueDeserializeimplementations for unsigned integer types. Previously, we were
converting from signed types to unsigned types using
as, which canlead to surprise integer values conversions (Deserializing into a type with an unsigned integer doesn't give the expected result #93).
Fixes #352 and #93