Skip to content
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

Jump to bottom

refactor(semantic): dereference IDs as quickly as possible #6303

Merged
merged 1 commit into from
Oct 6, 2024
Merged

refactor(semantic): dereference IDs as quickly as possible #6303

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
10 changes: 5 additions & 5 deletions crates/oxc_semantic/src/binder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ impl<'a> Binder<'a> for VariableDeclarator<'a> {
let name = &ident.name;
let mut declared_symbol_id = None;

for scope_id in &var_scope_ids {
for &scope_id in &var_scope_ids {
if let Some(symbol_id) =
builder.check_redeclaration(*scope_id, span, name, excludes, true)
builder.check_redeclaration(scope_id, span, name, excludes, true)
{
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;
Expand All @@ -92,10 +92,10 @@ impl<'a> Binder<'a> for VariableDeclarator<'a> {

// Finally, add the variable to all hoisted scopes
// to support redeclaration checks when declaring variables with the same name later.
for scope_id in &var_scope_ids {
for &scope_id in &var_scope_ids {
builder
.hoisting_variables
.entry(*scope_id)
.entry(scope_id)
.or_default()
.insert(name.clone(), symbol_id);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2025,8 +2025,8 @@ impl<'a> SemanticBuilder<'a> {
}

fn make_all_namespaces_valuelike(&mut self) {
for symbol_id in &self.namespace_stack {
let Some(symbol_id) = *symbol_id else {
for &symbol_id in &self.namespace_stack {
let Some(symbol_id) = symbol_id else {
continue;
};

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ impl<'a> AstNodes<'a> {
/// [`Program`]: oxc_ast::ast::Program
pub fn ancestors(&self, ast_node_id: NodeId) -> impl Iterator<Item = NodeId> + '_ {
let parent_ids = &self.parent_ids;
std::iter::successors(Some(ast_node_id), |node_id| parent_ids[*node_id])
std::iter::successors(Some(ast_node_id), |&node_id| parent_ids[node_id])
}

/// Create and add an [`AstNode`] to the [`AstNodes`] tree and get its [`NodeId`].
Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_semantic/src/scope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl ScopeTree {
/// The first element of this iterator will be the scope itself. This
/// guarantees the iterator will have at least 1 element.
pub fn ancestors(&self, scope_id: ScopeId) -> impl Iterator<Item = ScopeId> + '_ {
std::iter::successors(Some(scope_id), |scope_id| self.parent_ids[*scope_id])
std::iter::successors(Some(scope_id), |&scope_id| self.parent_ids[scope_id])
}

pub fn descendants_from_root(&self) -> impl Iterator<Item = ScopeId> + '_ {
Expand Down Expand Up @@ -206,8 +206,8 @@ impl ScopeTree {
/// found. If no binding is found, [`None`] is returned.
pub fn find_binding(&self, scope_id: ScopeId, name: &str) -> Option<SymbolId> {
for scope_id in self.ancestors(scope_id) {
if let Some(symbol_id) = self.bindings[scope_id].get(name) {
return Some(*symbol_id);
if let Some(&symbol_id) = self.bindings[scope_id].get(name) {
return Some(symbol_id);
}
}
None
Expand All @@ -234,7 +234,7 @@ impl ScopeTree {
/// [`iter_bindings_in`]: ScopeTree::iter_bindings_in
pub fn iter_bindings(&self) -> impl Iterator<Item = (ScopeId, SymbolId, &'_ CompactStr)> + '_ {
self.bindings.iter_enumerated().flat_map(|(scope_id, bindings)| {
bindings.iter().map(move |(name, symbol_id)| (scope_id, *symbol_id, name))
bindings.iter().map(move |(name, &symbol_id)| (scope_id, symbol_id, name))
})
}

Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_semantic/src/symbol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ impl SymbolTable {
/// let classes = semantic
/// .scopes()
/// .symbol_ids()
/// .filter(|symbol_id| {
/// let flags = semantic.symbols().get_flags(*symbol_id);
/// .filter(|&symbol_id| {
/// let flags = semantic.symbols().get_flags(symbol_id);
/// flags.is_class()
/// })
/// .collect::<Vec<_>>();
Expand Down Expand Up @@ -238,7 +238,7 @@ impl SymbolTable {
) -> impl DoubleEndedIterator<Item = &Reference> + '_ {
self.resolved_references[symbol_id]
.iter()
.map(|reference_id| &self.references[*reference_id])
.map(|&reference_id| &self.references[reference_id])
}

/// Delete a reference to a symbol.
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/tests/integration/util/class_tester.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pub struct ClassTester<'a> {

impl<'a> ClassTester<'a> {
pub(super) fn has_class(semantic: Semantic<'a>, name: &str) -> Self {
let class_id = semantic.classes().iter_enumerated().find_map(|(class_id, ast_node_id)| {
let kind = semantic.nodes().kind(*ast_node_id);
let class_id = semantic.classes().iter_enumerated().find_map(|(class_id, &ast_node_id)| {
let kind = semantic.nodes().kind(ast_node_id);
let class = kind.as_class()?;

if class.id.clone().is_some_and(|id| id.name == name) {
Expand Down
10 changes: 4 additions & 6 deletions crates/oxc_semantic/tests/integration/util/symbol_tester.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'a> SymbolTester<'a> {
0 => Err(OxcDiagnostic::error(format!("Could not find declaration for {target}"))),
1 => Ok(symbols_with_target_name
.iter()
.map(|(_, symbol_id, _)| *symbol_id)
.map(|&(_, symbol_id, _)| symbol_id)
.next()
.unwrap()),
n if n > 1 => Err(OxcDiagnostic::error(format!(
Expand Down Expand Up @@ -183,11 +183,9 @@ impl<'a> SymbolTester<'a> {
self.test_result = match self.test_result {
Ok(symbol_id) => {
let refs = {
self.semantic
.symbols()
.get_resolved_reference_ids(symbol_id)
.iter()
.map(|r_id| self.semantic.symbols().get_reference(*r_id).clone())
self.semantic.symbols().get_resolved_reference_ids(symbol_id).iter().map(
|&reference_id| self.semantic.symbols().get_reference(reference_id).clone(),
)
};
let num_accepted = refs.filter(filter).count();
if num_accepted == ref_count {
Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_semantic/tests/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
result.push_str("\"symbols\": ");
let bindings = scope_tree.get_bindings(scope_id);
result.push('[');
bindings.iter().enumerate().for_each(|(index, (name, symbol_id))| {
bindings.iter().enumerate().for_each(|(index, (name, &symbol_id))| {
if index != 0 {
result.push(',');
}
result.push('{');
result.push_str(
format!("\"flags\": \"{:?}\",", semantic.symbols().get_flags(*symbol_id)).as_str(),
format!("\"flags\": \"{:?}\",", semantic.symbols().get_flags(symbol_id)).as_str(),
);
result.push_str(format!("\"id\": {},", symbol_id.index()).as_str());
result.push_str(format!("\"name\": {name:?},").as_str());
Expand All @@ -63,7 +63,7 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
"\"node\": {:?},",
semantic
.nodes()
.kind(semantic.symbols().get_declaration(*symbol_id))
.kind(semantic.symbols().get_declaration(symbol_id))
.debug_name()
)
.as_str(),
Expand All @@ -73,14 +73,14 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
result.push('[');
semantic
.symbols()
.get_resolved_reference_ids(*symbol_id)
.get_resolved_reference_ids(symbol_id)
.iter()
.enumerate()
.for_each(|(index, reference_id)| {
.for_each(|(index, &reference_id)| {
if index != 0 {
result.push(',');
}
let reference = &semantic.symbols().references[*reference_id];
let reference = &semantic.symbols().references[reference_id];
result.push('{');
result
.push_str(format!("\"flags\": \"{:?}\",", reference.flags()).as_str());
Expand Down