From 3407d470aae9cbeda6a956a270abcebae7a7e0dc Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 4 Feb 2025 02:48:18 +0000 Subject: [PATCH] refactor(ast): move `#[clone_in(default)]` to types (#8876) Remove `#[clone_in(default)]` attributes from struct fields containing semantic IDs, and add that attribute to the `ScopeId`, `SymbolId` and `ReferenceId` types instead. This reduces pointless repetition in the AST type definitions. --- crates/oxc_ast/src/ast/js.rs | 13 ---- crates/oxc_ast/src/ast/ts.rs | 8 --- crates/oxc_syntax/src/reference.rs | 1 + crates/oxc_syntax/src/scope.rs | 1 + crates/oxc_syntax/src/symbol.rs | 1 + tasks/ast_tools/src/derives/clone_in.rs | 68 ++++++++++++++----- tasks/ast_tools/src/schema/defs/enum.rs | 3 + tasks/ast_tools/src/schema/defs/struct.rs | 4 +- .../src/schema/extensions/clone_in.rs | 7 ++ 9 files changed, 68 insertions(+), 38 deletions(-) diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 337477d4f0771..5def04414cae0 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -41,7 +41,6 @@ pub struct Program<'a> { pub directives: Vec<'a, Directive<'a>>, pub body: Vec<'a, Statement<'a>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -227,7 +226,6 @@ pub struct IdentifierReference<'a> { /// set in the bind step of semantic analysis, and will always be [`None`] /// immediately after parsing. #[estree(skip)] - #[clone_in(default)] pub reference_id: Cell>, } @@ -251,7 +249,6 @@ pub struct BindingIdentifier<'a> { /// /// [`semantic analysis`]: #[estree(skip)] - #[clone_in(default)] pub symbol_id: Cell>, } @@ -1043,7 +1040,6 @@ pub struct BlockStatement<'a> { pub span: Span, pub body: Vec<'a, Statement<'a>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1187,7 +1183,6 @@ pub struct ForStatement<'a> { pub update: Option>, pub body: Statement<'a>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1218,7 +1213,6 @@ pub struct ForInStatement<'a> { pub right: Expression<'a>, pub body: Statement<'a>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1249,7 +1243,6 @@ pub struct ForOfStatement<'a> { pub right: Expression<'a>, pub body: Statement<'a>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1301,7 +1294,6 @@ pub struct SwitchStatement<'a> { #[scope(enter_before)] pub cases: Vec<'a, SwitchCase<'a>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1391,7 +1383,6 @@ pub struct CatchClause<'a> { /// The statements run when an error is caught pub body: Box<'a, BlockStatement<'a>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1625,7 +1616,6 @@ pub struct Function<'a> { /// ``` pub body: Option>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1717,7 +1707,6 @@ pub struct ArrowFunctionExpression<'a> { /// See `expression` for whether this arrow expression returns an expression. pub body: Box<'a, FunctionBody<'a>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1804,7 +1793,6 @@ pub struct Class<'a> { /// Id of the scope created by the [`Class`], including type parameters and /// statements within the [`ClassBody`]. #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -2056,7 +2044,6 @@ pub struct StaticBlock<'a> { pub span: Span, pub body: Vec<'a, Statement<'a>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } diff --git a/crates/oxc_ast/src/ast/ts.rs b/crates/oxc_ast/src/ast/ts.rs index 24ce0c6874cbc..a56d50de56436 100644 --- a/crates/oxc_ast/src/ast/ts.rs +++ b/crates/oxc_ast/src/ast/ts.rs @@ -72,7 +72,6 @@ pub struct TSEnumDeclaration<'a> { pub r#const: bool, pub declare: bool, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -304,7 +303,6 @@ pub struct TSConditionalType<'a> { #[scope(exit_before)] pub false_type: TSType<'a>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -837,7 +835,6 @@ pub struct TSTypeAliasDeclaration<'a> { pub type_annotation: TSType<'a>, pub declare: bool, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -901,7 +898,6 @@ pub struct TSInterfaceDeclaration<'a> { /// `true` for `declare interface Foo {}` pub declare: bool, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1020,7 +1016,6 @@ pub struct TSMethodSignature<'a> { pub params: Box<'a, FormalParameters<'a>>, pub return_type: Option>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1035,7 +1030,6 @@ pub struct TSConstructSignatureDeclaration<'a> { pub params: Box<'a, FormalParameters<'a>>, pub return_type: Option>>, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1160,7 +1154,6 @@ pub struct TSModuleDeclaration<'a> { pub kind: TSModuleDeclarationKind, pub declare: bool, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } @@ -1431,7 +1424,6 @@ pub struct TSMappedType<'a> { /// ``` pub readonly: TSMappedTypeModifierOperator, #[estree(skip)] - #[clone_in(default)] pub scope_id: Cell>, } diff --git a/crates/oxc_syntax/src/reference.rs b/crates/oxc_syntax/src/reference.rs index 4560c72339617..4e7df9960a89c 100644 --- a/crates/oxc_syntax/src/reference.rs +++ b/crates/oxc_syntax/src/reference.rs @@ -13,6 +13,7 @@ use oxc_ast_macros::ast; #[ast] #[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[clone_in(default)] #[content_eq(skip)] pub struct ReferenceId(NonMaxU32); diff --git a/crates/oxc_syntax/src/scope.rs b/crates/oxc_syntax/src/scope.rs index c545078a54b1e..1744a3ffb6865 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)] +#[clone_in(default)] #[content_eq(skip)] pub struct ScopeId(NonMaxU32); diff --git a/crates/oxc_syntax/src/symbol.rs b/crates/oxc_syntax/src/symbol.rs index 92391f6dbd185..3e8f6cddb3682 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)] +#[clone_in(default)] #[content_eq(skip)] pub struct SymbolId(NonMaxU32); diff --git a/tasks/ast_tools/src/derives/clone_in.rs b/tasks/ast_tools/src/derives/clone_in.rs index 181e16c1f1d11..5c6ee281aa412 100644 --- a/tasks/ast_tools/src/derives/clone_in.rs +++ b/tasks/ast_tools/src/derives/clone_in.rs @@ -5,11 +5,13 @@ use quote::{format_ident, quote}; use syn::Ident; use crate::{ - schema::{Def, EnumDef, Schema, StructDef}, + schema::{Def, EnumDef, FieldDef, Schema, StructDef, TypeDef}, Result, }; -use super::{define_derive, AttrLocation, AttrPart, AttrPositions, Derive, StructOrEnum}; +use super::{ + attr_positions, define_derive, AttrLocation, AttrPart, AttrPositions, Derive, StructOrEnum, +}; /// Derive for `CloneIn` trait. pub struct DeriveCloneIn; @@ -29,23 +31,29 @@ impl Derive for DeriveCloneIn { "oxc_allocator" } - /// Register that accept `#[clone_in]` attr on struct fields. + /// Register that accept `#[clone_in]` 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)] { - &[("clone_in", AttrPositions::StructField)] + &[("clone_in", attr_positions!(StructMaybeDerived | EnumMaybeDerived | StructField))] } /// Parse `#[clone_in(default)]` on struct field. fn parse_attr(&self, _attr_name: &str, location: AttrLocation, part: AttrPart) -> Result<()> { // No need to check attr name is `clone_in`, because that's the only attribute this derive handles. - // Ditto location can only be `StructField`. - let AttrLocation::StructField(struct_def, field_index) = location else { unreachable!() }; + if !matches!(part, AttrPart::Tag("default")) { + return Err(()); + } - if matches!(part, AttrPart::Tag("default")) { - struct_def.fields[field_index].clone_in.is_default = true; - Ok(()) - } else { - Err(()) + match location { + AttrLocation::Struct(struct_def) => struct_def.clone_in.is_default = true, + AttrLocation::Enum(enum_def) => enum_def.clone_in.is_default = true, + AttrLocation::StructField(struct_def, field_index) => { + struct_def.fields[field_index].clone_in.is_default = true; + } + _ => return Err(()), } + + Ok(()) } fn prelude(&self) -> TokenStream { @@ -59,20 +67,25 @@ impl Derive for DeriveCloneIn { fn derive(&self, type_def: StructOrEnum, schema: &Schema) -> TokenStream { match type_def { - StructOrEnum::Struct(struct_def) => derive_struct(struct_def), + StructOrEnum::Struct(struct_def) => derive_struct(struct_def, schema), StructOrEnum::Enum(enum_def) => derive_enum(enum_def, schema), } } } -fn derive_struct(struct_def: &StructDef) -> TokenStream { - let type_ident = struct_def.ident(); +fn derive_struct(struct_def: &StructDef, schema: &Schema) -> TokenStream { + assert!( + !struct_def.clone_in.is_default, + "Cannot derive `CloneIn` on a type which has a `#[clone_in(default)]` attribute: `{}`", + struct_def.name() + ); + let type_ident = struct_def.ident(); let has_fields = !struct_def.fields.is_empty(); let body = if has_fields { let fields = struct_def.fields.iter().map(|field| { let field_ident = field.ident(); - if field.clone_in.is_default { + if struct_field_is_default(field, schema) { quote!( #field_ident: Default::default() ) } else { quote!( #field_ident: CloneIn::clone_in(&self.#field_ident, allocator) ) @@ -86,9 +99,32 @@ fn derive_struct(struct_def: &StructDef) -> TokenStream { generate_impl(&type_ident, &body, struct_def.has_lifetime, has_fields) } +/// Get if a struct field should be filled with default value when cloning. +/// +/// This is that case if either: +/// 1. Struct field has `#[clone_in(default)]` attr. or +/// 2. The field's type has `#[clone_in(default)]` attr. +fn struct_field_is_default(field: &FieldDef, schema: &Schema) -> bool { + if field.clone_in.is_default { + true + } else { + let innermost_type = field.type_def(schema).innermost_type(schema); + match innermost_type { + TypeDef::Struct(struct_def) => struct_def.clone_in.is_default, + TypeDef::Enum(enum_def) => enum_def.clone_in.is_default, + _ => false, + } + } +} + fn derive_enum(enum_def: &EnumDef, schema: &Schema) -> TokenStream { - let type_ident = enum_def.ident(); + assert!( + !enum_def.clone_in.is_default, + "Cannot derive `CloneIn` on a type which has a `#[clone_in(default)]` attribute: `{}`", + enum_def.name() + ); + let type_ident = enum_def.ident(); let mut uses_allocator = false; let match_arms = enum_def.all_variants(schema).map(|variant| { let ident = variant.ident(); diff --git a/tasks/ast_tools/src/schema/defs/enum.rs b/tasks/ast_tools/src/schema/defs/enum.rs index 5636d825e5239..ae7ccc7ead844 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::{ + clone_in::CloneInType, content_eq::ContentEqType, estree::{ESTreeEnum, ESTreeEnumVariant}, kind::Kind, @@ -34,6 +35,7 @@ pub struct EnumDef { pub visit: VisitEnum, pub kind: Kind, pub layout: Layout, + pub clone_in: CloneInType, pub content_eq: ContentEqType, pub estree: ESTreeEnum, } @@ -60,6 +62,7 @@ impl EnumDef { visit: VisitEnum::default(), kind: Kind::default(), layout: Layout::default(), + clone_in: CloneInType::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 9a73d5075189b..ed7b7c4df2a2e 100644 --- a/tasks/ast_tools/src/schema/defs/struct.rs +++ b/tasks/ast_tools/src/schema/defs/struct.rs @@ -8,7 +8,7 @@ use crate::utils::create_ident_tokens; use super::{ extensions::{ - clone_in::CloneInStructField, + clone_in::{CloneInStructField, CloneInType}, content_eq::{ContentEqStructField, ContentEqType}, estree::{ESTreeStruct, ESTreeStructField}, kind::Kind, @@ -32,6 +32,7 @@ pub struct StructDef { pub kind: Kind, pub layout: Layout, pub span: SpanStruct, + pub clone_in: CloneInType, pub content_eq: ContentEqType, pub estree: ESTreeStruct, } @@ -57,6 +58,7 @@ impl StructDef { kind: Kind::default(), layout: Layout::default(), span: SpanStruct::default(), + clone_in: CloneInType::default(), content_eq: ContentEqType::default(), estree: ESTreeStruct::default(), } diff --git a/tasks/ast_tools/src/schema/extensions/clone_in.rs b/tasks/ast_tools/src/schema/extensions/clone_in.rs index 2d2d2f9f3278c..a79baaa36c92d 100644 --- a/tasks/ast_tools/src/schema/extensions/clone_in.rs +++ b/tasks/ast_tools/src/schema/extensions/clone_in.rs @@ -1,3 +1,10 @@ +/// Details for `CloneIn` derive on a struct or enum. +#[derive(Default, Debug)] +pub struct CloneInType { + /// `true` if should be replaced with default value when cloning + pub is_default: bool, +} + /// Details for `CloneIn` derive on a struct field. #[derive(Default, Debug)] pub struct CloneInStructField {