Skip to content

Commit

Permalink
refactor(semantic): remove ScopeTree::child_ids (#5232)
Browse files Browse the repository at this point in the history
closes #5244
  • Loading branch information
Boshen committed Aug 27, 2024
1 parent 7c4f009 commit a17cf33
Show file tree
Hide file tree
Showing 13 changed files with 218 additions and 7,741 deletions.
30 changes: 0 additions & 30 deletions crates/oxc_semantic/src/post_transform_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,6 @@ impl<'s> PostTransformChecker<'s> {
self.errors.push_mismatch("Scope parent mismatch", scope_ids, parent_ids);
}

// Check children match
let child_ids = self
.get_pair(scope_ids, |data, scope_id| data.scopes.get_child_ids(scope_id).to_vec());
if self.remap_scope_ids_sets(&child_ids).is_mismatch() {
self.errors.push_mismatch("Scope children mismatch", scope_ids, child_ids);
}

// NB: Skip checking node IDs match - transformer does not set `AstNodeId`s
}
}
Expand Down Expand Up @@ -592,29 +585,6 @@ impl<'s> PostTransformChecker<'s> {
Pair::new(self.scope_ids_map.get(scope_ids.after_transform), Some(scope_ids.rebuilt))
}

/// Remap pair of arrays of `ScopeId`s.
/// Map `after_transform` IDs to `rebuilt` IDs.
/// Sort both sets.
fn remap_scope_ids_sets<V: AsRef<Vec<ScopeId>>>(
&self,
scope_ids: &Pair<V>,
) -> Pair<Vec<Option<ScopeId>>> {
let mut after_transform = scope_ids
.after_transform
.as_ref()
.iter()
.map(|&scope_id| self.scope_ids_map.get(scope_id))
.collect::<Vec<_>>();
let mut rebuilt =
scope_ids.rebuilt.as_ref().iter().copied().map(Option::Some).collect::<Vec<_>>();

after_transform.sort_unstable();
rebuilt.sort_unstable();

Pair::new(after_transform, rebuilt)
}

/// Remap pair of arrays of `SymbolId`s.
/// Map `after_transform` IDs to `rebuilt` IDs.
/// Sort both sets.
fn remap_symbol_ids_sets<V: AsRef<Vec<SymbolId>>>(
Expand Down
54 changes: 1 addition & 53 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<ReferenceId>>;
pub struct ScopeTree {
/// Maps a scope to the parent scope it belongs in.
parent_ids: IndexVec<ScopeId, Option<ScopeId>>,
/// Maps a scope to direct children scopes.
child_ids: IndexVec<ScopeId, Vec<ScopeId>>,
/// Maps a scope to its node id.
node_ids: IndexVec<ScopeId, AstNodeId>,
flags: IndexVec<ScopeId, ScopeFlags>,
Expand Down Expand Up @@ -67,46 +65,6 @@ impl ScopeTree {
std::iter::successors(Some(scope_id), |scope_id| self.parent_ids[*scope_id])
}

/// Iterate over scopes contained by a scope in breadth-first order.
///
/// Unlike [`ancestors`], this iterator will not include the scope itself.
///
/// [`ancestors`]: ScopeTree::ancestors
pub fn descendants(&self, scope_id: ScopeId) -> impl Iterator<Item = ScopeId> + '_ {
// Has to be a `fn` and pass arguments because we can't
// have recursive closures
fn add_to_list(
parent_id: ScopeId,
child_ids: &IndexVec<ScopeId, Vec<ScopeId>>,
items: &mut Vec<ScopeId>,
) {
if let Some(children) = child_ids.get(parent_id) {
for child_id in children {
items.push(*child_id);
add_to_list(*child_id, child_ids, items);
}
}
}

let mut list = vec![];

add_to_list(scope_id, &self.child_ids, &mut list);

list.into_iter()
}

/// Get the child scopes of a scope
#[inline]
pub fn get_child_ids(&self, scope_id: ScopeId) -> &[ScopeId] {
&self.child_ids[scope_id]
}

/// Get a mutable reference to a scope's children
#[inline]
pub fn get_child_ids_mut(&mut self, scope_id: ScopeId) -> &mut Vec<ScopeId> {
&mut self.child_ids[scope_id]
}

pub fn descendants_from_root(&self) -> impl Iterator<Item = ScopeId> + '_ {
self.parent_ids.iter_enumerated().map(|(scope_id, _)| scope_id)
}
Expand Down Expand Up @@ -173,9 +131,6 @@ impl ScopeTree {

pub fn set_parent_id(&mut self, scope_id: ScopeId, parent_id: Option<ScopeId>) {
self.parent_ids[scope_id] = parent_id;
if let Some(parent_id) = parent_id {
self.child_ids[parent_id].push(scope_id);
}
}

/// Get a variable binding by name that was declared in the top-level scope
Expand Down Expand Up @@ -266,12 +221,7 @@ impl ScopeTree {
node_id: AstNodeId,
flags: ScopeFlags,
) -> ScopeId {
let scope_id = self.add_scope_impl(Some(parent_id), node_id, flags);

// Set this scope as child of parent scope
self.child_ids[parent_id].push(scope_id);

scope_id
self.add_scope_impl(Some(parent_id), node_id, flags)
}

/// Create the root [`Program`] scope.
Expand All @@ -295,7 +245,6 @@ impl ScopeTree {
flags: ScopeFlags,
) -> ScopeId {
let scope_id = self.parent_ids.push(parent_id);
self.child_ids.push(vec![]);
self.flags.push(flags);
self.bindings.push(Bindings::default());
self.node_ids.push(node_id);
Expand All @@ -317,7 +266,6 @@ impl ScopeTree {
/// Reserve memory for an `additional` number of scopes.
pub fn reserve(&mut self, additional: usize) {
self.parent_ids.reserve(additional);
self.child_ids.reserve(additional);
self.flags.reserve(additional);
self.bindings.reserve(additional);
self.node_ids.reserve(additional);
Expand Down
4 changes: 0 additions & 4 deletions crates/oxc_semantic/tests/integration/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ fn test_only_program() {
// ancestors
assert_eq!(scopes.ancestors(root).count(), 1);
assert!(scopes.get_parent_id(root).is_none());

// children
assert_eq!(scopes.descendants(root).count(), 0);
assert!(scopes.get_child_ids(root).is_empty());
}

#[test]
Expand Down
15 changes: 11 additions & 4 deletions crates/oxc_semantic/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,23 @@ use oxc_semantic::{ScopeId, Semantic, SemanticBuilder};
use oxc_span::SourceType;

fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>) -> String {
let scope_tree = semantic.scopes();
let mut result = String::default();

result.push('[');
scopes.enumerate().for_each(|(index, scope_id)| {
if index != 0 {
result.push(',');
}
let flags = semantic.scopes().get_flags(scope_id);
let flags = scope_tree.get_flags(scope_id);
result.push('{');
let child_ids = semantic.scopes().get_child_ids(scope_id);
let child_ids = semantic
.scopes()
.descendants_from_root()
.filter(|id| {
scope_tree.get_parent_id(*id).is_some_and(|parent_id| parent_id == scope_id)
})
.collect::<Vec<_>>();
result.push_str("\"children\":");
result.push_str(&get_scope_snapshot(semantic, child_ids.iter().copied()));
result.push(',');
Expand All @@ -25,12 +32,12 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
result.push_str(
format!(
"\"node\": {:?},",
semantic.nodes().kind(semantic.scopes().get_node_id(scope_id)).debug_name()
semantic.nodes().kind(scope_tree.get_node_id(scope_id)).debug_name()
)
.as_str(),
);
result.push_str("\"symbols\": ");
let bindings = semantic.scopes().get_bindings(scope_id);
let bindings = scope_tree.get_bindings(scope_id);
result.push('[');
bindings.iter().enumerate().for_each(|(index, (name, symbol_id))| {
if index != 0 {
Expand Down
4 changes: 0 additions & 4 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,6 @@ impl TraverseScoping {
}

fn insert_scope_below(&mut self, child_scope_ids: &[ScopeId], flags: ScopeFlags) -> ScopeId {
// Remove these scopes from parent's children
let current_child_scope_ids = self.scopes.get_child_ids_mut(self.current_scope_id);
current_child_scope_ids.retain(|scope_id| !child_scope_ids.contains(scope_id));

// Create new scope as child of parent
let new_scope_id = self.create_child_scope_of_current(flags);

Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ oxc_linter = { workspace = true }
oxc_prettier = { workspace = true }
serde = { workspace = true }

wasm-bindgen = { workspace = true }
serde-wasm-bindgen = { workspace = true }
tsify = { workspace = true }
wasm-bindgen = { workspace = true }
serde-wasm-bindgen = { workspace = true }
tsify = { workspace = true }
console_error_panic_hook = "0.1.7"
16 changes: 12 additions & 4 deletions crates/oxc_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,21 @@ impl Oxc {
) {
let space = " ".repeat(depth * 2);

let scope_tree = semantic.scopes();
for scope_id in scope_ids {
let flags = semantic.scopes().get_flags(*scope_id);
let next_scope_ids = semantic.scopes().get_child_ids(*scope_id);
let flags = scope_tree.get_flags(*scope_id);
let child_scope_ids = scope_tree
.descendants_from_root()
.filter(|id| {
scope_tree
.get_parent_id(*id)
.is_some_and(|parent_id| parent_id == *scope_id)
})
.collect::<Vec<_>>();

scope_text
.push_str(&format!("{space}Scope{:?} ({flags:?}) {{\n", scope_id.index() + 1));
let bindings = semantic.scopes().get_bindings(*scope_id);
let bindings = scope_tree.get_bindings(*scope_id);
let binding_space = " ".repeat((depth + 1) * 2);
if !bindings.is_empty() {
scope_text.push_str(&format!("{binding_space}Bindings: {{"));
Expand All @@ -335,7 +343,7 @@ impl Oxc {
scope_text.push_str(&format!("\n{binding_space}}}\n"));
}

write_scope_text(semantic, scope_text, depth + 1, next_scope_ids);
write_scope_text(semantic, scope_text, depth + 1, &child_scope_ids);
scope_text.push_str(&format!("{space}}}\n"));
}
}
Expand Down
Loading

0 comments on commit a17cf33

Please sign in to comment.