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

Fix number deserialization with arbitrary_precision #586

Closed
wants to merge 3 commits into from

Conversation

bkchr
Copy link

@bkchr bkchr commented Nov 21, 2019

When we have a struct that uses #[serde(flatten)] and the feature
arbitrary_precision is enabled, the deserialization. The problem was
that the arbitrary number was forwarded as a "map" to the visitor and in
a later step this map failed as a number was expected.

Fixes: #505

Requires: serde-rs/serde#1679

When we have a struct that uses `#[serde(flatten)]` and the feature
`arbitrary_precision` is enabled, the deserialization. The problem was
that the arbitrary number was forwarded as a "map" to the visitor and in
a later step this map failed as a number was expected.

Fixes: serde-rs#505
@@ -956,21 +956,6 @@ fn test_parse_number() {
("\t1.0", "invalid number at line 1 column 1"),
("1.0\t", "invalid number at line 1 column 4"),
]);

#[cfg(feature = "arbitrary_precision")]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this test + the extra added function from_string_unchecked. In my opinion this test does not makes that much sense at all. Even if we would have parsed the 1e999 correctly into the Number, there is not way to get this value out of the object.

@WiSaGaN
Copy link

WiSaGaN commented Nov 29, 2019

The patch seems still failing the test, which would have been passed without "arbitrary_precision" feature.

use serde::{Deserialize};

#[derive(Deserialize)]
#[serde(tag = "tag")]
#[derive(Clone, Debug, PartialEq)]
pub enum E1 {
    A{v: f64},
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn serde_1() {
        let incoming_str = r#"{"tag":"A","v":0}"#;
        let expected_incoming = E1::A{ v: 0.0 };
        let incoming = serde_json::from_str(&incoming_str).expect("cannot deserialize incoming_str");
        assert_eq!(expected_incoming, incoming);
    }
}

"invalid type: unexpected i128, expected f64", line: 0, column: 0

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bkchr, thanks for the PR but I can't accept this because it breaks the following:

// [dependencies]
// serde_json = { version = "1.0", features = ["arbitrary_precision"] }

fn main() {
    println!("{}", serde_json::from_str::<serde_json::Value>("1e999").unwrap());
}

This is what is tested by the test you removed. The point of arbitrary_precision is to allow deserializing and round-tripping arbitrarily large numbers without losing precision.

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

Successfully merging this pull request may close these issues.

Arbitrary precision numbers don't work with serde(tag = ..) and serde(flatten)
3 participants