Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-number-hash.md
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 19 additions & 1 deletion crates/ast_node/src/ast_node_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self> {
Ok(Args { ty: i.parse()? })
let ty = i.parse()?;
let mut no_partial_eq = false;

if i.parse::<Option<Token![,]>>()?.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 })
}
}

Expand Down
16 changes: 11 additions & 5 deletions crates/ast_node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
57 changes: 36 additions & 21 deletions crates/swc_ecma_ast/src/lit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,13 +522,14 @@ impl<'a> arbitrary::Arbitrary<'a> for Regex {
/// All of `Box<Expr>`, `Expr`, `Lit`, `Number` implements `From<64>` and
/// `From<usize>`.

#[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
Expand All @@ -540,38 +541,52 @@ pub struct Number {
pub raw: Option<Atom>,
}

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<H: Hasher>(&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() {
Expand Down
45 changes: 43 additions & 2 deletions crates/swc_ecma_ast/tests/lit.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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]
Expand Down
Loading