diff --git a/Cargo.lock b/Cargo.lock index a6ccd065c2629..622b708445953 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -568,6 +568,12 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "foldhash" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f81ec6369c545a7d40e4589b5597581fa1c441fe1cce96dd1de43159910a36a2" + [[package]] name = "form_urlencoded" version = "1.2.1" @@ -765,6 +771,11 @@ name = "hashbrown" version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] [[package]] name = "hashlink" @@ -1914,6 +1925,8 @@ name = "oxc_semantic" version = "0.42.0" dependencies = [ "assert-unchecked", + "bumpalo", + "hashbrown 0.15.2", "insta", "itertools", "oxc_allocator", @@ -1928,6 +1941,7 @@ dependencies = [ "oxc_syntax", "phf", "rustc-hash", + "self_cell", "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 27b326ab7dbbf..d0af07feddeaa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -152,6 +152,7 @@ futures = "0.3.31" glob = "0.3.1" globset = "0.4.15" handlebars = "6.2.0" +hashbrown = "0.15.2" humansize = "2.1.3" ignore = "0.4.23" insta = "1.41.1" diff --git a/crates/oxc_semantic/Cargo.toml b/crates/oxc_semantic/Cargo.toml index 0afe38e353f97..98f06e30f2035 100644 --- a/crates/oxc_semantic/Cargo.toml +++ b/crates/oxc_semantic/Cargo.toml @@ -30,9 +30,12 @@ oxc_span = { workspace = true } oxc_syntax = { workspace = true } assert-unchecked = { workspace = true } +bumpalo = { workspace = true, features = ["allocator-api2"] } +hashbrown = { workspace = true, features = ["allocator-api2"] } itertools = { workspace = true } phf = { workspace = true, features = ["macros"] } rustc-hash = { workspace = true } +self_cell = { workspace = true } [dev-dependencies] insta = { workspace = true, features = ["glob"] } diff --git a/crates/oxc_semantic/src/binder.rs b/crates/oxc_semantic/src/binder.rs index dbfe8b2cfb05c..9747af557a0de 100644 --- a/crates/oxc_semantic/src/binder.rs +++ b/crates/oxc_semantic/src/binder.rs @@ -68,10 +68,9 @@ impl<'a> Binder<'a> for VariableDeclarator<'a> { builder.add_redeclare_variable(symbol_id, span); declared_symbol_id = Some(symbol_id); - let name = name.to_compact_str(); // remove current scope binding and add to target scope // avoid same symbols appear in multi-scopes - builder.scope.remove_binding(scope_id, &name); + builder.scope.remove_binding(scope_id, name); builder.scope.add_binding(target_scope_id, name, symbol_id); builder.symbols.scope_ids[symbol_id] = target_scope_id; break; diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 049addc0ab971..0ed9d2de0f853 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -23,9 +23,18 @@ use oxc_syntax::{ }; use crate::{ - binder::Binder, checker, class::ClassTableBuilder, diagnostics::redeclaration, - jsdoc::JSDocBuilder, label::UnusedLabels, node::AstNodes, scope::ScopeTree, stats::Stats, - symbol::SymbolTable, unresolved_stack::UnresolvedReferencesStack, JSDocFinder, Semantic, + binder::Binder, + checker, + class::ClassTableBuilder, + diagnostics::redeclaration, + jsdoc::JSDocBuilder, + label::UnusedLabels, + node::AstNodes, + scope::{Bindings, ScopeTree}, + stats::Stats, + symbol::SymbolTable, + unresolved_stack::UnresolvedReferencesStack, + JSDocFinder, Semantic, }; macro_rules! control_flow { @@ -389,10 +398,9 @@ impl<'a> SemanticBuilder<'a> { return symbol_id; } - let name = CompactStr::new(name); let symbol_id = self.symbols.create_symbol( span, - name.clone(), + CompactStr::new(name), includes, scope_id, self.current_node_id, @@ -456,15 +464,14 @@ impl<'a> SemanticBuilder<'a> { scope_id: ScopeId, includes: SymbolFlags, ) -> SymbolId { - let name = CompactStr::new(name); let symbol_id = self.symbols.create_symbol( span, - name.clone(), + CompactStr::new(name), includes, self.current_scope_id, self.current_node_id, ); - self.scope.get_bindings_mut(scope_id).insert(name, symbol_id); + self.scope.insert_binding(scope_id, name, symbol_id); symbol_id } @@ -702,14 +709,17 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { // Move all bindings from catch clause param scope to catch clause body scope // to make it easier to resolve references and check redeclare errors if self.scope.get_flags(parent_scope_id).is_catch_clause() { - let parent_bindings = self.scope.get_bindings_mut(parent_scope_id); - if !parent_bindings.is_empty() { - let parent_bindings = mem::take(parent_bindings); - for &symbol_id in parent_bindings.values() { - self.symbols.set_scope_id(symbol_id, self.current_scope_id); + self.scope.cell.with_dependent_mut(|allocator, inner| { + if !inner.bindings[parent_scope_id].is_empty() { + let mut parent_bindings = + Bindings::with_hasher_in(rustc_hash::FxBuildHasher, allocator); + mem::swap(&mut inner.bindings[parent_scope_id], &mut parent_bindings); + for &symbol_id in parent_bindings.values() { + self.symbols.set_scope_id(symbol_id, self.current_scope_id); + } + inner.bindings[self.current_scope_id] = parent_bindings; } - *self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings; - } + }); } self.visit_statements(&it.body); diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index 158f93611d728..1680db8df736c 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -282,7 +282,7 @@ mod tests { let top_level_a = semantic .scopes() .iter_bindings() - .find(|(_scope_id, _symbol_id, name)| name.as_str() == "Fn") + .find(|(_scope_id, _symbol_id, name)| *name == "Fn") .unwrap(); assert_eq!(semantic.symbols.get_scope_id(top_level_a.1), top_level_a.0); } diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index b1d5d066741fc..9d7927292310c 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -1,7 +1,8 @@ use std::mem; -use rustc_hash::FxHashMap; +use rustc_hash::{FxBuildHasher, FxHashMap}; +use bumpalo::Bump; use oxc_index::IndexVec; use oxc_span::CompactStr; use oxc_syntax::{ @@ -11,7 +12,7 @@ use oxc_syntax::{ symbol::SymbolId, }; -pub(crate) type Bindings = FxHashMap; +pub(crate) type Bindings<'a> = hashbrown::HashMap<&'a str, SymbolId, FxBuildHasher, &'a Bump>; pub type UnresolvedReferences = FxHashMap>; /// Scope Tree @@ -24,7 +25,6 @@ pub type UnresolvedReferences = FxHashMap>; /// - Nodes that create a scope store the [`ScopeId`] of the scope they create. /// /// `SoA` (Struct of Arrays) for memory efficiency. -#[derive(Debug, Default)] pub struct ScopeTree { /// Maps a scope to the parent scope it belongs in. parent_ids: IndexVec>, @@ -40,12 +40,40 @@ pub struct ScopeTree { flags: IndexVec, + pub(crate) root_unresolved_references: UnresolvedReferences, + + pub(crate) cell: ScopeTreeCell, +} + +impl Default for ScopeTree { + fn default() -> Self { + Self { + parent_ids: IndexVec::new(), + child_ids: IndexVec::new(), + build_child_ids: false, + node_ids: IndexVec::new(), + flags: IndexVec::new(), + root_unresolved_references: UnresolvedReferences::default(), + cell: ScopeTreeCell::new(Bump::new(), |_bump| ScopeTreeInner { + bindings: IndexVec::new(), + }), + } + } +} + +self_cell::self_cell!( + pub(crate) struct ScopeTreeCell { + owner: Bump, + #[covariant] + dependent: ScopeTreeInner, + } +); + +pub(crate) struct ScopeTreeInner<'cell> { /// Symbol bindings in a scope. /// /// A binding is a mapping from an identifier name to its [`SymbolId`] - bindings: IndexVec, - - pub(crate) root_unresolved_references: UnresolvedReferences, + pub(crate) bindings: IndexVec>, } impl ScopeTree { @@ -200,7 +228,7 @@ impl ScopeTree { /// Check if a symbol is declared in a certain scope. pub fn has_binding(&self, scope_id: ScopeId, name: &str) -> bool { - self.bindings[scope_id].contains_key(name) + self.cell.borrow_dependent().bindings[scope_id].contains_key(name) } /// Get the symbol bound to an identifier name in a scope. @@ -212,7 +240,7 @@ impl ScopeTree { /// /// [`find_binding`]: ScopeTree::find_binding pub fn get_binding(&self, scope_id: ScopeId, name: &str) -> Option { - self.bindings[scope_id].get(name).copied() + self.cell.borrow_dependent().bindings[scope_id].get(name).copied() } /// Find a binding by name in a scope or its ancestors. @@ -221,7 +249,7 @@ impl ScopeTree { /// found. If no binding is found, [`None`] is returned. pub fn find_binding(&self, scope_id: ScopeId, name: &str) -> Option { for scope_id in self.ancestors(scope_id) { - if let Some(&symbol_id) = self.bindings[scope_id].get(name) { + if let Some(symbol_id) = self.get_binding(scope_id, name) { return Some(symbol_id); } } @@ -231,7 +259,7 @@ impl ScopeTree { /// Get all bound identifiers in a scope. #[inline] pub fn get_bindings(&self, scope_id: ScopeId) -> &Bindings { - &self.bindings[scope_id] + &self.cell.borrow_dependent().bindings[scope_id] } /// Get the ID of the [`AstNode`] that created a scope. @@ -247,21 +275,24 @@ impl ScopeTree { /// If you only want bindings in a specific scope, use [`iter_bindings_in`]. /// /// [`iter_bindings_in`]: ScopeTree::iter_bindings_in - pub fn iter_bindings(&self) -> impl Iterator + '_ { - self.bindings.iter_enumerated().flat_map(|(scope_id, bindings)| { - bindings.iter().map(move |(name, &symbol_id)| (scope_id, symbol_id, name)) + pub fn iter_bindings(&self) -> impl Iterator + '_ { + self.cell.borrow_dependent().bindings.iter_enumerated().flat_map(|(scope_id, bindings)| { + bindings.iter().map(move |(&name, &symbol_id)| (scope_id, symbol_id, name)) }) } /// Iterate over bindings declared inside a scope. #[inline] pub fn iter_bindings_in(&self, scope_id: ScopeId) -> impl Iterator + '_ { - self.bindings[scope_id].values().copied() + self.cell.borrow_dependent().bindings[scope_id].values().copied() } #[inline] - pub(crate) fn get_bindings_mut(&mut self, scope_id: ScopeId) -> &mut Bindings { - &mut self.bindings[scope_id] + pub(crate) fn insert_binding(&mut self, scope_id: ScopeId, name: &str, symbol_id: SymbolId) { + self.cell.with_dependent_mut(|allocator, inner| { + let name = allocator.alloc_str(name); + inner.bindings[scope_id].insert(name, symbol_id); + }); } /// Return whether this `ScopeTree` has child IDs recorded @@ -307,7 +338,9 @@ impl ScopeTree { ) -> ScopeId { let scope_id = self.parent_ids.push(parent_id); self.flags.push(flags); - self.bindings.push(Bindings::default()); + self.cell.with_dependent_mut(|allocator, inner| { + inner.bindings.push(Bindings::with_hasher_in(FxBuildHasher, allocator)); + }); self.node_ids.push(node_id); if self.build_child_ids { self.child_ids.push(vec![]); @@ -321,21 +354,28 @@ impl ScopeTree { /// Add a binding to a scope. /// /// [`binding`]: Bindings - pub fn add_binding(&mut self, scope_id: ScopeId, name: CompactStr, symbol_id: SymbolId) { - self.bindings[scope_id].insert(name, symbol_id); + pub fn add_binding(&mut self, scope_id: ScopeId, name: &str, symbol_id: SymbolId) { + self.cell.with_dependent_mut(|allocator, inner| { + let name = allocator.alloc_str(name); + inner.bindings[scope_id].insert(name, symbol_id); + }); } /// Remove an existing binding from a scope. - pub fn remove_binding(&mut self, scope_id: ScopeId, name: &CompactStr) { - self.bindings[scope_id].remove(name); + pub fn remove_binding(&mut self, scope_id: ScopeId, name: &str) { + self.cell.with_dependent_mut(|_allocator, inner| { + inner.bindings[scope_id].remove(name); + }); } /// Move a binding from one scope to another. pub fn move_binding(&mut self, from: ScopeId, to: ScopeId, name: &str) { - let from_map = &mut self.bindings[from]; - if let Some((name, symbol_id)) = from_map.remove_entry(name) { - self.bindings[to].insert(name, symbol_id); - } + self.cell.with_dependent_mut(|_allocator, inner| { + let from_map = &mut inner.bindings[from]; + if let Some((name, symbol_id)) = from_map.remove_entry(name) { + inner.bindings[to].insert(name, symbol_id); + } + }); } /// Rename a binding to a new name. @@ -353,24 +393,29 @@ impl ScopeTree { scope_id: ScopeId, symbol_id: SymbolId, old_name: &str, - new_name: CompactStr, + new_name: &str, ) { - let bindings = &mut self.bindings[scope_id]; - // Insert on end - let existing_symbol_id = bindings.insert(new_name, symbol_id); - debug_assert!(existing_symbol_id.is_none()); - // Remove old entry. `swap_remove` swaps the last entry into the place of removed entry. - // We just inserted the new entry as last, so the new entry takes the place of the old. - // Order of entries is same as before, only the key has changed. - let old_symbol_id = bindings.remove(old_name); - debug_assert_eq!(old_symbol_id, Some(symbol_id)); + self.cell.with_dependent_mut(|allocator, inner| { + let new_name = allocator.alloc_str(new_name); + let bindings = &mut inner.bindings[scope_id]; + // Insert on end + let existing_symbol_id = bindings.insert(new_name, symbol_id); + debug_assert!(existing_symbol_id.is_none()); + // Remove old entry. `swap_remove` swaps the last entry into the place of removed entry. + // We just inserted the new entry as last, so the new entry takes the place of the old. + // Order of entries is same as before, only the key has changed. + let old_symbol_id = bindings.remove(old_name); + debug_assert_eq!(old_symbol_id, Some(symbol_id)); + }); } /// Reserve memory for an `additional` number of scopes. pub fn reserve(&mut self, additional: usize) { self.parent_ids.reserve(additional); self.flags.reserve(additional); - self.bindings.reserve(additional); + self.cell.with_dependent_mut(|_allocator, inner| { + inner.bindings.reserve(additional); + }); self.node_ids.reserve(additional); if self.build_child_ids { self.child_ids.reserve(additional); diff --git a/crates/oxc_semantic/tests/integration/util/symbol_tester.rs b/crates/oxc_semantic/tests/integration/util/symbol_tester.rs index e0ad1d5bc61aa..78672bed0855f 100644 --- a/crates/oxc_semantic/tests/integration/util/symbol_tester.rs +++ b/crates/oxc_semantic/tests/integration/util/symbol_tester.rs @@ -2,7 +2,6 @@ use std::rc::Rc; use oxc_diagnostics::{Error, OxcDiagnostic}; use oxc_semantic::{Reference, ScopeFlags, ScopeId, Semantic, SymbolFlags, SymbolId}; -use oxc_span::CompactStr; use super::{Expect, SemanticTester}; @@ -66,11 +65,8 @@ impl<'a> SymbolTester<'a> { semantic: Semantic<'a>, target: &str, ) -> Self { - let symbols_with_target_name: Vec<_> = semantic - .scopes() - .iter_bindings() - .filter(|(_, _, name)| name.as_str() == target) - .collect(); + let symbols_with_target_name: Vec<_> = + semantic.scopes().iter_bindings().filter(|(_, _, name)| *name == target).collect(); let data = match symbols_with_target_name.len() { 0 => Err(OxcDiagnostic::error(format!("Could not find declaration for {target}"))), 1 => Ok(symbols_with_target_name @@ -102,8 +98,8 @@ impl<'a> SymbolTester<'a> { semantic: Semantic<'a>, target: &str, ) -> Self { - let symbols_with_target_name: Option<(ScopeId, SymbolId, &CompactStr)> = - semantic.scopes().iter_bindings().find(|(_, _, name)| name.as_str() == target); + let symbols_with_target_name: Option<(ScopeId, SymbolId, &str)> = + semantic.scopes().iter_bindings().find(|(_, _, name)| *name == target); let data = match symbols_with_target_name { Some((_, symbol_id, _)) => Ok(symbol_id), diff --git a/crates/oxc_traverse/src/context/mod.rs b/crates/oxc_traverse/src/context/mod.rs index ce181c41c5ef9..4df72220d5c43 100644 --- a/crates/oxc_traverse/src/context/mod.rs +++ b/crates/oxc_traverse/src/context/mod.rs @@ -358,7 +358,7 @@ impl<'a> TraverseCtx<'a> { // Get name for UID let name = self.generate_uid_name(name); let name_atom = self.ast.atom(&name); - let symbol_id = self.scoping.add_binding(name, scope_id, flags); + let symbol_id = self.scoping.add_binding(&name, scope_id, flags); BoundIdentifier::new(name_atom, symbol_id) } diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index 5982c9abcd841..d8e150c13d4c2 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -171,12 +171,12 @@ impl TraverseScoping { #[inline] pub(crate) fn add_binding( &mut self, - name: CompactStr, + name: &str, scope_id: ScopeId, flags: SymbolFlags, ) -> SymbolId { let symbol_id = - self.symbols.create_symbol(SPAN, name.clone(), flags, scope_id, NodeId::DUMMY); + self.symbols.create_symbol(SPAN, name.into(), flags, scope_id, NodeId::DUMMY); self.scopes.add_binding(scope_id, name, symbol_id); symbol_id @@ -191,7 +191,7 @@ impl TraverseScoping { scope_id: ScopeId, flags: SymbolFlags, ) -> BoundIdentifier<'a> { - let symbol_id = self.add_binding(name.to_compact_str(), scope_id, flags); + let symbol_id = self.add_binding(name.as_str(), scope_id, flags); BoundIdentifier::new(name, symbol_id) } @@ -372,11 +372,12 @@ impl TraverseScoping { /// * No binding already exists in scope for `new_name`. /// /// Panics in debug mode if either of the above are not satisfied. + #[expect(clippy::needless_pass_by_value)] pub fn rename_symbol(&mut self, symbol_id: SymbolId, scope_id: ScopeId, new_name: CompactStr) { // Rename symbol let old_name = self.symbols.set_name(symbol_id, new_name.clone()); // Rename binding - self.scopes.rename_binding(scope_id, symbol_id, &old_name, new_name); + self.scopes.rename_binding(scope_id, symbol_id, &old_name, new_name.as_str()); } } diff --git a/tasks/transform_checker/src/lib.rs b/tasks/transform_checker/src/lib.rs index cae83c7f27b0f..ae90571598aa3 100644 --- a/tasks/transform_checker/src/lib.rs +++ b/tasks/transform_checker/src/lib.rs @@ -336,8 +336,12 @@ impl PostTransformChecker<'_, '_> { for scope_ids in self.scope_ids_map.pairs() { // Check bindings are the same fn get_sorted_binding_names(scoping: &Scoping, scope_id: ScopeId) -> Vec { - let mut binding_names = - scoping.scopes.get_bindings(scope_id).keys().cloned().collect::>(); + let mut binding_names = scoping + .scopes + .get_bindings(scope_id) + .keys() + .map(|k| CompactStr::new(k)) + .collect::>(); binding_names.sort_unstable(); binding_names }