Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arbitrary_precision leads to issues with adjacently tagged enums #959

Closed
notonamap opened this issue Nov 30, 2022 · 2 comments
Closed

arbitrary_precision leads to issues with adjacently tagged enums #959

notonamap opened this issue Nov 30, 2022 · 2 comments

Comments

@notonamap
Copy link

I see that there are a couple open issues around arbitrary_precision which are likely related. Leaving this here for people who ran into this particular issue.

When tag and content are reversed in an adjacently tagged repr, deserialization fails for floats:

use serde::Deserialize;
use serde_json::from_str;

fn main() {
    #[derive(Deserialize, Debug)]
    #[serde(tag = "t", content = "c")]
    enum Foo {
        A(u64),
        B(f64),
        C(bool),
    }
    println!("{:?}", from_str::<Foo>(r#"{"t":"A","c":1}"#));
    println!("{:?}", from_str::<Foo>(r#"{"t":"B","c":3.14}"#));
    println!("{:?}", from_str::<Foo>(r#"{"t":"C","c":true}"#));

    println!("{:?}", from_str::<Foo>(r#"{"c":1,"t":"A"}"#));
    println!("{:?}", from_str::<Foo>(r#"{"c":3.14,"t":"B"}"#));
    println!("{:?}", from_str::<Foo>(r#"{"c":true,"t":"C"}"#));
}

Output:

Ok(A(1))
Ok(B(3.14))
Ok(C(true))
Ok(A(1))
Err(Error("invalid type: map, expected f64", line: 1, column: 18))
Ok(C(true))
@noprofessional
Copy link

After researched this a couple of days, I don't think there is an easy way to fix this.

println!("{:?}", from_str::<Foo>(r#"{"c":3.14,"t":"B"}"#));

This is what happened. First, json deserializer will generate a value

ParserNumber::String(String) // where String is "3.14"

[1] Then since tag is not known at this point, it will be converted to Content

Content::Map(Vec<(Content, Content)>) 
// where Vec only has one element (Content::Str("$serde_json::private::Number"), Content::String(String)
// where String value is "3.14"

[2] Then tag is readed, serde will try deserialize to f64 from Content Deserializer which will call this function internally

ContentDeserializer::deserialize_f64()
// only return Ok if contained Content is Number type
// but now Content is Map type which cause the 
// Err(Error("invalid type: map, expected f64", line: 1, column: 18))

We can change [1] , so that when we convert to Content, Content::F64 is the final result. Then step [2] will succeed. But, this is a breaking change.

Or we can change [2], let Content::Map support converting f64. But the map format is only used in serde_json, which means we will bring implement details of serde_json [arbitrary_precision] into serde. This is not a good method

Or We can change [1], so that auto implement of Deserialize will not use Content as the temporary buffer instead use a deserailzer related type(Value in this case) ?

Just some thoughts.

@dtolnay
Copy link
Member

dtolnay commented Jan 2, 2023

Or We can change [1], so that auto implement of Deserialize will not use Content as the temporary buffer instead use a deserailzer related type(Value in this case) ?

Yes this is the plan, it's blocked on associated type defaults at the moment. This is tracked in serde-rs/serde#1183.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants