Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1039,11 +1039,10 @@ impl<'a> ArrowFunctionConverter<'a> {
let binding = self.arguments_var_stack.last_or_init(|| {
if let Some(symbol_id) = symbol_id {
let arguments_name = ctx.generate_uid_name("arguments");
let arguments_name_atom = ctx.ast.atom(&arguments_name);
Self::rename_arguments_symbol(symbol_id, arguments_name, ctx);
Self::rename_arguments_symbol(symbol_id, arguments_name.into_compact_str(), ctx);
// Record the symbol ID as a renamed `arguments` variable.
self.renamed_arguments_symbol_ids.insert(symbol_id);
BoundIdentifier::new(arguments_name_atom, symbol_id)
BoundIdentifier::new(arguments_name, symbol_id)
} else {
// We cannot determine the final scope ID of the `arguments` variable insertion,
// because the `arguments` variable will be inserted to a new scope which haven't been created yet,
Expand Down Expand Up @@ -1081,9 +1080,9 @@ impl<'a> ArrowFunctionConverter<'a> {

self.arguments_var_stack.last_or_init(|| {
let arguments_name = ctx.generate_uid_name("arguments");
ident.name = ctx.ast.atom(&arguments_name);
ident.name = arguments_name;
let symbol_id = ident.symbol_id();
Self::rename_arguments_symbol(symbol_id, arguments_name, ctx);
Self::rename_arguments_symbol(symbol_id, arguments_name.into_compact_str(), ctx);
// Record the symbol ID as a renamed `arguments` variable.
self.renamed_arguments_symbol_ids.insert(symbol_id);
BoundIdentifier::new(ident.name, symbol_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,9 @@ impl<'a> ClassProperties<'a, '_> {
// Generate replacement UID name
let new_name = ctx.generate_uid_name(name);
// Save replacement name in `clashing_symbols`
*name = ctx.ast.atom(&new_name);
*name = new_name;
// Rename symbol and binding
ctx.rename_symbol(symbol_id, constructor_scope_id, new_name);
ctx.rename_symbol(symbol_id, constructor_scope_id, new_name.into_compact_str());
}

// Rename identifiers for clashing symbols in constructor params and body
Expand Down
1 change: 0 additions & 1 deletion crates/oxc_traverse/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,5 @@ oxc_semantic = { workspace = true }
oxc_span = { workspace = true }
oxc_syntax = { workspace = true }

compact_str = { workspace = true }
itoa = { workspace = true }
rustc-hash = { workspace = true }
11 changes: 5 additions & 6 deletions crates/oxc_traverse/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub use scoping::TraverseScoping;
/// [`alloc`]: `TraverseCtx::alloc`
pub struct TraverseCtx<'a> {
pub ancestry: TraverseAncestry<'a>,
pub scoping: TraverseScoping,
pub scoping: TraverseScoping<'a>,
pub ast: AstBuilder<'a>,
}

Expand Down Expand Up @@ -328,7 +328,7 @@ impl<'a> TraverseCtx<'a> {
/// the resulting scopes will be:
/// ```ts
/// parentScope1: {
/// newScope: {
/// newScope: {
/// childScope: { }
/// }
/// childScope2: { }
Expand Down Expand Up @@ -396,8 +396,8 @@ impl<'a> TraverseCtx<'a> {
/// There are some potential "gotchas".
///
/// This is a shortcut for `ctx.scoping.generate_uid_name`.
pub fn generate_uid_name(&mut self, name: &str) -> CompactStr {
self.scoping.generate_uid_name(name)
pub fn generate_uid_name(&mut self, name: &str) -> Atom<'a> {
self.scoping.generate_uid_name(name, self.ast.allocator)
}

/// Generate UID.
Expand All @@ -413,9 +413,8 @@ impl<'a> TraverseCtx<'a> {
) -> BoundIdentifier<'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);
BoundIdentifier::new(name_atom, symbol_id)
BoundIdentifier::new(name, symbol_id)
}

/// Generate UID in current scope.
Expand Down
58 changes: 35 additions & 23 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use std::str;

use compact_str::CompactString;
use itoa::Buffer as ItoaBuffer;
use rustc_hash::FxHashSet;

use oxc_allocator::Vec as ArenaVec;
use oxc_allocator::{Allocator, String as ArenaString, Vec as ArenaVec};
use oxc_ast::ast::*;
use oxc_ast_visit::Visit;
use oxc_semantic::{NodeId, Reference, Scoping};
Expand All @@ -23,16 +22,16 @@ use crate::{BoundIdentifier, scopes_collector::ChildScopeCollector};
///
/// `current_scope_id` is the ID of current scope during traversal.
/// `walk_*` functions update this field when entering/exiting a scope.
pub struct TraverseScoping {
pub struct TraverseScoping<'a> {
scoping: Scoping,
uid_names: Option<FxHashSet<CompactStr>>,
uid_names: Option<FxHashSet<&'a str>>,
current_scope_id: ScopeId,
current_hoist_scope_id: ScopeId,
current_block_scope_id: ScopeId,
}

// Public methods
impl TraverseScoping {
impl<'a> TraverseScoping<'a> {
/// Get current scope ID
#[inline]
pub fn current_scope_id(&self) -> ScopeId {
Expand Down Expand Up @@ -257,7 +256,7 @@ impl TraverseScoping {
/// Generate binding.
///
/// Creates a symbol with the provided name and flags and adds it to the specified scope.
pub fn generate_binding<'a>(
pub fn generate_binding(
&mut self,
name: Atom<'a>,
scope_id: ScopeId,
Expand All @@ -270,7 +269,7 @@ impl TraverseScoping {
/// Generate binding in current scope.
///
/// Creates a symbol with the provided name and flags and adds it to the current scope.
pub fn generate_binding_in_current_scope<'a>(
pub fn generate_binding_in_current_scope(
&mut self,
name: Atom<'a>,
flags: SymbolFlags,
Expand Down Expand Up @@ -354,17 +353,17 @@ impl TraverseScoping {
/// i.e. if source contains identifiers `_foo` and `__bar`, create UIDs names `___0`, `___1`,
/// `___2` etc. They'll all be unique within the program.
#[expect(clippy::missing_panics_doc)]
pub fn generate_uid_name(&mut self, name: &str) -> CompactStr {
pub fn generate_uid_name(&mut self, name: &str, allocator: &'a Allocator) -> Atom<'a> {
// If `uid_names` is not already populated, initialize it
if self.uid_names.is_none() {
self.uid_names = Some(self.get_uid_names());
self.uid_names = Some(self.get_uid_names(allocator));
}
let uid_names = self.uid_names.as_mut().unwrap();

let base = get_uid_name_base(name);
let uid = get_unique_name(base, uid_names);
uid_names.insert(uid.clone());
uid
let uid = get_unique_name(base, uid_names, allocator).into_bump_str();
uid_names.insert(uid);
Atom::from(uid)
}

/// Create a reference bound to a `SymbolId`
Expand Down Expand Up @@ -449,7 +448,7 @@ impl TraverseScoping {
}

// Methods used internally within crate
impl TraverseScoping {
impl<'a> TraverseScoping<'a> {
/// Create new `TraverseScoping`
pub(super) fn new(scoping: Scoping) -> Self {
Self {
Expand Down Expand Up @@ -492,14 +491,14 @@ impl TraverseScoping {
///
/// Once this set is created, generating a UID is a relatively quick operation, rather than
/// iterating over all symbols and unresolved references every time generate a UID.
fn get_uid_names(&self) -> FxHashSet<CompactStr> {
fn get_uid_names(&self, allocator: &'a Allocator) -> FxHashSet<&'a str> {
self.scoping
.root_unresolved_references()
.keys()
.copied()
.chain(self.scoping.symbol_names())
.filter(|name| name.as_bytes().first() == Some(&b'_'))
.map(CompactStr::from)
.map(|str| allocator.alloc_str(str))
.collect()
}

Expand All @@ -526,8 +525,12 @@ fn get_uid_name_base(name: &str) -> &str {
unsafe { str::from_utf8_unchecked(bytes) }
}

fn get_unique_name(base: &str, uid_names: &FxHashSet<CompactStr>) -> CompactStr {
CompactStr::from(get_unique_name_impl(base, uid_names))
fn get_unique_name<'a>(
base: &str,
uid_names: &FxHashSet<&'a str>,
allocator: &'a Allocator,
) -> ArenaString<'a> {
get_unique_name_impl(base, uid_names, allocator)
}

// TODO: We could make this function more performant, especially when it checks a lot of names
Expand All @@ -538,17 +541,21 @@ fn get_unique_name(base: &str, uid_names: &FxHashSet<CompactStr>) -> CompactStr
// we could calculate an "unfinished" hash not including the last block, and then just add the final
// block to "finish" the hash on each iteration. With `FxHash` this would be straight line code and only
// a few operations.
fn get_unique_name_impl(base: &str, uid_names: &FxHashSet<CompactStr>) -> CompactString {
// Create `CompactString` prepending name with `_`, and with 1 byte excess capacity.
fn get_unique_name_impl<'a>(
base: &str,
uid_names: &FxHashSet<&'a str>,
allocator: &'a Allocator,
) -> ArenaString<'a> {
// Create `ArenaString` prepending name with `_`, and with 1 byte excess capacity.
// The extra byte is to avoid reallocation if need to add a digit on the end later,
// which will not be too uncommon.
// Having to add 2 digits will be uncommon, so we don't allocate 2 extra bytes for 2 digits.
let mut name = CompactString::with_capacity(base.len() + 2);
let mut name = ArenaString::with_capacity_in(base.len() + 2, allocator);
name.push('_');
name.push_str(base);

// It's fairly common that UIDs may need a numerical postfix, so we try to keep string
// operations to a minimum for postfixes up to 99 - reusing a single `CompactString`,
// operations to a minimum for postfixes up to 99 - reusing a single `ArenaString`,
// rather than generating a new string on each attempt.
// For speed we manipulate the string as bytes.
// Postfixes greater than 99 should be very uncommon, so don't bother optimizing.
Expand Down Expand Up @@ -625,9 +632,13 @@ fn get_unique_name_impl(base: &str, uid_names: &FxHashSet<CompactStr>) -> Compac
let mut buffer = ItoaBuffer::new();
for n in 100..=u32::MAX {
let digits = buffer.format(n);
/*
// SAFETY: `base_len` is always shorter than current `name.len()`, on a UTF-8 char boundary,
// and `name` contains at least `base_len` initialized bytes
unsafe { name.set_len(base_len) };
*/
// workaround for `set_len` does not exist in `ArenaString`
name.truncate(base_len);
name.push_str(digits);
if !uid_names.contains(name.as_str()) {
return name;
Expand Down Expand Up @@ -757,8 +768,9 @@ fn test_get_unique_name() {
),
];

let allocator = Allocator::default();
for (used, name, expected) in cases {
let used = used.iter().map(|name| CompactStr::from(*name)).collect();
assert_eq!(get_unique_name(name, &used), expected);
let used = used.iter().copied().collect::<FxHashSet<_>>();
assert_eq!(get_unique_name(name, &used, &allocator), *expected);
}
}
Loading