Skip to content

Commit

Permalink
fix(ast): fix ContentEq and ContentHash impls for literal types
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Jan 10, 2025
1 parent b29655f commit 21aa03e
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 95 deletions.
12 changes: 6 additions & 6 deletions crates/oxc_ast/src/ast/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub struct BooleanLiteral {
/// <https://tc39.es/ecma262/#sec-null-literals>
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral, add_ts = "value: null, raw: \"null\" | null")]
pub struct NullLiteral {
/// Node location in source code
Expand All @@ -45,7 +45,7 @@ pub struct NullLiteral {
/// <https://tc39.es/ecma262/#sec-literals-numeric-literals>
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral)]
pub struct NumericLiteral<'a> {
/// Node location in source code
Expand All @@ -66,7 +66,7 @@ pub struct NumericLiteral<'a> {
/// <https://tc39.es/ecma262/#sec-literals-string-literals>
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral)]
pub struct StringLiteral<'a> {
/// Node location in source code
Expand All @@ -85,7 +85,7 @@ pub struct StringLiteral<'a> {
/// BigInt literal
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral, add_ts = "value: null, bigint: string")]
pub struct BigIntLiteral<'a> {
/// Node location in source code
Expand All @@ -103,7 +103,7 @@ pub struct BigIntLiteral<'a> {
/// <https://tc39.es/ecma262/#sec-literals-regular-expression-literals>
#[ast(visit)]
#[derive(Debug)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(
type = "Literal",
via = crate::serialize::ESTreeLiteral,
Expand Down Expand Up @@ -141,7 +141,7 @@ pub struct RegExp<'a> {
/// This pattern may or may not be parsed.
#[ast]
#[derive(Debug)]
#[generate_derive(CloneIn, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, ESTree)]
pub enum RegExpPattern<'a> {
/// Unparsed pattern. Contains string slice of the pattern.
/// Pattern was not parsed, so may be valid or invalid.
Expand Down
108 changes: 99 additions & 9 deletions crates/oxc_ast/src/ast_impl/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ impl fmt::Display for BooleanLiteral {
}
}

impl ContentHash for NullLiteral {
#[inline]
fn content_hash<H: Hasher>(&self, state: &mut H) {
Hash::hash(&Option::<bool>::None, state);
}
}

impl fmt::Display for NullLiteral {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down Expand Up @@ -82,10 +75,45 @@ impl NumericLiteral<'_> {
}
}

impl ContentEq for NumericLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
// Note: `f64::content_eq` uses `==` equality.
// `f64::NAN != f64::NAN` and `0.0 == -0.0`, so we follow the same here.
// If we change that behavior, we should alter `content_hash` too.
ContentEq::content_eq(&self.value, &other.value)
}
}

impl ContentHash for NumericLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.base, state);
ContentHash::content_hash(&self.raw, state);
// `f64` does not implement `Hash` due to ambiguity over what is the right way to hash NaN
// and +/- zero values.
//
// # NaN
// IEEE 754 defines a whole range of NaN values.
// https://doc.rust-lang.org/std/primitive.f64.html#associatedconstant.NAN
// We can ignore that complication here as the rule is that 2 types which are equal (`==`)
// must hash the same.
// `ContentEq` uses `==` equality, and even the same NaN doesn't equal itself!
// `f64::NAN != f64::NAN`
// So it doesn't matter if two NaNs have the same hash or not.
//
// # Zero
// `ContentEq` uses `==` equality, which considers `0.0` and `-0.0` equal, so we must make
// them hash the same.
// But `(0.0).to_bits() != (-0.0).to_bits()`, so we need to convert `-0.0` to `0.0`.
//
// # Infinity
// `f64::INFINITY != -f64::INFINITY` and `f64::INFINITY.to_bits() != (-f64::INFINITY).to_bits()`
// so infinity needs no special handling.
//
// Whatever the value, we convert to `u64` using `to_bits`, and hash that.
let mut value = self.value;
if value == -0.0 {
value = 0.0;
}
let value = value.to_bits();
std::hash::Hash::hash(&value, state);
}
}

Expand Down Expand Up @@ -121,6 +149,18 @@ impl StringLiteral<'_> {
}
}

impl ContentEq for StringLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.value, &other.value)
}
}

impl ContentHash for StringLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.value, state);
}
}

impl AsRef<str> for StringLiteral<'_> {
#[inline]
fn as_ref(&self) -> &str {
Expand All @@ -142,12 +182,36 @@ impl BigIntLiteral<'_> {
}
}

impl ContentEq for BigIntLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.raw, &other.raw)
}
}

impl ContentHash for BigIntLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.raw, state);
}
}

impl fmt::Display for BigIntLiteral<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.raw.fmt(f)
}
}

impl ContentEq for RegExpLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.regex, &other.regex)
}
}

impl ContentHash for RegExpLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.regex, state);
}
}

impl fmt::Display for RegExp<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "/{}/{}", self.pattern, self.flags)
Expand Down Expand Up @@ -203,6 +267,32 @@ impl<'a> RegExpPattern<'a> {
}
}

impl ContentEq for RegExpPattern<'_> {
fn content_eq(&self, other: &Self) -> bool {
let self_str = match self {
Self::Raw(s) | Self::Invalid(s) => *s,
Self::Pattern(p) => &p.to_string(),
};

let other_str = match other {
Self::Raw(s) | Self::Invalid(s) => *s,
Self::Pattern(p) => &p.to_string(),
};

self_str == other_str
}
}

impl ContentHash for RegExpPattern<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
let self_str = match self {
Self::Raw(s) | Self::Invalid(s) => s,
Self::Pattern(p) => &&*p.to_string(),
};
ContentHash::content_hash(self_str, state);
}
}

impl fmt::Display for RegExpPattern<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Expand Down
46 changes: 0 additions & 46 deletions crates/oxc_ast/src/generated/derive_content_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,59 +27,13 @@ impl ContentEq for NullLiteral {
}
}

impl ContentEq for NumericLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.value, &other.value)
&& ContentEq::content_eq(&self.base, &other.base)
}
}

impl ContentEq for StringLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.value, &other.value)
}
}

impl ContentEq for BigIntLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.raw, &other.raw)
&& ContentEq::content_eq(&self.base, &other.base)
}
}

impl ContentEq for RegExpLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.regex, &other.regex)
&& ContentEq::content_eq(&self.raw, &other.raw)
}
}

impl ContentEq for RegExp<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.pattern, &other.pattern)
&& ContentEq::content_eq(&self.flags, &other.flags)
}
}

impl ContentEq for RegExpPattern<'_> {
fn content_eq(&self, other: &Self) -> bool {
match self {
Self::Raw(it) => match other {
Self::Raw(other) if ContentEq::content_eq(it, other) => true,
_ => false,
},
Self::Invalid(it) => match other {
Self::Invalid(other) if ContentEq::content_eq(it, other) => true,
_ => false,
},
Self::Pattern(it) => match other {
Self::Pattern(other) if ContentEq::content_eq(it, other) => true,
_ => false,
},
}
}
}

impl ContentEq for Program<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.source_type, &other.source_type)
Expand Down
32 changes: 2 additions & 30 deletions crates/oxc_ast/src/generated/derive_content_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,8 @@ impl ContentHash for BooleanLiteral {
}
}

impl ContentHash for StringLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.value, state);
ContentHash::content_hash(&self.raw, state);
}
}

impl ContentHash for BigIntLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.raw, state);
ContentHash::content_hash(&self.base, state);
}
}

impl ContentHash for RegExpLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.regex, state);
ContentHash::content_hash(&self.raw, state);
}
impl ContentHash for NullLiteral {
fn content_hash<H: Hasher>(&self, _: &mut H) {}
}

impl ContentHash for RegExp<'_> {
Expand All @@ -51,17 +34,6 @@ impl ContentHash for RegExp<'_> {
}
}

impl ContentHash for RegExpPattern<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&discriminant(self), state);
match self {
Self::Raw(it) => ContentHash::content_hash(it, state),
Self::Invalid(it) => ContentHash::content_hash(it, state),
Self::Pattern(it) => ContentHash::content_hash(it, state),
}
}
}

impl ContentHash for Program<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.source_type, state);
Expand Down
4 changes: 0 additions & 4 deletions tasks/ast_tools/src/derives/content_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ fn derive_struct(def: &StructDef) -> (&str, TokenStream) {
.iter()
.filter(|field| {
let Some(name) = field.name.as_ref() else { return false };
if name == "raw" && matches!(def.name.as_str(), "NumericLiteral" | "StringLiteral")
{
return false;
}
!IGNORE_FIELDS
.iter()
.any(|it| name == it.0 && field.typ.name().inner_name() == it.1)
Expand Down

0 comments on commit 21aa03e

Please sign in to comment.