Skip to content
Closed
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: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/oxc_semantic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ oxc_index = { workspace = true }
oxc_span = { workspace = true }
oxc_syntax = { workspace = true }

fixedbitset = { workspace = true }
itertools = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
Expand Down
32 changes: 30 additions & 2 deletions crates/oxc_semantic/src/node.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::iter::FusedIterator;

use fixedbitset::FixedBitSet;
use oxc_allocator::{Address, GetAddress};
use oxc_ast::{AstKind, ast::Program};
use oxc_ast::{AstKind, ast::Program, ast_kind::AST_TYPE_MAX};
use oxc_cfg::BlockNodeId;
use oxc_index::{IndexSlice, IndexVec};
use oxc_span::{GetSpan, Span};
Expand Down Expand Up @@ -96,14 +97,29 @@ impl GetAddress for AstNode<'_> {
}

/// Untyped AST nodes flattened into an vec
#[derive(Debug, Default)]
#[derive(Debug)]
pub struct AstNodes<'a> {
program: Option<&'a Program<'a>>,
nodes: IndexVec<NodeId, AstNode<'a>>,
/// Stores a set of bits of a fixed size, where each bit represents a single [`AstKind`]. If the bit is set (1),
/// then the AST contains at least one node of that kind. If the bit is not set (0), then the AST does not contain
/// any nodes of that kind.
node_kinds_set: FixedBitSet,
/// `node` -> `parent`
parent_ids: IndexVec<NodeId, NodeId>,
}

impl Default for AstNodes<'_> {
fn default() -> Self {
Self {
program: None,
nodes: IndexVec::new(),
node_kinds_set: FixedBitSet::with_capacity(AST_TYPE_MAX as usize + 1),
parent_ids: IndexVec::new(),
}
}
}

impl<'a> AstNodes<'a> {
/// Iterate over all [`AstNode`]s in this AST.
pub fn iter(&self) -> impl Iterator<Item = &AstNode<'a>> + '_ {
Expand Down Expand Up @@ -210,6 +226,12 @@ impl<'a> AstNodes<'a> {
let node_id = self.parent_ids.push(parent_node_id);
let node = AstNode::new(kind, scope_id, cfg_id, flags, node_id);
self.nodes.push(node);
debug_assert!((kind.ty() as usize) < self.node_kinds_set.len());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bounds check should be updated to use <= instead of < since the FixedBitSet was created with capacity AST_TYPE_MAX + 1, making AST_TYPE_MAX a valid index. Consider changing to:

debug_assert!((kind.ty() as usize) <= AST_TYPE_MAX as usize);

This ensures the assertion correctly validates that the index is within the allocated capacity.

Suggested change
debug_assert!((kind.ty() as usize) < self.node_kinds_set.len());
debug_assert!((kind.ty() as usize) <= AST_TYPE_MAX as usize);

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

// SAFETY: `AstKind` maps exactly to `AstType`, and there should be exactly
// enough bits to insert it into `node_kinds`, so we can skip a bounds check here.
unsafe {
self.node_kinds_set.insert_unchecked(kind.ty() as usize);
}
node_id
}

Expand All @@ -233,6 +255,12 @@ impl<'a> AstNodes<'a> {
let node_id = self.parent_ids.push(NodeId::ROOT);
let node = AstNode::new(kind, scope_id, cfg_id, flags, node_id);
self.nodes.push(node);
debug_assert!((kind.ty() as usize) < self.node_kinds_set.len());
// SAFETY: `AstKind` maps exactly to `AstType`, and there should be exactly
// enough bits to insert it into `node_kinds`, so we can skip a bounds check here.
unsafe {
self.node_kinds_set.insert_unchecked(kind.ty() as usize);
}
node_id
}

Expand Down
Loading