From 22dec6ace557c67786201aa20e0b823b2320c567 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Thu, 22 Jan 2026 07:46:26 +0000 Subject: [PATCH] feat(semantic)!: remove `Scoping::scope_build_child_ids` and all related APIs (#18362) Remove `Scoping::scope_build_child_ids` and all related APIs to make the semantics cleaner. This could also make it easier for us to sync Scoping data since we don't need to worry about child scope IDs. This is a breaking change, but it is okay because all use of `scope_build_child_ids` in the Oxc and Rolldown has been removed. --- crates/oxc_semantic/src/builder.rs | 6 - crates/oxc_semantic/src/scoping.rs | 112 +----------------- .../oxc_semantic/tests/integration/scopes.rs | 69 ----------- .../tests/integration/util/mod.rs | 1 - .../async_generator_functions/for_await.rs | 12 +- .../static_block_and_prop_init.rs | 5 - crates/oxc_traverse/src/context/scoping.rs | 10 -- tasks/transform_checker/src/lib.rs | 38 +----- 8 files changed, 7 insertions(+), 246 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 3ba84ef05b707..3de7ae8e950e1 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -191,12 +191,6 @@ impl<'a> SemanticBuilder<'a> { self } - #[must_use] - pub fn with_scope_tree_child_ids(mut self, yes: bool) -> Self { - self.scoping.scope_build_child_ids = yes; - self - } - /// Provide statistics about AST to optimize memory usage of semantic analysis. /// /// Accurate statistics can greatly improve performance, especially for large ASTs. diff --git a/crates/oxc_semantic/src/scoping.rs b/crates/oxc_semantic/src/scoping.rs index f4ef152752a10..99837165076e9 100644 --- a/crates/oxc_semantic/src/scoping.rs +++ b/crates/oxc_semantic/src/scoping.rs @@ -72,9 +72,6 @@ pub struct Scoping { /// Maps a scope to the parent scope it belongs in. scope_parent_ids: IndexVec>, - /// Runtime flag for constructing child_ids. - pub(crate) scope_build_child_ids: bool, - /// Maps a scope to its node id. scope_node_ids: IndexVec, @@ -99,7 +96,6 @@ impl Default for Scoping { references: IndexVec::new(), no_side_effects: FxHashSet::default(), scope_parent_ids: IndexVec::new(), - scope_build_child_ids: false, scope_node_ids: IndexVec::new(), scope_flags: IndexVec::new(), cell: ScopingCell::new(Allocator::default(), |allocator| ScopingInner { @@ -107,7 +103,6 @@ impl Default for Scoping { resolved_references: ArenaVec::new_in(allocator), symbol_redeclarations: FxHashMap::default(), bindings: IndexVec::new(), - scope_child_ids: ArenaVec::new_in(allocator), root_unresolved_references: UnresolvedReferences::new_in(allocator), }), } @@ -269,9 +264,6 @@ pub struct ScopingInner<'cell> { /// A binding is a mapping from an identifier name to its [`SymbolId`] pub(crate) bindings: IndexVec>, - /// Maps a scope to direct children scopes. - scope_child_ids: ArenaVec<'cell, ArenaVec<'cell, ScopeId>>, - pub(crate) root_unresolved_references: UnresolvedReferences<'cell>, } @@ -555,11 +547,6 @@ impl Scoping { cell.bindings.reserve(additional_scopes); }); self.scope_node_ids.reserve(additional_scopes); - if self.scope_build_child_ids { - self.cell.with_dependent_mut(|_allocator, cell| { - cell.scope_child_ids.reserve_exact(additional_scopes); - }); - } } pub fn no_side_effects(&self) -> &FxHashSet { @@ -688,48 +675,13 @@ impl Scoping { pub fn set_scope_parent_id(&mut self, scope_id: ScopeId, parent_id: Option) { self.scope_parent_ids[scope_id] = parent_id; - if self.scope_build_child_ids { - // Set this scope as child of parent scope - if let Some(parent_id) = parent_id { - self.cell.with_dependent_mut(|_allocator, cell| { - cell.scope_child_ids[parent_id.index()].push(scope_id); - }); - } - } } /// Change the parent scope of a scope. /// /// This will also remove the scope from the child list of the old parent and add it to the new parent. pub fn change_scope_parent_id(&mut self, scope_id: ScopeId, new_parent_id: Option) { - let old_parent_id = mem::replace(&mut self.scope_parent_ids[scope_id], new_parent_id); - if self.scope_build_child_ids { - self.cell.with_dependent_mut(|_allocator, cell| { - // Remove this scope from old parent scope - if let Some(old_parent_id) = old_parent_id { - cell.scope_child_ids[old_parent_id.index()] - .retain(|&child_id| child_id != scope_id); - } - // And add it to new parent scope - if let Some(parent_id) = new_parent_id { - cell.scope_child_ids[parent_id.index()].push(scope_id); - } - }); - } - } - - /// Delete a scope. - pub fn delete_scope(&mut self, scope_id: ScopeId) { - if self.scope_build_child_ids { - self.cell.with_dependent_mut(|_allocator, cell| { - cell.scope_child_ids[scope_id.index()].clear(); - let parent_id = self.scope_parent_ids[scope_id]; - if let Some(parent_id) = parent_id { - cell.scope_child_ids[parent_id.index()] - .retain(|&child_id| child_id != scope_id); - } - }); - } + self.scope_parent_ids[scope_id] = new_parent_id; } /// Get a variable binding by name that was declared in the top-level scope @@ -815,57 +767,6 @@ impl Scoping { }); } - /// Return whether this `ScopeTree` has child IDs recorded - #[inline] - pub fn has_scope_child_ids(&self) -> bool { - self.scope_build_child_ids - } - - /// Get the child scopes of a scope - #[inline] - pub fn get_scope_child_ids(&self, scope_id: ScopeId) -> &[ScopeId] { - &self.cell.borrow_dependent().scope_child_ids[scope_id.index()] - } - - pub fn iter_all_scope_child_ids( - &self, - scope_id: ScopeId, - ) -> impl Iterator + '_ { - let mut stack = self.cell.borrow_dependent().scope_child_ids[scope_id.index()] - .iter() - .copied() - .collect::>(); - let child_ids = &self.cell.borrow_dependent().scope_child_ids; - std::iter::from_fn(move || { - if let Some(scope_id) = stack.pop() { - if let Some(children) = child_ids.get(scope_id.index()) { - stack.extend(children.iter().copied()); - } - Some(scope_id) - } else { - None - } - }) - } - - pub fn remove_child_scopes(&mut self, scope_id: ScopeId, child_scope_ids: &[ScopeId]) { - self.cell.with_dependent_mut(|_allocator, cell| { - cell.scope_child_ids[scope_id.index()] - .retain(|scope_id| !child_scope_ids.contains(scope_id)); - }); - } - - /// Remove a child scope from a parent scope. - /// # Panics - /// Panics if the child scope is not a child of the parent scope. - pub fn remove_child_scope(&mut self, scope_id: ScopeId, child_scope_id: ScopeId) { - self.cell.with_dependent_mut(|_allocator, cell| { - let child_ids = &mut cell.scope_child_ids[scope_id.index()]; - let index = child_ids.iter().position(|&scope_id| scope_id == child_scope_id).unwrap(); - child_ids.swap_remove(index); - }); - } - /// Create a scope. #[inline] pub fn add_scope( @@ -880,14 +781,7 @@ impl Scoping { cell.bindings.push(Bindings::new_in(allocator)); }); self.scope_node_ids.push(node_id); - if self.scope_build_child_ids { - self.cell.with_dependent_mut(|allocator, cell| { - cell.scope_child_ids.push(ArenaVec::new_in(allocator)); - if let Some(parent_id) = parent_id { - cell.scope_child_ids[parent_id.index()].push(scope_id); - } - }); - } + scope_id } @@ -995,7 +889,6 @@ impl Scoping { references: self.references.clone(), no_side_effects: self.no_side_effects.clone(), scope_parent_ids: self.scope_parent_ids.clone(), - scope_build_child_ids: self.scope_build_child_ids, scope_node_ids: self.scope_node_ids.clone(), scope_flags: self.scope_flags.clone(), cell: { @@ -1015,7 +908,6 @@ impl Scoping { .iter() .map(|map| map.clone_in_with_semantic_ids(allocator)) .collect(), - scope_child_ids: cell.scope_child_ids.clone_in_with_semantic_ids(allocator), root_unresolved_references: cell .root_unresolved_references .clone_in_with_semantic_ids(allocator), diff --git a/crates/oxc_semantic/tests/integration/scopes.rs b/crates/oxc_semantic/tests/integration/scopes.rs index 0da543de6e35b..a22fd4aef1468 100644 --- a/crates/oxc_semantic/tests/integration/scopes.rs +++ b/crates/oxc_semantic/tests/integration/scopes.rs @@ -223,24 +223,6 @@ fn var_hoisting() { .test(); } -#[test] -fn get_child_ids() { - let test = SemanticTester::js( - " - function foo() { - } - ", - ) - .with_scope_tree_child_ids(true); - let semantic = test.build(); - let scoping = semantic.into_scoping(); - - let child_scope_ids = scoping.get_scope_child_ids(scoping.root_scope_id()); - assert_eq!(child_scope_ids.len(), 1); - let child_scope_ids = scoping.get_scope_child_ids(child_scope_ids[0]); - assert!(child_scope_ids.is_empty()); -} - #[test] fn test_ts_conditional_types() { SemanticTester::ts("type A = T extends string ? T : false;") @@ -284,54 +266,3 @@ fn test_eval() { assert!(!semantic.scoping().root_scope_flags().contains_direct_eval()); } } - -#[test] -fn test_with_statement() { - // Test that with statement creates a scope - let tester = SemanticTester::js( - " - const foo = { x: 1 }; - with (foo) x; - ", - ) - .with_module(false) - .with_scope_tree_child_ids(true); - - let semantic = tester.build(); - let scopes = semantic.scoping(); - - // Should have root scope + with statement scope - assert_eq!(scopes.scopes_len(), 2, "with statement should create a scope"); - - // Verify scope tree structure - let root_id = semantic.scoping().root_scope_id(); - let child_ids = semantic.scoping().get_scope_child_ids(root_id); - assert_eq!(child_ids.len(), 1, "with statement scope should be a child of root scope"); - - // Verify the child scope is for the with statement - let with_scope_id = child_ids[0]; - let with_scope_node_id = scopes.get_node_id(with_scope_id); - let with_node = semantic.nodes().get_node(with_scope_node_id); - assert!( - matches!(with_node.kind(), AstKind::WithStatement(_)), - "Child scope should be associated with WithStatement" - ); - - // Test with block statement as body - let tester2 = SemanticTester::js( - " - const foo = { x: 1 }; - with (foo) { - const y = 2; - } - ", - ) - .with_module(false) - .with_scope_tree_child_ids(true); - - let semantic2 = tester2.build(); - let scopes2 = semantic2.scoping(); - - // Should have root scope + with statement scope + block statement scope - assert_eq!(scopes2.scopes_len(), 3, "with statement and its block should create scopes"); -} diff --git a/crates/oxc_semantic/tests/integration/util/mod.rs b/crates/oxc_semantic/tests/integration/util/mod.rs index 569b32348270e..ed333e520d987 100644 --- a/crates/oxc_semantic/tests/integration/util/mod.rs +++ b/crates/oxc_semantic/tests/integration/util/mod.rs @@ -175,7 +175,6 @@ impl<'a> SemanticTester<'a> { SemanticBuilder::new() .with_check_syntax_error(true) .with_cfg(self.cfg) - .with_scope_tree_child_ids(self.scope_tree_child_ids) .build(self.allocator.alloc(parse.program)) } diff --git a/crates/oxc_transformer/src/es2018/async_generator_functions/for_await.rs b/crates/oxc_transformer/src/es2018/async_generator_functions/for_await.rs index 3adea897fc5a8..1b58f2369abac 100644 --- a/crates/oxc_transformer/src/es2018/async_generator_functions/for_await.rs +++ b/crates/oxc_transformer/src/es2018/async_generator_functions/for_await.rs @@ -134,14 +134,10 @@ impl<'a> AsyncGeneratorFunctions<'a, '_> { let mut statements = ctx.ast.vec_with_capacity(2); statements.push(assignment_statement); let stmt_body = &mut stmt.body; - if let Statement::BlockStatement(block) = stmt_body { - if block.body.is_empty() { - // If the block is empty, we don’t need to add it to the body; - // instead, we need to remove the useless scope. - ctx.scoping_mut().delete_scope(block.scope_id()); - } else { - statements.push(stmt_body.take_in(ctx.ast)); - } + if let Statement::BlockStatement(block) = stmt_body + && !block.body.is_empty() + { + statements.push(stmt_body.take_in(ctx.ast)); } statements }; diff --git a/crates/oxc_transformer/src/es2022/class_properties/static_block_and_prop_init.rs b/crates/oxc_transformer/src/es2022/class_properties/static_block_and_prop_init.rs index 4ac31994ae62c..073f4bf905708 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/static_block_and_prop_init.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/static_block_and_prop_init.rs @@ -70,7 +70,6 @@ impl<'a> ClassProperties<'a, '_> { { return self.convert_static_block_with_single_expression_to_expression( &mut stmt.expression, - scope_id, make_sloppy_mode, ctx, ); @@ -93,7 +92,6 @@ impl<'a> ClassProperties<'a, '_> { fn convert_static_block_with_single_expression_to_expression( &mut self, expr: &mut Expression<'a>, - scope_id: ScopeId, make_sloppy_mode: bool, ctx: &mut TraverseCtx<'a>, ) -> Expression<'a> { @@ -101,9 +99,6 @@ impl<'a> ClassProperties<'a, '_> { let mut replacer = StaticVisitor::new(make_sloppy_mode, true, self, ctx); replacer.visit_expression(expr); - // Delete scope for static block - ctx.scoping_mut().delete_scope(scope_id); - expr.take_in(ctx.ast) } diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index 2fc1446ca7d90..f5ce6c9400a90 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -156,11 +156,6 @@ impl<'a> TraverseScoping<'a> { child_scope_ids: &[ScopeId], flags: ScopeFlags, ) -> ScopeId { - // Remove these scopes from parent's children - if self.scoping.has_scope_child_ids() { - self.scoping.remove_child_scopes(scope_id, child_scope_ids); - } - // Create new scope as child of parent let new_scope_id = self.create_child_scope(scope_id, flags); @@ -205,9 +200,6 @@ impl<'a> TraverseScoping<'a> { "Child scope must be a child of parent scope" ); - if self.scoping.has_scope_child_ids() { - self.scoping.remove_child_scope(parent_id, child_id); - } self.scoping.set_scope_parent_id(child_id, Some(scope_id)); scope_id } @@ -234,8 +226,6 @@ impl<'a> TraverseScoping<'a> { self.scoping.set_scope_parent_id(child_id, parent_id); } } - - self.scoping.delete_scope(scope_id); } /// Add binding to `ScopeTree` and `SymbolTable`. diff --git a/tasks/transform_checker/src/lib.rs b/tasks/transform_checker/src/lib.rs index 58520081866a0..7ec152b291ef8 100644 --- a/tasks/transform_checker/src/lib.rs +++ b/tasks/transform_checker/src/lib.rs @@ -134,11 +134,7 @@ pub fn check_semantic_after_transform( // so the cloned AST will be "clean" of all semantic data, as if it had come fresh from the parser. let allocator = Allocator::default(); let program = program.clone_in(&allocator); - let scoping_rebuilt = SemanticBuilder::new() - .with_scope_tree_child_ids(scoping_after_transform.has_scope_child_ids()) - .build(&program) - .semantic - .into_scoping(); + let scoping_rebuilt = SemanticBuilder::new().build(&program).semantic.into_scoping(); let (scope_ids_rebuilt, symbol_ids_rebuilt, reference_ids_rebuilt, _) = SemanticIdsCollector::new(&mut errors).collect(&program); @@ -367,16 +363,6 @@ impl PostTransformChecker<'_, '_> { self.errors.push_mismatch("Scope parent mismatch", scope_ids, parent_ids); } - // Check children match - if self.scoping_after_transform.has_scope_child_ids() { - let child_ids = self.get_pair(scope_ids, |scoping, scope_id| { - scoping.get_scope_child_ids(scope_id).to_vec() - }); - if self.remap_scope_ids_sets(&child_ids).is_mismatch() { - self.errors.push_mismatch("Scope children mismatch", scope_ids, child_ids); - } - } - // NB: Skip checking node IDs match - transformer does not set `NodeId`s } } @@ -538,28 +524,6 @@ impl PostTransformChecker<'_, '_> { Pair::new(self.scope_ids_map.get(scope_ids.after_transform), Some(scope_ids.rebuilt)) } - /// Remap pair of arrays of `ScopeId`s. - /// Map `after_transform` IDs to `rebuilt` IDs. - /// Sort both sets. - fn remap_scope_ids_sets>>( - &self, - scope_ids: &Pair, - ) -> Pair>> { - let mut after_transform = scope_ids - .after_transform - .as_ref() - .iter() - .map(|&scope_id| self.scope_ids_map.get(scope_id)) - .collect::>(); - let mut rebuilt = - scope_ids.rebuilt.as_ref().iter().copied().map(Option::Some).collect::>(); - - after_transform.sort_unstable(); - rebuilt.sort_unstable(); - - Pair::new(after_transform, rebuilt) - } - /// Remap pair of arrays of `SymbolId`s. /// Map `after_transform` IDs to `rebuilt` IDs. /// Sort both sets.