Add support for 128-bit integers in flatten structs and internally tagged enums #2781
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 |
5 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