Skip to content

feat(semantic)!: remove Scoping::scope_build_child_ids and all related APIs #18362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Jan 22, 2026
Merged

feat(semantic)!: remove Scoping::scope_build_child_ids and all related APIs #18362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 0 additions & 6 deletions crates/oxc_semantic/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
112 changes: 2 additions & 110 deletions crates/oxc_semantic/src/scoping.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ pub struct Scoping {
/// Maps a scope to the parent scope it belongs in.
scope_parent_ids: IndexVec<ScopeId, Option<ScopeId>>,

/// Runtime flag for constructing child_ids.
pub(crate) scope_build_child_ids: bool,

/// Maps a scope to its node id.
scope_node_ids: IndexVec<ScopeId, NodeId>,

Expand All @@ -99,15 +96,13 @@ 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 {
symbol_names: ArenaVec::new_in(allocator),
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),
}),
}
Expand Down Expand Up @@ -269,9 +264,6 @@ pub struct ScopingInner<'cell> {
/// A binding is a mapping from an identifier name to its [`SymbolId`]
pub(crate) bindings: IndexVec<ScopeId, Bindings<'cell>>,

/// Maps a scope to direct children scopes.
scope_child_ids: ArenaVec<'cell, ArenaVec<'cell, ScopeId>>,

pub(crate) root_unresolved_references: UnresolvedReferences<'cell>,
}

Expand Down Expand Up @@ -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<SymbolId> {
Expand Down Expand Up @@ -688,48 +675,13 @@ impl Scoping {

pub fn set_scope_parent_id(&mut self, scope_id: ScopeId, parent_id: Option<ScopeId>) {
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<ScopeId>) {
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
Expand Down Expand Up @@ -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<Item = ScopeId> + '_ {
let mut stack = self.cell.borrow_dependent().scope_child_ids[scope_id.index()]
.iter()
.copied()
.collect::<Vec<_>>();
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(
Expand All @@ -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
}

Expand Down Expand Up @@ -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: {
Expand All @@ -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),
Expand Down
69 changes: 0 additions & 69 deletions crates/oxc_semantic/tests/integration/scopes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> = T extends string ? T : false;")
Expand Down Expand Up @@ -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");
}
1 change: 0 additions & 1 deletion crates/oxc_semantic/tests/integration/util/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
12 changes: 4 additions & 8 deletions crates/oxc_transformer/src/es2018/async_generator_functions/for_await.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
5 changes: 0 additions & 5 deletions crates/oxc_transformer/src/es2022/class_properties/static_block_and_prop_init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand All @@ -93,17 +92,13 @@ 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> {
// Note: Reparent scopes
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)
}

Expand Down
10 changes: 0 additions & 10 deletions crates/oxc_traverse/src/context/scoping.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
}
Expand All @@ -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`.
Expand Down
Loading
Loading