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

refactor(semantic): remove ScopeTree::child_ids #5232

Merged
merged 1 commit into from
Aug 27, 2024
Merged

refactor(semantic): remove ScopeTree::child_ids #5232

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
30 changes: 0 additions & 30 deletions crates/oxc_semantic/src/post_transform_checker.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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