From 81bed37114d1675a36009e2a7c1b468eb70340b8 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Mon, 10 Feb 2025 12:46:41 +0000 Subject: [PATCH] fix(span): `f64::content_eq` return `false` for `0` and `-0` (#9007) Fixes #8982. Previously `0f64.content_eq(-0f64)` returned `true`. Now it returns `false`. This also affects the behavior for `NaN`. Previously `f64::NAN.content_eq(f64::NAN)` returned `false`. Now it returns `true`. But it's complicated: ```rs f64::NAN.content_eq(f64::NAN) == true f64::NAN.content_eq(-f64::NAN) == false f64::NAN.content_eq(--f64::NAN) == true ``` This does *not* align with JS's `Object.is` which returns `true` for *any* two `NaN` values. If this matters, we could instead implement `f64::content_eq` as: ```rs if self.is_nan() && other.is_nan() { true } else { self.to_bits() == other.to_bits() } ``` But this generates quite a lot of assembly for all `f64` values, just to cover this small edge case with `NaN`: https://godbolt.org/z/q9zYodn4e So I suggest that we go with it as in this PR for now, and see if `NaN` causes us problems in reality or not. --- crates/oxc_span/src/cmp.rs | 45 +++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/crates/oxc_span/src/cmp.rs b/crates/oxc_span/src/cmp.rs index 95f985d4bdb4d..946d24a292886 100644 --- a/crates/oxc_span/src/cmp.rs +++ b/crates/oxc_span/src/cmp.rs @@ -34,6 +34,48 @@ impl ContentEq for () { } } +/// Compare `f64` as bits instead of using `==`. +/// +/// Result is the same as `partial_eq` (`==`), with the following exceptions: +/// +/// * `+0` and `-0` are not `content_eq` (they are `partial_eq`). +/// * `f64::NAN` and `f64::NAN` are `content_eq` (they are not `partial_eq`). +/// +/// +/// +/// ### NaN +/// +/// Comparison of `NaN` is complicated. From Rust's docs for `f64`: +/// +/// > Note that IEEE 754 doesn’t define just a single NaN value; +/// > a plethora of bit patterns are considered to be NaN. +/// +/// +/// +/// If either value is `NaN`, `f64::content_eq` only returns `true` if both are the *same* `NaN`, +/// with the same bit pattern. This means, for example: +/// +/// ``` +/// f64::NAN.content_eq(f64::NAN) == true +/// f64::NAN.content_eq(-f64::NAN) == false +/// f64::NAN.content_eq(--f64::NAN) == true +/// ``` +/// +/// Any other `NaN`s which are created through an arithmetic operation, rather than explicitly +/// with `f64::NAN`, are not guaranteed to equal `f64::NAN`. +/// +/// ``` +/// // This results in `false` on at least some flavors of `x84_64`, +/// // but that's not specified - could also result in `true`! +/// (-1f64).sqrt().content_eq(f64::NAN) == false +/// ``` +impl ContentEq for f64 { + #[inline] + fn content_eq(&self, other: &Self) -> bool { + self.to_bits() == other.to_bits() + } +} + /// Blanket implementation for [Option] types impl ContentEq for Option { #[inline] @@ -77,8 +119,6 @@ impl ContentEq for oxc_allocator::Vec<'_, T> { } mod content_eq_auto_impls { - #![expect(clippy::float_cmp)] - use super::ContentEq; macro_rules! content_eq_impl { @@ -97,6 +137,5 @@ mod content_eq_auto_impls { bool isize usize u8 u16 u32 u64 u128 i8 i16 i32 i64 i128 - f32 f64 } }