From caa651c63c5ce69eec9110fe498f54f764b8859a Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 4 Feb 2025 02:48:17 +0000 Subject: [PATCH] refactor(ast): `#[content_eq(skip)]` attr (#8875) Introduce custom attribute `#[content_eq(skip)]` to guide code generation for `ContentEq` trait. Instead of a hard-coded list of types to skip in codegen, use this attribute on types to control the behavior. --- crates/oxc_ast_macros/src/lib.rs | 5 +- crates/oxc_span/src/span/types.rs | 1 + crates/oxc_syntax/src/reference.rs | 4 +- crates/oxc_syntax/src/scope.rs | 1 + crates/oxc_syntax/src/symbol.rs | 1 + tasks/ast_tools/src/derives/content_eq.rs | 90 ++++++++++++++----- tasks/ast_tools/src/schema/defs/enum.rs | 3 + tasks/ast_tools/src/schema/defs/struct.rs | 5 ++ .../src/schema/extensions/content_eq.rs | 13 +++ tasks/ast_tools/src/schema/mod.rs | 1 + 10 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 tasks/ast_tools/src/schema/extensions/content_eq.rs diff --git a/crates/oxc_ast_macros/src/lib.rs b/crates/oxc_ast_macros/src/lib.rs index 44d0b1ce9e4d2..7a27231982b2b 100644 --- a/crates/oxc_ast_macros/src/lib.rs +++ b/crates/oxc_ast_macros/src/lib.rs @@ -47,7 +47,10 @@ pub fn ast(_args: TokenStream, input: TokenStream) -> TokenStream { /// Its only purpose is to allow the occurrence of helper attributes used in `tasks/ast_tools`. /// /// See [`macro@ast`] for further details. -#[proc_macro_derive(Ast, attributes(clone_in, estree, generate_derive, scope, span, ts, visit))] +#[proc_macro_derive( + Ast, + attributes(clone_in, content_eq, estree, generate_derive, scope, span, ts, visit) +)] pub fn ast_derive(_input: TokenStream) -> TokenStream { TokenStream::new() } diff --git a/crates/oxc_span/src/span/types.rs b/crates/oxc_span/src/span/types.rs index 1a62df8a6bc75..e3a1d6918f960 100644 --- a/crates/oxc_span/src/span/types.rs +++ b/crates/oxc_span/src/span/types.rs @@ -61,6 +61,7 @@ use super::PointerAlign; #[ast(visit)] #[derive(Default, Clone, Copy, Eq, PartialOrd, Ord)] #[generate_derive(ESTree)] +#[content_eq(skip)] #[estree(no_type, flatten)] pub struct Span { /// The zero-based start offset of the span diff --git a/crates/oxc_syntax/src/reference.rs b/crates/oxc_syntax/src/reference.rs index 5903dc4b718ce..4560c72339617 100644 --- a/crates/oxc_syntax/src/reference.rs +++ b/crates/oxc_syntax/src/reference.rs @@ -1,17 +1,19 @@ #![allow(missing_docs)] // fixme use bitflags::bitflags; use nonmax::NonMaxU32; -use oxc_allocator::CloneIn; use oxc_index::Idx; #[cfg(feature = "serialize")] use serde::{Serialize, Serializer}; +use oxc_allocator::CloneIn; + use crate::{node::NodeId, symbol::SymbolId}; use oxc_ast_macros::ast; #[ast] #[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[content_eq(skip)] pub struct ReferenceId(NonMaxU32); impl Idx for ReferenceId { diff --git a/crates/oxc_syntax/src/scope.rs b/crates/oxc_syntax/src/scope.rs index fedb1e2179b46..c545078a54b1e 100644 --- a/crates/oxc_syntax/src/scope.rs +++ b/crates/oxc_syntax/src/scope.rs @@ -9,6 +9,7 @@ use oxc_ast_macros::ast; #[ast] #[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[content_eq(skip)] pub struct ScopeId(NonMaxU32); impl ScopeId { diff --git a/crates/oxc_syntax/src/symbol.rs b/crates/oxc_syntax/src/symbol.rs index c6ec65330559f..92391f6dbd185 100644 --- a/crates/oxc_syntax/src/symbol.rs +++ b/crates/oxc_syntax/src/symbol.rs @@ -9,6 +9,7 @@ use oxc_ast_macros::ast; #[ast] #[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[content_eq(skip)] pub struct SymbolId(NonMaxU32); impl SymbolId { diff --git a/tasks/ast_tools/src/derives/content_eq.rs b/tasks/ast_tools/src/derives/content_eq.rs index e6da14437c850..03b4a773a85f4 100644 --- a/tasks/ast_tools/src/derives/content_eq.rs +++ b/tasks/ast_tools/src/derives/content_eq.rs @@ -3,11 +3,14 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use crate::schema::{Def, EnumDef, Schema, StructDef}; +use crate::{ + schema::{Def, EnumDef, Schema, StructDef, TypeDef}, + Result, +}; -use super::{define_derive, Derive, StructOrEnum}; - -const IGNORE_FIELD_TYPES: [&str; 4] = ["Span", "ScopeId", "SymbolId", "ReferenceId"]; +use super::{ + attr_positions, define_derive, AttrLocation, AttrPart, AttrPositions, Derive, StructOrEnum, +}; /// Derive for `ContentEq` trait. pub struct DeriveContentEq; @@ -23,6 +26,31 @@ impl Derive for DeriveContentEq { "oxc_span" } + /// Register that accept `#[content_eq]` attr on structs, enums, or struct fields. + /// Allow attr on structs and enums which don't derive this trait. + fn attrs(&self) -> &[(&'static str, AttrPositions)] { + &[("content_eq", attr_positions!(StructMaybeDerived | EnumMaybeDerived | StructField))] + } + + /// Parse `#[content_eq(skip)]` attr. + fn parse_attr(&self, _attr_name: &str, location: AttrLocation, part: AttrPart) -> Result<()> { + // No need to check attr name is `content_eq`, because that's the only attribute this derive handles. + if !matches!(part, AttrPart::Tag("skip")) { + return Err(()); + } + + match location { + AttrLocation::Struct(struct_def) => struct_def.content_eq.skip = true, + AttrLocation::Enum(enum_def) => enum_def.content_eq.skip = true, + AttrLocation::StructField(struct_def, field_index) => { + struct_def.fields[field_index].content_eq.skip = true; + } + _ => return Err(()), + } + + Ok(()) + } + fn prelude(&self) -> TokenStream { quote! { #![allow(clippy::match_same_arms)] @@ -41,30 +69,49 @@ impl Derive for DeriveContentEq { } fn derive_struct(struct_def: &StructDef, schema: &Schema) -> TokenStream { - let fields = struct_def - .fields - .iter() - .filter(|field| { - let innermost_type = field.type_def(schema).innermost_type(schema); - !IGNORE_FIELD_TYPES.contains(&innermost_type.name()) - }) - .map(|field| { - let ident = field.ident(); - quote!( ContentEq::content_eq(&self.#ident, &other.#ident) ) - }); - - let mut body = quote!( #(#fields)&&* ); let mut other_name = "other"; - if body.is_empty() { - body = quote!(true); + + let body = if struct_def.content_eq.skip { + // Struct has `#[content_eq(skip)]` attr. So `content_eq` always returns true. other_name = "_"; + quote!(true) + } else { + let fields = struct_def + .fields + .iter() + .filter(|field| !field.content_eq.skip) + .filter(|field| { + let innermost_type = field.type_def(schema).innermost_type(schema); + match innermost_type { + TypeDef::Struct(struct_def) => !struct_def.content_eq.skip, + TypeDef::Enum(enum_def) => !enum_def.content_eq.skip, + _ => true, + } + }) + .map(|field| { + let ident = field.ident(); + quote!( ContentEq::content_eq(&self.#ident, &other.#ident) ) + }); + + let mut body = quote!( #(#fields)&&* ); + if body.is_empty() { + body = quote!(true); + other_name = "_"; + }; + body }; generate_impl(&struct_def.ty_anon(schema), other_name, &body) } fn derive_enum(enum_def: &EnumDef, schema: &Schema) -> TokenStream { - let body = if enum_def.is_fieldless() { + let mut other_name = "other"; + + let body = if enum_def.content_eq.skip { + // Enum has `#[content_eq(skip)]` attr. So `content_eq` always returns true. + other_name = "_"; + quote!(true) + } else if enum_def.is_fieldless() { // We assume fieldless enums implement `PartialEq` quote!(self == other) } else { @@ -76,6 +123,7 @@ fn derive_enum(enum_def: &EnumDef, schema: &Schema) -> TokenStream { quote!( (Self::#ident(a), Self::#ident(b)) => a.content_eq(b) ) } }); + quote! { match (self, other) { #(#matches,)* @@ -84,7 +132,7 @@ fn derive_enum(enum_def: &EnumDef, schema: &Schema) -> TokenStream { } }; - generate_impl(&enum_def.ty_anon(schema), "other", &body) + generate_impl(&enum_def.ty_anon(schema), other_name, &body) } fn generate_impl(ty: &TokenStream, other_name: &str, body: &TokenStream) -> TokenStream { diff --git a/tasks/ast_tools/src/schema/defs/enum.rs b/tasks/ast_tools/src/schema/defs/enum.rs index 9f8b41cc9e668..5636d825e5239 100644 --- a/tasks/ast_tools/src/schema/defs/enum.rs +++ b/tasks/ast_tools/src/schema/defs/enum.rs @@ -9,6 +9,7 @@ use crate::utils::create_ident; use super::{ extensions::{ + content_eq::ContentEqType, estree::{ESTreeEnum, ESTreeEnumVariant}, kind::Kind, layout::Layout, @@ -33,6 +34,7 @@ pub struct EnumDef { pub visit: VisitEnum, pub kind: Kind, pub layout: Layout, + pub content_eq: ContentEqType, pub estree: ESTreeEnum, } @@ -58,6 +60,7 @@ impl EnumDef { visit: VisitEnum::default(), kind: Kind::default(), layout: Layout::default(), + content_eq: ContentEqType::default(), estree: ESTreeEnum::default(), } } diff --git a/tasks/ast_tools/src/schema/defs/struct.rs b/tasks/ast_tools/src/schema/defs/struct.rs index 19d13ec495880..9a73d5075189b 100644 --- a/tasks/ast_tools/src/schema/defs/struct.rs +++ b/tasks/ast_tools/src/schema/defs/struct.rs @@ -9,6 +9,7 @@ use crate::utils::create_ident_tokens; use super::{ extensions::{ clone_in::CloneInStructField, + content_eq::{ContentEqStructField, ContentEqType}, estree::{ESTreeStruct, ESTreeStructField}, kind::Kind, layout::{Layout, Offset}, @@ -31,6 +32,7 @@ pub struct StructDef { pub kind: Kind, pub layout: Layout, pub span: SpanStruct, + pub content_eq: ContentEqType, pub estree: ESTreeStruct, } @@ -55,6 +57,7 @@ impl StructDef { kind: Kind::default(), layout: Layout::default(), span: SpanStruct::default(), + content_eq: ContentEqType::default(), estree: ESTreeStruct::default(), } } @@ -118,6 +121,7 @@ pub struct FieldDef { pub visit: VisitFieldOrVariant, pub offset: Offset, pub clone_in: CloneInStructField, + pub content_eq: ContentEqStructField, pub estree: ESTreeStructField, } @@ -137,6 +141,7 @@ impl FieldDef { visit: VisitFieldOrVariant::default(), offset: Offset::default(), clone_in: CloneInStructField::default(), + content_eq: ContentEqStructField::default(), estree: ESTreeStructField::default(), } } diff --git a/tasks/ast_tools/src/schema/extensions/content_eq.rs b/tasks/ast_tools/src/schema/extensions/content_eq.rs new file mode 100644 index 0000000000000..2e49904df2689 --- /dev/null +++ b/tasks/ast_tools/src/schema/extensions/content_eq.rs @@ -0,0 +1,13 @@ +/// Details for `ContentEq` derive on a struct or enum. +#[derive(Default, Debug)] +pub struct ContentEqType { + /// `true` if type should ignored by `ContentEq` + pub skip: bool, +} + +/// Details for `ContentEq` derive on a struct field. +#[derive(Default, Debug)] +pub struct ContentEqStructField { + /// `true` if field should ignored by `ContentEq` + pub skip: bool, +} diff --git a/tasks/ast_tools/src/schema/mod.rs b/tasks/ast_tools/src/schema/mod.rs index 68165be23735c..8bf41c163162b 100644 --- a/tasks/ast_tools/src/schema/mod.rs +++ b/tasks/ast_tools/src/schema/mod.rs @@ -14,6 +14,7 @@ pub use file::File; /// Extensions to schema for specific derives / generators pub mod extensions { pub mod clone_in; + pub mod content_eq; pub mod estree; pub mod kind; pub mod layout;