Use a custom deserializer instead of wrapper types

[romac]

No description provided.

[rnbguy]

To document how I solved the num_bigin::BigInt deserialization.

The ITF trace deserialization to native types works as follows,

1. deserialize raw itf JSON to serde_json::Value
2. then deserializing serde_json::Value to itf::value::Value (impl<'de> Deserialize<'de> for itf::value::Value)
3. then deserializing itf::value::Value to T (impl<'de> Deserializer<'de> for itf::value::Value)

To get native deserialization of num_bigint::BigInt, itf::value::Value::BigInt needs to be deserialized as (int, [uint]). I added a56363c for this. And, 92bb9d3 allows itf::value::Value::BigInt to be deserialized as a tuple.

But this creates an weird issue - itf::from_str::<itf::value::Value> will deserialize {"#bigint": "99"} as [1, [99]]. As itf::from_str will deserialize itf-bigint format to itf::value::Value and then again to itself. The second deserialization makes it a tuple and itf::value::Value does not have any information on how to deserialize it back to itf::value::Value::BigInt. So I added c03bec1.

Note, with this, itf::from_str does not differentiate between {"#bigint": "99"} and [1, [99]]; they both can be deserialized to num_bigint::BigInt or any native integer types.

[rnbguy]

Note, this does not support num_bigint::BigUint, which has a different deserialization format than num_bigint::BigInt.

• BigInt: (int, [uint])
• BigUint: [uint]

This is definitely feasible. But probably, not necessary as ITF doesn't include unsigned integers.

[rnbguy]

Value::deserialize(Value) is creating equivalency issues for Value::{Set, Map, List} too. I added testcases in d89d2fb.

I suggest making itf::Value private. This way we can also revert c03bec1 🙂

[romac]

Amazing stuff @rnbguy! Thanks so much!

I suggest making itf::Value private. This way we can also revert c03bec1 🙂

Yeah let's do that!

[romac]

Good idea!

[rnbguy]

@romac any idea why this (9920493) is not working?

itf-rs/itf/tests/regression.rs

Lines 141 to 154 in 9920493

	let itf = serde_json::json!({
	"_foo": {"#bigint": "1"},
	"typ": "Foo",
	});
	#[derive(Deserialize, Debug)]
	#[serde(tag = "typ")]
	enum FooBarInt {
	// try to deserialize _foo as i64, instead of BigInt
	Foo { _foo: i64 },
	Bar { _bar: String },
	}
	assert!(itf::from_value::<FooBarInt>(itf.clone()).is_err());

Update: I realized why ! In this case, deserialize_any is being called - which always deserializes #bigint to num_bigint::BigInt.

itf-rs/itf/src/de.rs

Line 120 in 9920493

Value::BigInt(v) => visit_bigint(v, visitor),

IMO, this requires something like a TryVisitor trait, which allows one to visit an itf::Value multiple times until success. I am not sure if it's possible. Otherwise, we should not let users deserialize #bigint to i64 to avoid confusion.

[romac]

Ah good catch! Yeah it's a shame we can't seem to support this.

That said, we can perhaps still enable this by providing a from_bigint<'de, A: TryFrom<BigInt>, D>(d: D) -> Result<A, D::Error> helper function which could be used with #[serde(deserialize_with = "itf::de::from_bigint")].

See a2eef53

What do you think?

[rnbguy]

Yea serde(with) is the last straw 😅

Btw. I would prefer serde_with::DeserializeAs with serde_with::As. This adds a dependency to serde_with. But this way, we can support container types too.

At itf-rs crate,

impl<'de, T> serde_with::DeserializeAs<'de, T> for itf::value::BigInt
where
    T: TryFrom<num_bigint::BigInt>
{ ... }

Usage,

use itf::value::BigInt;
use serde_with::As;

#[derive(Deserialize, Debug)]
enum FooBarWithInt {
    Foo {
        #[serde(with = "As::<Vec<Vec<BigInt>>>")]
        _foo: Vec<Vec<i64>>,
    },
    Bar {
        #[serde(with = "As::<HashMap<BigInt, BigInt>>")]
        _bar: HashMap<i64, i64>,
    },
}

I can take care of the changes tomorrow 🙂

[romac]

Ah good point, didn't think of containers! I am fine with adding serde-with as a dep :)

[rnbguy]

I removed the code that implicitly serialized BigInt to (u)int and vice-versa to avoid ambiguity while deserialization (see #6 (comment)). Now, {"#bigint": "<num>"} is always deserialized to BigInt.

For Rust primitive integer types, a user has to explicitly use serde-with with serde_with::As (e.g. #[serde(with = "As::<itf::de::Integer>")] in #6 (comment)) or serde_with::serde_as attribute accordingly.

[rnbguy]

Btw, serde_with provides TryFromInto - which can be used in our context as follows - #[serde(with = "As::<TryFromInto<BigInt>>")]. itf::de::Integer is just a convenient shorthand for the same.

[romac]

Awesome!

[rnbguy]

maybe it makes sense to redefine Integer as just

pub type Integer = TryFromInto<BigInt>;