dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 12:19:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(semantic): change Bindings to a plain FxHashMap (#8019)

Browse source 
`IndexMap` was needed for the insertion order requirement in mangler.

Bindings (symbol ids) are monotonically increasing by binding
positions, which means we can get insertion order by sorting the symbol ids in
mangler.

Previous attempt: https://github.com/oxc-project/oxc/pull/4228
This commit is contained in:
Boshen 2024-12-19 14:16:28 +00:00
parent 862838fd28
commit 02f968d02d
9 changed files with 29 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -1914,7 +1914,6 @@ name = "oxc_semantic"
version = "0.42.0"
dependencies = [
"assert-unchecked",
"indexmap",
"insta",
"itertools",
"oxc_allocator",

9
crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs
View file

@ -43,9 +43,10 @@ impl Rule for NoUnsafeDeclarationMerging {
match node.kind() {
AstKind::Class(decl) => {
if let Some(ident) = decl.id.as_ref() {
for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) {
for symbol_id in ctx.semantic().scopes().get_bindings(node.scope_id()).values()
{
if let AstKind::TSInterfaceDeclaration(scope_interface) =
get_symbol_kind(symbol_id, ctx)
get_symbol_kind(*symbol_id, ctx)
{
check_and_diagnostic(ident, &scope_interface.id, ctx);
}
@ -53,8 +54,8 @@ impl Rule for NoUnsafeDeclarationMerging {
}
}
AstKind::TSInterfaceDeclaration(decl) => {
for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) {
if let AstKind::Class(scope_class) = get_symbol_kind(symbol_id, ctx) {
for symbol_id in ctx.semantic().scopes().get_bindings(node.scope_id()).values() {
if let AstKind::Class(scope_class) = get_symbol_kind(*symbol_id, ctx) {
if let Some(scope_class_ident) = scope_class.id.as_ref() {
check_and_diagnostic(&decl.id, scope_class_ident, ctx);
}

6
crates/oxc_mangler/src/lib.rs
View file

@ -110,8 +110,10 @@ impl Mangler {
let mut slot = parent_max_slot;
if !bindings.is_empty() {
// `bindings` are stored in order, traverse and increment slot
for symbol_id in bindings.values().copied() {
// Sort `bindings` in declaration order.
let mut bindings = bindings.values().copied().collect::<Vec<_>>();
bindings.sort_unstable();
for symbol_id in bindings {
slots[symbol_id] = slot;
slot += 1;
}

5
crates/oxc_semantic/Cargo.toml
View file

@ -30,16 +30,13 @@ oxc_span = { workspace = true }
oxc_syntax = { workspace = true }
assert-unchecked = { workspace = true }
indexmap = { workspace = true }
itertools = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
[dev-dependencies]
oxc_parser = { workspace = true }
indexmap = { workspace = true }
insta = { workspace = true, features = ["glob"] }
oxc_parser = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
serde_json = { workspace = true }

21
crates/oxc_semantic/src/builder.rs
View file

@ -23,18 +23,9 @@ use oxc_syntax::{
};
use crate::{
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,
binder::Binder, checker, class::ClassTableBuilder, diagnostics::redeclaration,
jsdoc::JSDocBuilder, label::UnusedLabels, node::AstNodes, scope::ScopeTree, stats::Stats,
symbol::SymbolTable, unresolved_stack::UnresolvedReferencesStack, JSDocFinder, Semantic,
};
macro_rules! control_flow {
@ -713,10 +704,10 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
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 = parent_bindings.drain(..).collect::<Bindings>();
parent_bindings.values().for_each(|&symbol_id| {
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.get_bindings_mut(self.current_scope_id) = parent_bindings;
}
}

15
crates/oxc_semantic/src/scope.rs
View file

@ -1,7 +1,6 @@
use std::mem;
use indexmap::IndexMap;
use rustc_hash::{FxBuildHasher, FxHashMap};
use rustc_hash::FxHashMap;
use oxc_index::IndexVec;
use oxc_span::CompactStr;
@ -12,9 +11,7 @@ use oxc_syntax::{
symbol::SymbolId,
};
type FxIndexMap<K, V> = IndexMap<K, V, FxBuildHasher>;
pub(crate) type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type Bindings = FxHashMap<CompactStr, SymbolId>;
pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<ReferenceId>>;
/// Scope Tree
@ -203,7 +200,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].get(name).is_some()
self.bindings[scope_id].contains_key(name)
}
/// Get the symbol bound to an identifier name in a scope.
@ -330,13 +327,13 @@ impl ScopeTree {
/// Remove an existing binding from a scope.
pub fn remove_binding(&mut self, scope_id: ScopeId, name: &CompactStr) {
self.bindings[scope_id].shift_remove(name);
self.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.swap_remove_entry(name) {
if let Some((name, symbol_id)) = from_map.remove_entry(name) {
self.bindings[to].insert(name, symbol_id);
}
}
@ -365,7 +362,7 @@ impl ScopeTree {
// 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.swap_remove(old_name);
let old_symbol_id = bindings.remove(old_name);
debug_assert_eq!(old_symbol_id, Some(symbol_id));
}

3
crates/oxc_semantic/tests/main.rs
View file

@ -46,7 +46,8 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
.as_str(),
);
result.push_str("\"symbols\": ");
let bindings = scope_tree.get_bindings(scope_id);
let mut bindings = scope_tree.get_bindings(scope_id).iter().collect::<Vec<_>>();
bindings.sort_unstable_by_key(|&(_, symbol_id)| symbol_id);
result.push('[');
bindings.iter().enumerate().for_each(|(index, (name, &symbol_id))| {
if index != 0 {

4
crates/oxc_wasm/src/lib.rs
View file

@ -387,10 +387,10 @@ impl Oxc {
let bindings = self.scopes.get_bindings(scope_id);
if !bindings.is_empty() {
self.write_line("Bindings: {");
bindings.iter().for_each(|(name, &symbol_id)| {
for (name, &symbol_id) in bindings {
let symbol_flags = self.symbols.get_flags(symbol_id);
self.write_line(format!(" {name} ({symbol_id:?} {symbol_flags:?})",));
});
}
self.write_line("}");
}
}

4
tasks/transform_checker/src/lib.rs
View file

@ -346,10 +346,12 @@ impl PostTransformChecker<'_, '_> {
if binding_names.is_mismatch() {
self.errors.push_mismatch("Bindings mismatch", scope_ids, binding_names);
} else {
let symbol_ids = self.get_pair(scope_ids, |scoping, scope_id| {
let mut symbol_ids = self.get_pair(scope_ids, |scoping, scope_id| {
scoping.scopes.get_bindings(scope_id).values().copied().collect::<Vec<_>>()
});
if self.remap_symbol_ids_sets(&symbol_ids).is_mismatch() {
symbol_ids.after_transform.sort_unstable();
symbol_ids.rebuilt.sort_unstable();
self.errors.push_mismatch("Binding symbols mismatch", scope_ids, symbol_ids);
}
}