de::tests::test_any_number_precision and de::tests::test_value_special_floats fail on riscv64.

[plugwash]

Ron 0.12.0 was recently uploaded to debian and we observed failures in our CI on riscv64 (tests passed on all other tested architectures)

https://ci.debian.net/packages/r/rust-ron/testing/riscv64/66493262/

6181s ---- de::tests::test_any_number_precision stdout ----
6181s 
6181s thread 'de::tests::test_any_number_precision' panicked at src/de/tests.rs:655:5:
6181s assertion `left == right` failed
6181s   left: F64(F64(NaN))
6181s  right: F32(F32(NaN))
6181s stack backtrace:
6181s    0: __rustc::rust_begin_unwind
6181s              at /usr/src/rustc-1.90.0/library/std/src/panicking.rs:697:5
6181s    1: core::panicking::panic_fmt
6181s              at /usr/src/rustc-1.90.0/library/core/src/panicking.rs:75:14
6181s    2: core::panicking::assert_failed_inner
6181s    3: core::panicking::assert_failed
6181s              at /usr/src/rustc-1.90.0/library/core/src/panicking.rs:403:5
6181s    4: ron::de::tests::check_de_any_number
6181s              at ./src/de/tests.rs:655:5
6181s    5: ron::de::tests::test_any_number_precision
6181s              at ./src/de/tests.rs:675:5
6181s    6: ron::de::tests::test_any_number_precision::{{closure}}
6181s              at ./src/de/tests.rs:663:31
6181s    7: core::ops::function::FnOnce::call_once
6181s              at /usr/src/rustc-1.90.0/library/core/src/ops/function.rs:253:5
6181s    8: core::ops::function::FnOnce::call_once
6181s              at /usr/src/rustc-1.90.0/library/core/src/ops/function.rs:253:5
6181s note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
6181s 
6181s ---- de::tests::test_value_special_floats stdout ----
6181s 
6181s thread 'de::tests::test_value_special_floats' panicked at src/de/tests.rs:711:5:
6181s assertion `left == right` failed
6181s   left: Ok(Number(F64(F64(NaN))))
6181s  right: Ok(Number(F32(F32(NaN))))
6181s stack backtrace:
6181s    0: __rustc::rust_begin_unwind
6181s              at /usr/src/rustc-1.90.0/library/std/src/panicking.rs:697:5
6181s    1: core::panicking::panic_fmt
6181s              at /usr/src/rustc-1.90.0/library/core/src/panicking.rs:75:14
6181s    2: core::panicking::assert_failed_inner
6181s    3: core::panicking::assert_failed
6181s              at /usr/src/rustc-1.90.0/library/core/src/panicking.rs:403:5
6181s    4: ron::de::tests::test_value_special_floats
6181s              at ./src/de/tests.rs:711:5
6181s    5: ron::de::tests::test_value_special_floats::{{closure}}
6181s              at ./src/de/tests.rs:700:31
6181s    6: core::ops::function::FnOnce::call_once
6181s              at /usr/src/rustc-1.90.0/library/core/src/ops/function.rs:253:5
6181s    7: core::ops::function::FnOnce::call_once
6181s              at /usr/src/rustc-1.90.0/library/core/src/ops/function.rs:253:5
6181s note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I bisected the issue and the first "bad" commit was b7acecb099f18826e0c4b09b80188a0a7fc1859f

The failing testcases seem to be newly added in that commit.

[plugwash]

The problem seems to be specifically with the "negative NaN" testcases.

[juntyr]

Hah, oh no ...

So when we're parsing a number that could be f32 or f64, we first parse it as f64 and then cast to f32 and back to f64 to see if the f32 value is equivalent, in that case we say the number is f32 (see https://github.com/ron-rs/ron/blob/c2d90f6d40948d8566d3cc906565852143567a9c/src/parse.rs#L1577).

What seems to be happening is that f64::from(f64::from_str("-NaN") as f32) doesn't total compare equal to f64::from_str("-NaN"). How does riscv64 handle NaNs?

[plugwash]

I'm not a riscv64 expert, but I wrote a simple test program and ran it on riscv64.

use std::str::FromStr;
fn main() {
    println!("{:#066b}",f64::from(f64::from_str("-NaN").unwrap() as f32).to_bits());
    println!("{:#034b}",(f64::from_str("-NaN").unwrap() as f32).to_bits());
    println!("{:#066b}",f64::from_str("-NaN").unwrap().to_bits());
}

Returned

0b0111111111111000000000000000000000000000000000000000000000000000
0b01111111110000000000000000000000
0b1111111111111000000000000000000000000000000000000000000000000000

So it looks like the sign bit is lost in the conversion to f32.

Reading the float-semantics rfc (which is not accepted yet, but seems to be the best documentation of what the compiler intends to do) it seems that nothing is gauranteed with regard to the signedness of NaNs returned from operations.

Edit: it seems the rfc has been accepted but the documentation has not been updated yet.

[juntyr]

Mmmm. We definitely do not guarantee the bit pattern of the parsed NaNs in RON, but preserving the sign would be good. Perhaps we could add a special NaN path, since NaNs in f64 are always convertible to f32, that uses f32::NAN.copysign(if value.is_sign_negative() { -1.0 } else {1.0}). What do you think @plugwash?