From 6ea0c1a7a65f9330194a1338525d2e9a7f4bf7c8 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 27 Oct 2023 09:27:17 -0700 Subject: [PATCH 1/2] feat: Serialize NaN without sign (serde only) --- crates/toml/src/value.rs | 10 ++++++---- crates/toml/tests/testsuite/float.rs | 4 ++-- crates/toml_edit/src/ser/value.rs | 20 ++++++++++++++------ crates/toml_edit/src/value.rs | 1 + 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/crates/toml/src/value.rs b/crates/toml/src/value.rs index c8065a6c..e6f832a7 100644 --- a/crates/toml/src/value.rs +++ b/crates/toml/src/value.rs @@ -921,12 +921,14 @@ impl ser::Serializer for ValueSerializer { } fn serialize_f32(self, value: f32) -> Result { - // Preserve sign of NaN. The `as` produces a nondeterministic sign. - let sign = if value.is_sign_positive() { 1.0 } else { -1.0 }; - self.serialize_f64((value as f64).copysign(sign)) + self.serialize_f64(value as f64) } - fn serialize_f64(self, value: f64) -> Result { + fn serialize_f64(self, mut value: f64) -> Result { + // Discard sign of NaN. See ValueSerializer::serialize_f64. + if value.is_nan() { + value = value.copysign(1.0); + } Ok(Value::Float(value)) } diff --git a/crates/toml/tests/testsuite/float.rs b/crates/toml/tests/testsuite/float.rs index d0081347..1d9ce1a3 100644 --- a/crates/toml/tests/testsuite/float.rs +++ b/crates/toml/tests/testsuite/float.rs @@ -47,7 +47,7 @@ macro_rules! float_inf_tests { assert!(inf.sf5.is_nan()); assert!(inf.sf5.is_sign_positive()); assert!(inf.sf6.is_nan()); - assert!(inf.sf6.is_sign_negative()); + assert!(inf.sf6.is_sign_negative()); // NOTE: serializes to just `nan` assert_eq!(inf.sf7, 0.0); assert!(inf.sf7.is_sign_positive()); @@ -63,7 +63,7 @@ sf2 = inf sf3 = -inf sf4 = nan sf5 = nan -sf6 = -nan +sf6 = nan sf7 = 0.0 sf8 = -0.0 " diff --git a/crates/toml_edit/src/ser/value.rs b/crates/toml_edit/src/ser/value.rs index f250d63d..47a17a3b 100644 --- a/crates/toml_edit/src/ser/value.rs +++ b/crates/toml_edit/src/ser/value.rs @@ -105,12 +105,20 @@ impl serde::ser::Serializer for ValueSerializer { } fn serialize_f32(self, v: f32) -> Result { - // Preserve sign of NaN. The `as` produces a nondeterministic sign. - let sign = if v.is_sign_positive() { 1.0 } else { -1.0 }; - self.serialize_f64((v as f64).copysign(sign)) - } - - fn serialize_f64(self, v: f64) -> Result { + self.serialize_f64(v as f64) + } + + fn serialize_f64(self, mut v: f64) -> Result { + // Discard sign of NaN when serialized using Serde. + // + // In all likelihood the sign of NaNs is not meaningful in the user's + // program. Ending up with `-nan` in the TOML document would usually be + // surprising and undesirable, when the sign of the NaN was not + // intentionally controlled by the caller, or may even be + // nondeterministic if it comes from arithmetic operations or a cast. + if v.is_nan() { + v = v.copysign(1.0); + } Ok(v.into()) } diff --git a/crates/toml_edit/src/value.rs b/crates/toml_edit/src/value.rs index f10da9a4..62eb30c7 100644 --- a/crates/toml_edit/src/value.rs +++ b/crates/toml_edit/src/value.rs @@ -284,6 +284,7 @@ impl From for Value { impl From for Value { fn from(f: f64) -> Self { + // Preserve sign of NaN. It may get written to TOML as `-nan`. Value::Float(Formatted::new(f)) } } From 7b359a840194f5c5caf4f3b2bb471d4052717182 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 27 Oct 2023 09:33:31 -0700 Subject: [PATCH 2/2] test: Add toml_edit floats test --- crates/toml_edit/tests/testsuite/float.rs | 60 +++++++++++++++++++++++ crates/toml_edit/tests/testsuite/main.rs | 1 + 2 files changed, 61 insertions(+) create mode 100644 crates/toml_edit/tests/testsuite/float.rs diff --git a/crates/toml_edit/tests/testsuite/float.rs b/crates/toml_edit/tests/testsuite/float.rs new file mode 100644 index 00000000..34e792ed --- /dev/null +++ b/crates/toml_edit/tests/testsuite/float.rs @@ -0,0 +1,60 @@ +use toml_edit::Document; + +macro_rules! float_inf_tests { + ($ty:ty) => {{ + let document = r" + # infinity + sf1 = inf # positive infinity + sf2 = +inf # positive infinity + sf3 = -inf # negative infinity + + # not a number + sf4 = nan # actual sNaN/qNaN encoding is implementation specific + sf5 = +nan # same as `nan` + sf6 = -nan # valid, actual encoding is implementation specific + + # zero + sf7 = +0.0 + sf8 = -0.0 + "; + + let document = document.parse::().unwrap(); + let float = |k| document[k].as_float().unwrap(); + + assert!(float("sf1").is_infinite()); + assert!(float("sf1").is_sign_positive()); + assert!(float("sf2").is_infinite()); + assert!(float("sf2").is_sign_positive()); + assert!(float("sf3").is_infinite()); + assert!(float("sf3").is_sign_negative()); + + assert!(float("sf4").is_nan()); + assert!(float("sf4").is_sign_positive()); + assert!(float("sf5").is_nan()); + assert!(float("sf5").is_sign_positive()); + assert!(float("sf6").is_nan()); + assert!(float("sf6").is_sign_negative()); + + assert_eq!(float("sf7"), 0.0); + assert!(float("sf7").is_sign_positive()); + assert_eq!(float("sf8"), 0.0); + assert!(float("sf8").is_sign_negative()); + + let mut document = Document::new(); + document["sf4"] = toml_edit::value(f64::NAN.copysign(1.0)); + document["sf6"] = toml_edit::value(f64::NAN.copysign(-1.0)); + assert_eq!( + document.to_string(), + "\ +sf4 = nan +sf6 = -nan +" + ); + }}; +} + +#[test] +fn test_float() { + float_inf_tests!(f32); + float_inf_tests!(f64); +} diff --git a/crates/toml_edit/tests/testsuite/main.rs b/crates/toml_edit/tests/testsuite/main.rs index 1476c5da..0592f3c0 100644 --- a/crates/toml_edit/tests/testsuite/main.rs +++ b/crates/toml_edit/tests/testsuite/main.rs @@ -3,6 +3,7 @@ mod convert; mod datetime; mod edit; +mod float; mod invalid; mod parse; mod stackoverflow;