Skip to content

Commit

Permalink
Merge pull request #2637 from dtolnay/nansign
Browse files Browse the repository at this point in the history
Ensure f32 deserialized from f64 and vice versa preserve NaN sign
  • Loading branch information
dtolnay authored Oct 26, 2023
2 parents bb4135c + 6ba9c12 commit 11c2917
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
6 changes: 6 additions & 0 deletions serde/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ fn main() {
println!("cargo:rustc-cfg=no_relaxed_trait_bounds");
}

// f32::copysign and f64::copysign stabilized in Rust 1.35.
// https://blog.rust-lang.org/2019/05/23/Rust-1.35.0.html#copy-the-sign-of-a-floating-point-number-onto-another
if minor < 35 {
println!("cargo:rustc-cfg=no_float_copysign");
}

// Current minimum supported version of serde_derive crate is Rust 1.56.
if minor < 56 {
println!("cargo:rustc-cfg=no_serde_derive");
Expand Down
26 changes: 24 additions & 2 deletions serde/src/de/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,28 @@ macro_rules! num_as_self {
};
}

macro_rules! num_as_copysign_self {
($ty:ident : $visit:ident) => {
#[inline]
fn $visit<E>(self, v: $ty) -> Result<Self::Value, E>
where
E: Error,
{
#[cfg(any(no_float_copysign, not(feature = "std")))]
{
Ok(v as Self::Value)
}

#[cfg(all(not(no_float_copysign), feature = "std"))]
{
// Preserve sign of NaN. The `as` produces a nondeterministic sign.
let sign = if v.is_sign_positive() { 1.0 } else { -1.0 };
Ok((v as Self::Value).copysign(sign))
}
}
};
}

macro_rules! int_to_int {
($ty:ident : $visit:ident) => {
#[inline]
Expand Down Expand Up @@ -351,15 +373,15 @@ impl_deserialize_num! {
impl_deserialize_num! {
f32, deserialize_f32
num_self!(f32:visit_f32);
num_as_self!(f64:visit_f64);
num_as_copysign_self!(f64:visit_f64);
num_as_self!(i8:visit_i8 i16:visit_i16 i32:visit_i32 i64:visit_i64);
num_as_self!(u8:visit_u8 u16:visit_u16 u32:visit_u32 u64:visit_u64);
}

impl_deserialize_num! {
f64, deserialize_f64
num_self!(f64:visit_f64);
num_as_self!(f32:visit_f32);
num_as_copysign_self!(f32:visit_f32);
num_as_self!(i8:visit_i8 i16:visit_i16 i32:visit_i32 i64:visit_i64);
num_as_self!(u8:visit_u8 u16:visit_u16 u32:visit_u32 u64:visit_u64);
}
Expand Down
21 changes: 21 additions & 0 deletions test_suite/tests/test_de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#![cfg_attr(feature = "unstable", feature(never_type))]

use fnv::FnvHasher;
use serde::de::value::{F32Deserializer, F64Deserializer};
use serde::de::{Deserialize, DeserializeOwned, Deserializer, IntoDeserializer};
use serde_derive::Deserialize;
use serde_test::{assert_de_tokens, Configure, Token};
Expand Down Expand Up @@ -832,6 +833,26 @@ fn test_f64() {
test(1.11, &[Token::F64(1.11)]);
}

#[test]
fn test_nan() {
let f32_deserializer = F32Deserializer::<serde::de::value::Error>::new;
let f64_deserializer = F64Deserializer::<serde::de::value::Error>::new;

let pos_f32_nan = f32_deserializer(f32::NAN.copysign(1.0));
let pos_f64_nan = f64_deserializer(f64::NAN.copysign(1.0));
assert!(f32::deserialize(pos_f32_nan).unwrap().is_sign_positive());
assert!(f32::deserialize(pos_f64_nan).unwrap().is_sign_positive());
assert!(f64::deserialize(pos_f32_nan).unwrap().is_sign_positive());
assert!(f64::deserialize(pos_f64_nan).unwrap().is_sign_positive());

let neg_f32_nan = f32_deserializer(f32::NAN.copysign(-1.0));
let neg_f64_nan = f64_deserializer(f64::NAN.copysign(-1.0));
assert!(f32::deserialize(neg_f32_nan).unwrap().is_sign_negative());
assert!(f32::deserialize(neg_f64_nan).unwrap().is_sign_negative());
assert!(f64::deserialize(neg_f32_nan).unwrap().is_sign_negative());
assert!(f64::deserialize(neg_f64_nan).unwrap().is_sign_negative());
}

#[test]
fn test_char() {
test('a', &[Token::Char('a')]);
Expand Down

0 comments on commit 11c2917

Please sign in to comment.