Fix 128bit support when deserializing to Rust types

[mikonieminen]

This fixes support for 128bit values and it is tested currently with simd-json, because that's what I needed first and how to fix serde_json was not right away obvious to me.

This fixes #1719

[mikonieminen]

How should I address separation of 128bit supported version of the code below and non-128bit version?

[mikonieminen]

@dtolnay could you have a look at this PR?It should be quite obvious if I'm doing the right thing. Only thing I'm not sure about, is the one where I added fixme comment.

I tested it against simd-json where it solved the related issue.

[dtolnay]

Thanks for the PR! @mikonieminen I am leaning toward not accepting this, since additions to Content tend to be extremely expensive to compile time. Basically this would cause ~25% increase in compile time of untagged enums and externally tagged enums due to the greater number of visitor methods instantiated when deserializing from Content, in service of a use case that is quite rare (most codebases do not use 128 bit integers at all, and of those the subset to use one inside untagged enums is even smaller) which is not clearly the right tradeoff.

[mikonieminen]

@dtolnay thanks for the reply. Does this mean that you're planning to remove 128bit support completely from serde? If not, I find it quite confusing that some things work with 128bit values while others do not. I completely understand the worry over increased compilation time. Maybe 128bit support should be turned into a non-default feature so that if one needs it, it's available, while by default it's disabled? It would be a shame if serde cannot be used when one needs support for 128bit integers. I suppose this also brings question for supporting arbitrary large integers, but I think support for 128bit integers is a valid separate feature since the language has native support for these.

So in short, what would you say if 128bit integer support would be turned into a non-default feature instead of automatically enabling it if you have new enough Rust version?

[dunnock]

@dtolnay would it make things better if this would be behind a feature flag i128 just similar to how arbitrary_precision is?

[c410-f3r]

cc #1679

[mikonieminen]

@dtolnay would it be sensible to use some better media where we could discuss this and other topics like 256 bit and arbitrary precision support? You probably know who should join such discussion. Maybe we could have a call, discuss in discord channel or start the topic in users.rust-lang.org, what would you prefer?

What I'm worried is that arbitrary precision support would not perform that well compared to fixed 128 or 256 bit support and usually when (de)serializing your data, you already know if 64 bit is enough or you need 128 or 256 bit versions. I think arbitrary precision is seldom really needed and for example in my use cases that performance penalty would be unacceptable.

I would suggest that default implementation is 64 bit only, while 128 or 256 bit support (maybe even arbitrary precision) would be a feature that you need to enable explicitly.

Any thoughts?

[dtolnay]

I'll go ahead and close this PR. Thanks anyway! I think the right fix here is going to be #1183 followed by using a JSON-specific type (serde_json::Value, serde_json::value::RawValue, or something like it) instead of Content as the transient buffer when deserializing untagged or internally tagged enums from JSON.