diff --git a/.changeset/fix-number-hash.md b/.changeset/fix-number-hash.md new file mode 100644 index 000000000000..725370dadb15 --- /dev/null +++ b/.changeset/fix-number-hash.md @@ -0,0 +1,7 @@ +--- +ast_node: minor +swc_core: major +swc_ecma_ast: major +--- + +fix(es/ast): Support NaN and infinity in Number while preserving Hash. diff --git a/crates/ast_node/src/ast_node_macro.rs b/crates/ast_node/src/ast_node_macro.rs index 1b76b3dfa296..faabde72dc60 100644 --- a/crates/ast_node/src/ast_node_macro.rs +++ b/crates/ast_node/src/ast_node_macro.rs @@ -7,11 +7,29 @@ use syn::{ #[derive(Clone)] pub struct Args { pub ty: Literal, + pub no_partial_eq: bool, } impl Parse for Args { fn parse(i: ParseStream<'_>) -> syn::Result { - Ok(Args { ty: i.parse()? }) + let ty = i.parse()?; + let mut no_partial_eq = false; + + if i.parse::>()?.is_some() { + let option: Ident = i.parse()?; + + if option == "no_partial_eq" { + no_partial_eq = true; + } else { + return Err(Error::new(option.span(), "unknown ast_node option")); + } + } + + if !i.is_empty() { + return Err(i.error("unexpected ast_node arguments")); + } + + Ok(Args { ty, no_partial_eq }) } } diff --git a/crates/ast_node/src/lib.rs b/crates/ast_node/src/lib.rs index b508eb5881fe..f2177dba0523 100644 --- a/crates/ast_node/src/lib.rs +++ b/crates/ast_node/src/lib.rs @@ -151,10 +151,11 @@ pub fn ast_serde( print("ast_serde", item) } -/// Alias for -/// `#[derive(Spanned, Fold, Clone, Debug, PartialEq)]` for a struct and -/// `#[derive(Spanned, Fold, Clone, Debug, PartialEq, FromVariant)]` for an -/// enum. +/// Adds the standard AST node derives and serialization metadata. +/// +/// Structs derive `PartialEq` by default. Pass `no_partial_eq` after the node +/// tag when the type provides a manual implementation: +/// `#[ast_node("CustomNode", no_partial_eq)]`. #[proc_macro_attribute] pub fn ast_node( args: proc_macro::TokenStream, @@ -289,9 +290,14 @@ pub fn ast_node( .as_ref() .map(|args| ast_node_macro::expand_struct(args.clone(), input.clone())); + let partial_eq = match &args { + Some(args) if args.no_partial_eq => None, + _ => Some(quote!(PartialEq,)), + }; + item.extend(quote!( #[allow(clippy::derive_partial_eq_without_eq)] - #[derive(::swc_common::Spanned, Clone, Debug, PartialEq)] + #[derive(::swc_common::Spanned, Clone, Debug, #partial_eq)] #[cfg_attr( feature = "serde-impl", derive(::serde::Serialize, ::serde::Deserialize) diff --git a/crates/swc_ecma_ast/src/lit.rs b/crates/swc_ecma_ast/src/lit.rs index 8a929e444e07..3071980577e1 100644 --- a/crates/swc_ecma_ast/src/lit.rs +++ b/crates/swc_ecma_ast/src/lit.rs @@ -522,13 +522,14 @@ impl<'a> arbitrary::Arbitrary<'a> for Regex { /// All of `Box`, `Expr`, `Lit`, `Number` implements `From<64>` and /// `From`. -#[ast_node("NumericLiteral")] +#[ast_node("NumericLiteral", no_partial_eq)] #[cfg_attr(feature = "shrink-to-fit", derive(shrink_to_fit::ShrinkToFit))] pub struct Number { pub span: Span, - /// **Note**: This should not be `NaN`. Use [crate::Ident] to represent NaN. + /// The numeric value, including `NaN` and positive or negative infinity. /// - /// If you store `NaN` in this field, a hash map will behave strangely. + /// Equality treats all `NaN` representations as equal and distinguishes + /// positive and negative zero. pub value: f64, /// Use `None` value only for transformations to avoid recalculate @@ -540,38 +541,52 @@ pub struct Number { pub raw: Option, } +impl PartialEq for Number { + fn eq(&self, other: &Self) -> bool { + self.span == other.span && number_value_eq(self.value, other.value) && self.raw == other.raw + } +} + impl Eq for Number {} impl EqIgnoreSpan for Number { fn eq_ignore_span(&self, other: &Self) -> bool { - self.value == other.value && self.value.is_sign_positive() == other.value.is_sign_positive() + number_value_eq(self.value, other.value) } } -#[allow(clippy::derived_hash_with_manual_eq)] -#[allow(unnecessary_transmutes)] impl Hash for Number { fn hash(&self, state: &mut H) { - fn integer_decode(val: f64) -> (u64, i16, i8) { - let bits: u64 = val.to_bits(); - let sign: i8 = if bits >> 63 == 0 { 1 } else { -1 }; - let mut exponent: i16 = ((bits >> 52) & 0x7ff) as i16; - let mantissa = if exponent == 0 { - (bits & 0xfffffffffffff) << 1 - } else { - (bits & 0xfffffffffffff) | 0x10000000000000 - }; - - exponent -= 1023 + 52; - (mantissa, exponent, sign) - } - self.span.hash(state); - integer_decode(self.value).hash(state); + canonical_number_bits(self.value).hash(state); self.raw.hash(state); } } +/// Compares numeric values using the equality semantics required by `Number`. +/// +/// NaN payloads are not observable in ECMAScript, while the sign of zero is. +#[inline] +fn number_value_eq(left: f64, right: f64) -> bool { + // Compare bits so positive and negative zero remain distinct. + left.to_bits() == right.to_bits() || (left.is_nan() && right.is_nan()) +} + +/// Returns bits consistent with [`number_value_eq`]. +/// +/// This follows the NaN canonicalization used by `ordered-float`, but retains +/// the sign bit of zero. +#[inline] +fn canonical_number_bits(value: f64) -> u64 { + const CANONICAL_NAN_BITS: u64 = 0x7ff8_0000_0000_0000; + + if value.is_nan() { + CANONICAL_NAN_BITS + } else { + value.to_bits() + } +} + impl Display for Number { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { if self.value.is_infinite() { diff --git a/crates/swc_ecma_ast/tests/lit.rs b/crates/swc_ecma_ast/tests/lit.rs index ac568f4a8f9a..ea1f4650d95f 100644 --- a/crates/swc_ecma_ast/tests/lit.rs +++ b/crates/swc_ecma_ast/tests/lit.rs @@ -1,6 +1,8 @@ +use std::collections::HashSet; + use swc_atoms::{wtf8::Wtf8, Atom}; -use swc_common::DUMMY_SP; -use swc_ecma_ast::{Str, TplElement}; +use swc_common::{EqIgnoreSpan, DUMMY_SP}; +use swc_ecma_ast::{Number, Str, TplElement}; /// Convert Wtf8 to a string representation, escaping invalid surrogates. fn convert_wtf8_to_raw(s: &Wtf8) -> String { @@ -112,6 +114,45 @@ fn combined_escapes() { test_from_tpl_raw("\\t\\tindented", "\t\tindented"); } +#[test] +fn number_nan_has_consistent_equality_and_hash() { + let first = Number::from(f64::NAN); + let second = Number::from(f64::from_bits(0xfff8_0000_0000_0001)); + + assert_eq!(first, second); + assert!(first.eq_ignore_span(&second)); + + let mut numbers = HashSet::new(); + numbers.insert(first); + + assert!(numbers.contains(&second)); +} + +#[test] +fn number_distinguishes_signed_zero() { + let positive = Number::from(0.0); + let negative = Number::from(-0.0); + + assert_ne!(positive, negative); + assert!(!positive.eq_ignore_span(&negative)); + + let numbers = HashSet::from([positive, negative]); + + assert_eq!(numbers.len(), 2); +} + +#[test] +fn number_infinity_has_consistent_equality_and_hash() { + let first = Number::from(f64::INFINITY); + let second = Number::from(f64::INFINITY); + + assert_eq!(first, second); + + let numbers = HashSet::from([first]); + + assert!(numbers.contains(&second)); +} + // Tests for octal escape sequences that should be rejected. // These will panic because octal escapes are not allowed in template strings. #[test]