Add support for 128-bit integers in flatten structs and internally tagged enums#2781
Add support for 128-bit integers in flatten structs and internally tagged enums#2781Mingun wants to merge 8 commits intoserde-rs:masterfrom
Conversation
|
This looks like a pretty clean fix. @dtolnay would you take a look please? |
Mingun
force-pushed
the
integer128
branch
2 times, most recently
from
8126c2c to
8d5f1d1
Compare
|
Ping? |
|
Up |
(tests\ui\unimplemented\required_by_dependency.rs broken because of changed paths to serde_test)
(review this commit with "ignore whitespace changes" option on)
failures (5):
newtype::enum_::newtype (deserialize)
newtype::enum_::struct_ (serialize)
newtype::enum_::tuple (serialize)
newtype::struct_ (deserialize)
struct_ (deserialize)
…of internally tagged enums
Deserialization not checked because there is no sequence of tokens
that could represent test values
failures (7):
newtype::enum_::newtype (deserialize)
newtype::enum_::struct (serialize)
newtype::enum_::tuple (serialize)
newtype::i128 (serialize)
newtype::struct_ (deserialize)
newtype::u128 (serialize)
struct_ (deserialize)
failures (8):
newtype::enum_::newtype (deserialize)
newtype::enum_::struct_ (serialize)
newtype::enum_::tuple (serialize)
newtype::i128 (serialize)
newtype::newtype_struct (deserialize)
newtype::struct_ (deserialize)
newtype::u128 (serialize)
struct_ (deserialize)
`Content` size doesn't changed, because it is already big enough
(64 bytes due to `TupleVariant` and `StructVariant`).
Fixes (2):
newtype::i128 (serialize)
newtype::u128 (serialize)
failures (6):
newtype::enum_::newtype (deserialize)
newtype::enum_:;struct (deserialize)
newtype::enum_::tuple (deserialize)
newtype::newtype_struct (deserialize)
newtype::struct_ (deserialize)
struct_ (deserialize)
…y tagged enums
Fixed (6):
newtype::enum_::newtype
newtype::enum_::struct_
newtype::enum_::tuple
newtype::newtype_struct
newtype::struct_
struct_
|
@Mingun thanks for this PR. I tried it out for my case. It partially fixes the problem I have, hence I comment directly on this PR. In my case I want to be able to deserialize a It applies to all signed integer type, but let's make things concrete and use A workaround is adding in the Does this belong to this PR or would it be a separate PR? Do you think that would be the correct fix for the issue? I sadly can't provide a simple test case for it. Mine is pretty entangled as it involves even a Deserializer implementation and I wasn't able to create a simpler version. |
|
@vmx, you're right, I should update my PR |
|
For those running into this issue: if you have the chance to use a different numeric type, like |
| serde = { path = "../serde", features = ["rc"] } | ||
| serde_derive = { path = "../serde_derive", features = ["deserialize_in_place"] } | ||
| serde_test = "1.0.176" | ||
| serde_test = { git = "https://github.com/Mingun/serde-test", branch = "integer128" } |
There was a problem hiding this comment.
Of course. Corresponding PR should be merged first.
7 participants
This PR depends on serde-deprecated/test#6 and currently temporary uses my fork with branch that adds necessary support for testing 128-bit integers. It is possible, however, drop the first two commits, but that will mean that 128-bit integers will not be tested. I'd prefer to merge serde-deprecated/test#6 and then update this PR to use new version of
serde_test.Closes #1682, closes #2230, closes #2576, closes #2948