Skip to content

Commit 7fcff1a

Browse files
committed
refactor(semantic): move AstNode::cfg_id to struct of arrays in AstNodes
part of #13646 Reduces AstNode from 32 to 24 bytes in the linter pipeline.
1 parent 5ba765c commit 7fcff1a

File tree

9 files changed

+98
-92
lines changed

9 files changed

+98
-92
lines changed

crates/oxc_linter/src/rules/eslint/getter_return.rs

Lines changed: 62 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -234,72 +234,73 @@ impl GetterReturn {
234234
break 'returns true;
235235
}
236236
}
237-
let output = set_depth_first_search(graph, Some(node.cfg_id()), |event| {
238-
match event {
239-
// We only need to check paths that are normal or jump.
240-
DfsEvent::TreeEdge(a, b) => {
241-
let edges = graph.edges_connecting(a, b).collect::<Vec<_>>();
242-
if edges.iter().any(|e| {
243-
matches!(
244-
e.weight(),
245-
EdgeType::Normal
246-
| EdgeType::Jump
247-
| EdgeType::Error(ErrorEdgeKind::Explicit)
248-
)
249-
}) {
250-
Control::Continue
251-
} else {
252-
Control::Prune
237+
let output =
238+
set_depth_first_search(graph, Some(ctx.nodes().cfg_id(node.id())), |event| {
239+
match event {
240+
// We only need to check paths that are normal or jump.
241+
DfsEvent::TreeEdge(a, b) => {
242+
let edges = graph.edges_connecting(a, b).collect::<Vec<_>>();
243+
if edges.iter().any(|e| {
244+
matches!(
245+
e.weight(),
246+
EdgeType::Normal
247+
| EdgeType::Jump
248+
| EdgeType::Error(ErrorEdgeKind::Explicit)
249+
)
250+
}) {
251+
Control::Continue
252+
} else {
253+
Control::Prune
254+
}
253255
}
254-
}
255-
DfsEvent::Discover(basic_block_id, _) => {
256-
let return_instruction =
257-
cfg.basic_block(basic_block_id).instructions().iter().find(|it| {
258-
match it.kind {
259-
// Throws are classified as returning.
260-
InstructionKind::Return(_) | InstructionKind::Throw => true,
261-
262-
// Ignore irrelevant elements.
263-
InstructionKind::ImplicitReturn
264-
| InstructionKind::Break(_)
265-
| InstructionKind::Continue(_)
266-
| InstructionKind::Iteration(_)
267-
| InstructionKind::Unreachable
268-
| InstructionKind::Condition
269-
| InstructionKind::Statement => false,
270-
}
256+
DfsEvent::Discover(basic_block_id, _) => {
257+
let return_instruction =
258+
cfg.basic_block(basic_block_id).instructions().iter().find(|it| {
259+
match it.kind {
260+
// Throws are classified as returning.
261+
InstructionKind::Return(_) | InstructionKind::Throw => true,
262+
263+
// Ignore irrelevant elements.
264+
InstructionKind::ImplicitReturn
265+
| InstructionKind::Break(_)
266+
| InstructionKind::Continue(_)
267+
| InstructionKind::Iteration(_)
268+
| InstructionKind::Unreachable
269+
| InstructionKind::Condition
270+
| InstructionKind::Statement => false,
271+
}
272+
});
273+
274+
let does_return = return_instruction.is_some_and(|ret| {
275+
!matches!( ret.kind,
276+
InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined)
277+
if !self.allow_implicit
278+
)
271279
});
272280

273-
let does_return = return_instruction.is_some_and(|ret| {
274-
!matches!( ret.kind,
275-
InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined)
276-
if !self.allow_implicit
277-
)
278-
});
279-
280-
// Return true as the second argument to signify we should
281-
// continue walking this branch, as we haven't seen anything
282-
// that will signify to us that this path of the program will
283-
// definitely return or throw.
284-
if graph.edges_directed(basic_block_id, Direction::Outgoing).any(|e| {
285-
matches!(
286-
e.weight(),
287-
EdgeType::Jump
288-
| EdgeType::Normal
289-
| EdgeType::Backedge
290-
| EdgeType::Error(ErrorEdgeKind::Explicit)
291-
)
292-
}) {
293-
Control::Continue
294-
} else if does_return {
295-
Control::Prune
296-
} else {
297-
Control::Break(())
281+
// Return true as the second argument to signify we should
282+
// continue walking this branch, as we haven't seen anything
283+
// that will signify to us that this path of the program will
284+
// definitely return or throw.
285+
if graph.edges_directed(basic_block_id, Direction::Outgoing).any(|e| {
286+
matches!(
287+
e.weight(),
288+
EdgeType::Jump
289+
| EdgeType::Normal
290+
| EdgeType::Backedge
291+
| EdgeType::Error(ErrorEdgeKind::Explicit)
292+
)
293+
}) {
294+
Control::Continue
295+
} else if does_return {
296+
Control::Prune
297+
} else {
298+
Control::Break(())
299+
}
298300
}
301+
_ => Control::Continue,
299302
}
300-
_ => Control::Continue,
301-
}
302-
});
303+
});
303304

304305
output.break_value().is_none()
305306
};

crates/oxc_linter/src/rules/eslint/no_fallthrough.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ impl Rule for NoFallthrough {
260260
let AstKind::SwitchStatement(switch) = node.kind() else { return };
261261

262262
let cfg = ctx.cfg();
263-
let switch_id = node.cfg_id();
263+
let switch_id = ctx.nodes().cfg_id(node.id());
264264
let graph = cfg.graph();
265265

266266
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch);
@@ -438,7 +438,7 @@ fn get_switch_semantic_cases(
438438
let graph = cfg.graph();
439439
let has_default = switch.cases.iter().any(SwitchCase::is_default_case);
440440
let (mut cfg_ids, tests, exit) = graph
441-
.edges_directed(node.cfg_id(), Direction::Outgoing)
441+
.edges_directed(ctx.nodes().cfg_id(node.id()), Direction::Outgoing)
442442
.fold((Vec::new(), Vec::new(), None), |(mut cfg_ids, mut conds, exit), it| {
443443
let target = it.target();
444444
if !matches!(it.weight(), EdgeType::Normal) {

crates/oxc_linter/src/rules/eslint/no_this_before_super.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl Rule for NoThisBeforeSuper {
7575
}
7676
}
7777
AstKind::Super(_) => {
78-
let basic_block_id = node.cfg_id();
78+
let basic_block_id = ctx.nodes().cfg_id(node.id());
7979
if let AstKind::CallExpression(call_expr) = ctx.nodes().parent_kind(node.id()) {
8080
let has_this_or_super_in_args =
8181
Self::contains_this_or_super_in_args(&call_expr.arguments);
@@ -92,7 +92,7 @@ impl Rule for NoThisBeforeSuper {
9292
}
9393
}
9494
AstKind::ThisExpression(_) => {
95-
let basic_block_id = node.cfg_id();
95+
let basic_block_id = ctx.nodes().cfg_id(node.id());
9696
if !basic_blocks_with_super_called.contains(&basic_block_id) {
9797
basic_blocks_with_local_violations
9898
.entry(basic_block_id)
@@ -109,7 +109,7 @@ impl Rule for NoThisBeforeSuper {
109109
for node in wanted_nodes {
110110
let output = Self::analyze(
111111
cfg,
112-
node.cfg_id(),
112+
ctx.nodes().cfg_id(node.id()),
113113
&basic_blocks_with_super_called,
114114
&basic_blocks_with_local_violations,
115115
false,

crates/oxc_linter/src/rules/eslint/no_unreachable.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,13 @@ impl Rule for NoUnreachable {
6868
let mut infinite_loops = Vec::new();
6969

7070
// Set the root as reachable.
71-
unreachables[root.cfg_id().index()] = false;
71+
let root_cfg_id = ctx.nodes().cfg_id(root.id());
72+
unreachables[root_cfg_id.index()] = false;
7273

7374
// In our first path we first check if each block is definitely unreachable, If it is then
7475
// we set it as such, If we encounter an infinite loop we keep its end block since it can
7576
// prevent other reachable blocks from ever getting executed.
76-
let _: Control<()> = depth_first_search(graph, Some(root.cfg_id()), |event| {
77+
let _: Control<()> = depth_first_search(graph, Some(root_cfg_id), |event| {
7778
if let DfsEvent::Finish(node, _) = event {
7879
let unreachable = cfg.basic_block(node).is_unreachable();
7980
unreachables[node.index()] = unreachable;
@@ -168,7 +169,7 @@ impl Rule for NoUnreachable {
168169
continue;
169170
}
170171

171-
if unreachables[node.cfg_id().index()] {
172+
if unreachables[ctx.nodes().cfg_id(node.id()).index()] {
172173
ctx.diagnostic(no_unreachable_diagnostic(node.kind().span()));
173174
}
174175
}

crates/oxc_linter/src/rules/promise/always_return.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ fn is_nodejs_terminal_statement(node: &AstNode) -> bool {
332332
fn has_no_return_code_path(node: &AstNode, ctx: &LintContext) -> bool {
333333
let cfg = ctx.cfg();
334334
let graph = cfg.graph();
335-
let output = set_depth_first_search(graph, Some(node.cfg_id()), |event| {
335+
let output = set_depth_first_search(graph, Some(ctx.nodes().cfg_id(node.id())), |event| {
336336
match event {
337337
// We only need to check paths that are normal or jump.
338338
DfsEvent::TreeEdge(a, b) => {

crates/oxc_linter/src/rules/react/require_render_return.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ fn contains_return_statement(node: &AstNode, ctx: &LintContext) -> bool {
116116
let cfg = ctx.cfg();
117117
let state = neighbors_filtered_by_edge_weight(
118118
cfg.graph(),
119-
node.cfg_id(),
119+
ctx.nodes().cfg_id(node.id()),
120120
&|edge| match edge {
121121
// We only care about normal edges having a return statement.
122122
EdgeType::Jump | EdgeType::Normal => None,

crates/oxc_linter/src/rules/react/rules_of_hooks.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,8 @@ impl Rule for RulesOfHooks {
275275
return;
276276
}
277277

278-
let node_cfg_id = node.cfg_id();
279-
let func_cfg_id = parent_func.cfg_id();
278+
let node_cfg_id = ctx.nodes().cfg_id(node.id());
279+
let func_cfg_id = ctx.nodes().cfg_id(parent_func.id());
280280

281281
// there is no branch between us and our parent function
282282
if node_cfg_id == func_cfg_id {
@@ -296,20 +296,21 @@ impl Rule for RulesOfHooks {
296296
return ctx.diagnostic(diagnostics::loop_hook(span, hook_name));
297297
}
298298

299-
if has_conditional_path_accept_throw(cfg, parent_func, node) {
299+
if has_conditional_path_accept_throw(ctx.nodes(), cfg, parent_func, node) {
300300
#[expect(clippy::needless_return)]
301301
return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name));
302302
}
303303
}
304304
}
305305

306306
fn has_conditional_path_accept_throw(
307+
nodes: &AstNodes<'_>,
307308
cfg: &ControlFlowGraph,
308309
from: &AstNode<'_>,
309310
to: &AstNode<'_>,
310311
) -> bool {
311-
let from_graph_id = from.cfg_id();
312-
let to_graph_id = to.cfg_id();
312+
let from_graph_id = nodes.cfg_id(from.id());
313+
let to_graph_id = nodes.cfg_id(to.id());
313314
let graph = cfg.graph();
314315
if graph
315316
.edges(to_graph_id)

crates/oxc_semantic/examples/cfg.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ fn main() -> std::io::Result<()> {
9696

9797
let mut ast_nodes_by_block = FxHashMap::<_, Vec<_>>::default();
9898
for node in semantic.semantic.nodes() {
99-
let block = node.cfg_id();
99+
let block = semantic.semantic.nodes().cfg_id(node.id());
100100
let block_ix = cfg.graph.node_weight(block).unwrap();
101101
ast_nodes_by_block.entry(*block_ix).or_default().push(node);
102102
}

crates/oxc_semantic/src/node.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ pub struct AstNode<'a> {
2222

2323
/// Associated Scope (initialized by binding)
2424
scope_id: ScopeId,
25-
26-
/// Associated `BasicBlockId` in CFG (initialized by control_flow)
27-
#[cfg(feature = "cfg")]
28-
cfg_id: BlockNodeId,
2925
}
3026

3127
impl<'a> AstNode<'a> {
@@ -34,10 +30,10 @@ impl<'a> AstNode<'a> {
3430
pub(crate) fn new(
3531
kind: AstKind<'a>,
3632
scope_id: ScopeId,
37-
cfg_id: BlockNodeId,
33+
_cfg_id: BlockNodeId,
3834
id: NodeId,
3935
) -> Self {
40-
Self { id, kind, scope_id, cfg_id }
36+
Self { id, kind, scope_id }
4137
}
4238

4339
#[cfg(not(feature = "cfg"))]
@@ -51,15 +47,6 @@ impl<'a> AstNode<'a> {
5147
self.id
5248
}
5349

54-
/// ID of the control flow graph node this node is in.
55-
///
56-
/// See [oxc_cfg::ControlFlowGraph] for more information.
57-
#[inline]
58-
#[cfg(feature = "cfg")]
59-
pub fn cfg_id(&self) -> BlockNodeId {
60-
self.cfg_id
61-
}
62-
6350
/// Access the underlying struct from [`oxc_ast`].
6451
#[inline]
6552
pub fn kind(&self) -> AstKind<'a> {
@@ -99,6 +86,9 @@ pub struct AstNodes<'a> {
9986
parent_ids: IndexVec<NodeId, NodeId>,
10087
/// `node` -> `flags`
10188
flags: IndexVec<NodeId, NodeFlags>,
89+
/// `node` -> `cfg_id` (control flow graph node)
90+
#[cfg(feature = "cfg")]
91+
cfg_ids: IndexVec<NodeId, BlockNodeId>,
10292
/// Stores a set of bits of a fixed size, where each bit represents a single [`AstKind`]. If the bit is set (1),
10393
/// then the AST contains at least one node of that kind. If the bit is not set (0), then the AST does not contain
10494
/// any nodes of that kind.
@@ -197,6 +187,15 @@ impl<'a> AstNodes<'a> {
197187
&mut self.flags[node_id]
198188
}
199189

190+
/// ID of the control flow graph node this node is in.
191+
///
192+
/// See [oxc_cfg::ControlFlowGraph] for more information.
193+
#[inline]
194+
#[cfg(feature = "cfg")]
195+
pub fn cfg_id(&self, node_id: NodeId) -> BlockNodeId {
196+
self.cfg_ids[node_id]
197+
}
198+
200199
/// Get the [`Program`] that's also the root of the AST.
201200
#[inline]
202201
pub fn program(&self) -> &'a Program<'a> {
@@ -228,6 +227,7 @@ impl<'a> AstNodes<'a> {
228227
let node = AstNode::new(kind, scope_id, cfg_id, node_id);
229228
self.nodes.push(node);
230229
self.flags.push(flags);
230+
self.cfg_ids.push(cfg_id);
231231
self.node_kinds_set.set(kind.ty());
232232
node_id
233233
}
@@ -271,6 +271,7 @@ impl<'a> AstNodes<'a> {
271271
self.parent_ids.push(NodeId::ROOT);
272272
self.nodes.push(AstNode::new(kind, scope_id, cfg_id, NodeId::ROOT));
273273
self.flags.push(flags);
274+
self.cfg_ids.push(cfg_id);
274275
self.node_kinds_set.set(AstType::Program);
275276
NodeId::ROOT
276277
}
@@ -300,6 +301,8 @@ impl<'a> AstNodes<'a> {
300301
self.nodes.reserve(additional);
301302
self.parent_ids.reserve(additional);
302303
self.flags.reserve(additional);
304+
#[cfg(feature = "cfg")]
305+
self.cfg_ids.reserve(additional);
303306
}
304307

305308
/// Checks if the AST contains any nodes of the given types.

0 commit comments

Comments
 (0)