mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 04:08:41 +00:00
refactor(semantic/cfg): alias petgraph's NodeIndex as BasicBlockId. (#3380)
Hides petgraph's general `NodeIndex` type behind `BasicBlockId`.
This commit is contained in:
parent
40ab95b055
commit
78e6326e48
10 changed files with 77 additions and 74 deletions
|
|
@ -182,7 +182,7 @@ impl GetterReturn {
|
|||
|
||||
let output = neighbors_filtered_by_edge_weight(
|
||||
&cfg.graph,
|
||||
node.cfg_ix(),
|
||||
node.cfg_id(),
|
||||
&|edge| match edge {
|
||||
EdgeType::Normal => None,
|
||||
// We don't need to handle backedges because we would have already visited
|
||||
|
|
@ -229,7 +229,7 @@ impl GetterReturn {
|
|||
}
|
||||
|
||||
// Scan through the values in this basic block.
|
||||
for entry in cfg.basic_block_by_index(*basic_block_id) {
|
||||
for entry in cfg.basic_block(*basic_block_id) {
|
||||
match entry {
|
||||
// If the element is an assignment.
|
||||
//
|
||||
|
|
|
|||
|
|
@ -7,9 +7,7 @@ use oxc_ast::{
|
|||
AstKind,
|
||||
};
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
use oxc_semantic::{
|
||||
petgraph::stable_graph::NodeIndex, pg::neighbors_filtered_by_edge_weight, AstNodeId, EdgeType,
|
||||
};
|
||||
use oxc_semantic::{pg::neighbors_filtered_by_edge_weight, AstNodeId, BasicBlockId, EdgeType};
|
||||
use oxc_span::{GetSpan, Span};
|
||||
|
||||
use crate::{context::LintContext, rule::Rule, AstNode};
|
||||
|
|
@ -59,8 +57,8 @@ impl Rule for NoThisBeforeSuper {
|
|||
|
||||
// first pass -> find super calls and local violations
|
||||
let mut wanted_nodes = Vec::new();
|
||||
let mut basic_blocks_with_super_called = HashSet::<NodeIndex>::new();
|
||||
let mut basic_blocks_with_local_violations = HashMap::<NodeIndex, Vec<AstNodeId>>::new();
|
||||
let mut basic_blocks_with_super_called = HashSet::<BasicBlockId>::new();
|
||||
let mut basic_blocks_with_local_violations = HashMap::<BasicBlockId, Vec<AstNodeId>>::new();
|
||||
for node in semantic.nodes().iter() {
|
||||
match node.kind() {
|
||||
AstKind::Function(_) | AstKind::ArrowFunctionExpression(_) => {
|
||||
|
|
@ -69,7 +67,7 @@ impl Rule for NoThisBeforeSuper {
|
|||
}
|
||||
}
|
||||
AstKind::Super(_) => {
|
||||
let basic_block_id = node.cfg_ix();
|
||||
let basic_block_id = node.cfg_id();
|
||||
if let Some(parent) = semantic.nodes().parent_node(node.id()) {
|
||||
if let AstKind::CallExpression(_) = parent.kind() {
|
||||
// Note: we don't need to worry about also having invalid
|
||||
|
|
@ -86,7 +84,7 @@ impl Rule for NoThisBeforeSuper {
|
|||
}
|
||||
}
|
||||
AstKind::ThisExpression(_) => {
|
||||
let basic_block_id = node.cfg_ix();
|
||||
let basic_block_id = node.cfg_id();
|
||||
if !basic_blocks_with_super_called.contains(&basic_block_id) {
|
||||
basic_blocks_with_local_violations
|
||||
.entry(basic_block_id)
|
||||
|
|
@ -103,7 +101,7 @@ impl Rule for NoThisBeforeSuper {
|
|||
for node in wanted_nodes {
|
||||
let output = neighbors_filtered_by_edge_weight(
|
||||
&cfg.graph,
|
||||
node.cfg_ix(),
|
||||
node.cfg_id(),
|
||||
&|edge| match edge {
|
||||
EdgeType::Normal => None,
|
||||
EdgeType::Backedge | EdgeType::NewFunction => {
|
||||
|
|
|
|||
|
|
@ -94,7 +94,7 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b
|
|||
let cfg = ctx.semantic().cfg();
|
||||
let state = neighbors_filtered_by_edge_weight(
|
||||
&cfg.graph,
|
||||
node.cfg_ix(),
|
||||
node.cfg_id(),
|
||||
&|edge| match edge {
|
||||
// We only care about normal edges having a return statement.
|
||||
EdgeType::Normal => None,
|
||||
|
|
@ -109,7 +109,7 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b
|
|||
}
|
||||
}
|
||||
|
||||
for entry in cfg.basic_block_by_index(*basic_block_id) {
|
||||
for entry in cfg.basic_block(*basic_block_id) {
|
||||
if let BasicBlockElement::Assignment(to_reg, val) = entry {
|
||||
if matches!(to_reg, Register::Return)
|
||||
&& matches!(val, AssignmentValue::NotImplicitUndefined)
|
||||
|
|
|
|||
|
|
@ -5,9 +5,9 @@ use oxc_ast::{
|
|||
};
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
use oxc_semantic::{
|
||||
petgraph::{self, graph::NodeIndex},
|
||||
petgraph::{self},
|
||||
pg::neighbors_filtered_by_edge_weight,
|
||||
AstNodeId, AstNodes, BasicBlockElement, EdgeType, Register,
|
||||
AstNodeId, AstNodes, BasicBlockElement, BasicBlockId, EdgeType, Register,
|
||||
};
|
||||
use oxc_span::{Atom, CompactStr};
|
||||
use oxc_syntax::operator::AssignmentOperator;
|
||||
|
|
@ -220,18 +220,18 @@ impl Rule for RulesOfHooks {
|
|||
return;
|
||||
}
|
||||
|
||||
let node_cfg_ix = node.cfg_ix();
|
||||
let func_cfg_ix = parent_func.cfg_ix();
|
||||
let node_cfg_id = node.cfg_id();
|
||||
let func_cfg_id = parent_func.cfg_id();
|
||||
|
||||
// there is no branch between us and our parent function
|
||||
if node_cfg_ix == func_cfg_ix {
|
||||
if node_cfg_id == func_cfg_id {
|
||||
return;
|
||||
}
|
||||
|
||||
if !petgraph::algo::has_path_connecting(
|
||||
&semantic.cfg().graph,
|
||||
func_cfg_ix,
|
||||
node_cfg_ix,
|
||||
func_cfg_id,
|
||||
node_cfg_id,
|
||||
None,
|
||||
) {
|
||||
// There should always be a control flow path between a parent and child node.
|
||||
|
|
@ -246,8 +246,8 @@ impl Rule for RulesOfHooks {
|
|||
return ctx.diagnostic(diagnostics::loop_hook(span, hook_name));
|
||||
}
|
||||
|
||||
if self.is_conditional(ctx, func_cfg_ix, node_cfg_ix)
|
||||
|| self.breaks_early(ctx, func_cfg_ix, node_cfg_ix)
|
||||
if self.is_conditional(ctx, func_cfg_id, node_cfg_id)
|
||||
|| self.breaks_early(ctx, func_cfg_id, node_cfg_id)
|
||||
{
|
||||
#[allow(clippy::needless_return)]
|
||||
return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name));
|
||||
|
|
@ -278,42 +278,42 @@ impl RulesOfHooks {
|
|||
fn is_conditional(
|
||||
&self,
|
||||
ctx: &LintContext,
|
||||
func_cfg_ix: NodeIndex,
|
||||
node_cfg_ix: NodeIndex,
|
||||
func_cfg_id: BasicBlockId,
|
||||
node_cfg_id: BasicBlockId,
|
||||
) -> bool {
|
||||
let graph = &ctx.semantic().cfg().graph;
|
||||
// All nodes should be reachable from our hook, Otherwise we have a conditional/branching flow.
|
||||
petgraph::algo::dijkstra(graph, func_cfg_ix, Some(node_cfg_ix), |e| match e.weight() {
|
||||
petgraph::algo::dijkstra(graph, func_cfg_id, Some(node_cfg_id), |e| match e.weight() {
|
||||
EdgeType::NewFunction => 1,
|
||||
EdgeType::Backedge | EdgeType::Normal => 0,
|
||||
})
|
||||
.into_iter()
|
||||
.filter(|(_, val)| *val == 0)
|
||||
.any(|(f, _)| !petgraph::algo::has_path_connecting(graph, f, node_cfg_ix, None))
|
||||
.any(|(f, _)| !petgraph::algo::has_path_connecting(graph, f, node_cfg_id, None))
|
||||
}
|
||||
|
||||
#[inline(always)]
|
||||
fn breaks_early(
|
||||
&self,
|
||||
ctx: &LintContext,
|
||||
func_cfg_ix: NodeIndex,
|
||||
node_cfg_ix: NodeIndex,
|
||||
func_cfg_id: BasicBlockId,
|
||||
node_cfg_id: BasicBlockId,
|
||||
) -> bool {
|
||||
let cfg = ctx.semantic().cfg();
|
||||
neighbors_filtered_by_edge_weight(
|
||||
&cfg.graph,
|
||||
func_cfg_ix,
|
||||
func_cfg_id,
|
||||
&|e| match e {
|
||||
EdgeType::Normal => None,
|
||||
EdgeType::Backedge | EdgeType::NewFunction => Some(State::default()),
|
||||
},
|
||||
&mut |ix: &NodeIndex, mut state: State| {
|
||||
if node_cfg_ix == *ix {
|
||||
&mut |id: &BasicBlockId, mut state: State| {
|
||||
if node_cfg_id == *id {
|
||||
return (state, false);
|
||||
}
|
||||
|
||||
let (push, keep_walking) = cfg
|
||||
.basic_block_by_index(*ix)
|
||||
.basic_block(*id)
|
||||
.iter()
|
||||
.fold_while((false, true), |acc, it| match it {
|
||||
BasicBlockElement::Break(_) => FoldWhile::Done((true, false)),
|
||||
|
|
@ -327,7 +327,7 @@ impl RulesOfHooks {
|
|||
.into_inner();
|
||||
|
||||
if push {
|
||||
state.0.push(*ix);
|
||||
state.0.push(*id);
|
||||
}
|
||||
(state, keep_walking)
|
||||
},
|
||||
|
|
@ -340,7 +340,7 @@ impl RulesOfHooks {
|
|||
}
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
struct State(Vec<NodeIndex>);
|
||||
struct State(Vec<BasicBlockId>);
|
||||
|
||||
fn parent_func<'a>(nodes: &'a AstNodes<'a>, node: &AstNode) -> Option<&'a AstNode<'a>> {
|
||||
nodes.ancestors(node.id()).map(|id| nodes.get_node(id)).find(|it| it.kind().is_function_like())
|
||||
|
|
@ -380,10 +380,10 @@ fn is_somewhere_inside_component_or_hook(nodes: &AstNodes, node_id: AstNodeId) -
|
|||
},
|
||||
)
|
||||
})
|
||||
.any(|(ix, id)| {
|
||||
id.is_some_and(|name| {
|
||||
.any(|(id, ident)| {
|
||||
ident.is_some_and(|name| {
|
||||
is_react_component_or_hook_name(name.as_str())
|
||||
|| is_memo_or_forward_ref_callback(nodes, ix)
|
||||
|| is_memo_or_forward_ref_callback(nodes, id)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ fn main() -> std::io::Result<()> {
|
|||
|
||||
let mut ast_nodes_by_block = HashMap::<_, Vec<_>>::new();
|
||||
for node in semantic.semantic.nodes().iter() {
|
||||
let block = node.cfg_ix();
|
||||
let block = node.cfg_id();
|
||||
let block_ix = semantic.semantic.cfg().graph.node_weight(block).unwrap();
|
||||
ast_nodes_by_block.entry(*block_ix).or_default().push(node);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,12 +1,14 @@
|
|||
use super::{
|
||||
AssignmentValue, AstNodeId, BasicBlockElement, CompactStr, ControlFlowGraph, EdgeType, Graph,
|
||||
NodeIndex, PreservedExpressionState, PreservedStatementState, Register,
|
||||
AssignmentValue, AstNodeId, BasicBlockElement, BasicBlockId, CompactStr, ControlFlowGraph,
|
||||
EdgeType, Graph, PreservedExpressionState, PreservedStatementState, Register,
|
||||
StatementControlFlowType,
|
||||
};
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub struct ControlFlowGraphBuilder {
|
||||
pub graph: Graph<usize, EdgeType>,
|
||||
pub basic_blocks: Vec<Vec<BasicBlockElement>>,
|
||||
pub current_node_ix: BasicBlockId,
|
||||
// note: this should only land in the big box for all things that take arguments
|
||||
// ie: callexpression, arrayexpression, etc
|
||||
// todo: add assert that it is used every time?
|
||||
|
|
@ -22,16 +24,14 @@ pub struct ControlFlowGraphBuilder {
|
|||
// (start, tail, last_register_used)
|
||||
pub saved_stores: Vec<(Vec<BasicBlockElement>, Option<Register>)>,
|
||||
pub saved_store: Option<usize>,
|
||||
pub graph: Graph<usize, EdgeType>,
|
||||
pub current_node_ix: NodeIndex,
|
||||
pub basic_blocks_with_breaks: Vec<Vec<NodeIndex>>,
|
||||
pub basic_blocks_with_continues: Vec<Vec<NodeIndex>>,
|
||||
pub basic_blocks_with_breaks: Vec<Vec<BasicBlockId>>,
|
||||
pub basic_blocks_with_continues: Vec<Vec<BasicBlockId>>,
|
||||
// node indexes of the basic blocks of switch case conditions
|
||||
pub switch_case_conditions: Vec<Vec<NodeIndex>>,
|
||||
pub switch_case_conditions: Vec<Vec<BasicBlockId>>,
|
||||
pub next_label: Option<CompactStr>,
|
||||
pub label_to_ast_node_ix: Vec<(CompactStr, AstNodeId)>,
|
||||
pub ast_node_to_break_continue: Vec<(AstNodeId, usize, Option<usize>)>,
|
||||
pub after_throw_block: Option<NodeIndex>,
|
||||
pub after_throw_block: Option<BasicBlockId>,
|
||||
}
|
||||
|
||||
impl ControlFlowGraphBuilder {
|
||||
|
|
@ -51,7 +51,7 @@ impl ControlFlowGraphBuilder {
|
|||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn new_basic_block_for_function(&mut self) -> NodeIndex {
|
||||
pub fn new_basic_block_for_function(&mut self) -> BasicBlockId {
|
||||
self.basic_blocks.push(Vec::new());
|
||||
let basic_block_id = self.basic_blocks.len() - 1;
|
||||
let graph_index = self.graph.add_node(basic_block_id);
|
||||
|
|
@ -66,7 +66,7 @@ impl ControlFlowGraphBuilder {
|
|||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn new_basic_block(&mut self) -> NodeIndex {
|
||||
pub fn new_basic_block(&mut self) -> BasicBlockId {
|
||||
self.basic_blocks.push(Vec::new());
|
||||
let graph_index = self.graph.add_node(self.basic_blocks.len() - 1);
|
||||
self.current_node_ix = graph_index;
|
||||
|
|
@ -79,7 +79,7 @@ impl ControlFlowGraphBuilder {
|
|||
graph_index
|
||||
}
|
||||
|
||||
pub fn add_edge(&mut self, a: NodeIndex, b: NodeIndex, weight: EdgeType) {
|
||||
pub fn add_edge(&mut self, a: BasicBlockId, b: BasicBlockId, weight: EdgeType) {
|
||||
self.graph.add_edge(a, b, weight);
|
||||
}
|
||||
|
||||
|
|
@ -202,8 +202,8 @@ impl ControlFlowGraphBuilder {
|
|||
&mut self,
|
||||
preserved_state: &PreservedStatementState,
|
||||
id: AstNodeId,
|
||||
break_jump_position: NodeIndex<u32>,
|
||||
continue_jump_position: Option<NodeIndex<u32>>,
|
||||
break_jump_position: BasicBlockId,
|
||||
continue_jump_position: Option<BasicBlockId>,
|
||||
) {
|
||||
let basic_blocks_with_breaks = self
|
||||
.basic_blocks_with_breaks
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ use crate::AstNodeId;
|
|||
|
||||
pub use builder::ControlFlowGraphBuilder;
|
||||
|
||||
pub type BasicBlockId = NodeIndex;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||
pub enum Register {
|
||||
Index(u32),
|
||||
|
|
@ -128,17 +130,15 @@ pub struct ControlFlowGraph {
|
|||
|
||||
impl ControlFlowGraph {
|
||||
/// # Panics
|
||||
pub fn basic_block_by_index(&self, index: NodeIndex) -> &Vec<BasicBlockElement> {
|
||||
let idx =
|
||||
*self.graph.node_weight(index).expect("expected a valid node index in self.graph");
|
||||
self.basic_blocks.get(idx).expect("expected a valid node index in self.basic_blocks")
|
||||
pub fn basic_block(&self, id: BasicBlockId) -> &Vec<BasicBlockElement> {
|
||||
let ix = *self.graph.node_weight(id).expect("expected a valid node id in self.graph");
|
||||
self.basic_blocks.get(ix).expect("expected a valid node id in self.basic_blocks")
|
||||
}
|
||||
|
||||
/// # Panics
|
||||
pub fn basic_block_by_index_mut(&mut self, index: NodeIndex) -> &mut Vec<BasicBlockElement> {
|
||||
let idx =
|
||||
*self.graph.node_weight(index).expect("expected a valid node index in self.graph");
|
||||
self.basic_blocks.get_mut(idx).expect("expected a valid node index in self.basic_blocks")
|
||||
pub fn basic_block_mut(&mut self, id: BasicBlockId) -> &mut Vec<BasicBlockElement> {
|
||||
let ix = *self.graph.node_weight(id).expect("expected a valid node id in self.graph");
|
||||
self.basic_blocks.get_mut(ix).expect("expected a valid node id in self.basic_blocks")
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -31,10 +31,10 @@ use rustc_hash::FxHashSet;
|
|||
|
||||
pub use crate::{
|
||||
control_flow::{
|
||||
print_basic_block, AssignmentValue, BasicBlockElement, BinaryAssignmentValue, BinaryOp,
|
||||
CallType, CalleeWithArgumentsAssignmentValue, CollectionAssignmentValue, ControlFlowGraph,
|
||||
EdgeType, ObjectPropertyAccessAssignmentValue, Register, UnaryExpressioneAssignmentValue,
|
||||
UpdateAssignmentValue,
|
||||
print_basic_block, AssignmentValue, BasicBlockElement, BasicBlockId, BinaryAssignmentValue,
|
||||
BinaryOp, CallType, CalleeWithArgumentsAssignmentValue, CollectionAssignmentValue,
|
||||
ControlFlowGraph, EdgeType, ObjectPropertyAccessAssignmentValue, Register,
|
||||
UnaryExpressioneAssignmentValue, UpdateAssignmentValue,
|
||||
},
|
||||
node::{AstNode, AstNodeId, AstNodes},
|
||||
reference::{Reference, ReferenceFlag, ReferenceId},
|
||||
|
|
|
|||
|
|
@ -1,9 +1,7 @@
|
|||
use petgraph::stable_graph::NodeIndex;
|
||||
|
||||
use oxc_ast::AstKind;
|
||||
use oxc_index::IndexVec;
|
||||
|
||||
use crate::scope::ScopeId;
|
||||
use crate::{control_flow::BasicBlockId, scope::ScopeId};
|
||||
|
||||
pub use oxc_syntax::node::{AstNodeId, NodeFlags};
|
||||
|
||||
|
|
@ -17,23 +15,28 @@ pub struct AstNode<'a> {
|
|||
/// Associated Scope (initialized by binding)
|
||||
scope_id: ScopeId,
|
||||
|
||||
/// Associated NodeIndex in CFG (initialized by control_flow)
|
||||
cfg_ix: NodeIndex,
|
||||
/// Associated `BasicBlockId` in CFG (initialized by control_flow)
|
||||
cfg_id: BasicBlockId,
|
||||
|
||||
flags: NodeFlags,
|
||||
}
|
||||
|
||||
impl<'a> AstNode<'a> {
|
||||
pub fn new(kind: AstKind<'a>, scope_id: ScopeId, cfg_ix: NodeIndex, flags: NodeFlags) -> Self {
|
||||
Self { id: AstNodeId::new(0), kind, cfg_ix, scope_id, flags }
|
||||
pub fn new(
|
||||
kind: AstKind<'a>,
|
||||
scope_id: ScopeId,
|
||||
cfg_id: BasicBlockId,
|
||||
flags: NodeFlags,
|
||||
) -> Self {
|
||||
Self { id: AstNodeId::new(0), kind, cfg_id, scope_id, flags }
|
||||
}
|
||||
|
||||
pub fn id(&self) -> AstNodeId {
|
||||
self.id
|
||||
}
|
||||
|
||||
pub fn cfg_ix(&self) -> NodeIndex {
|
||||
self.cfg_ix
|
||||
pub fn cfg_id(&self) -> BasicBlockId {
|
||||
self.cfg_id
|
||||
}
|
||||
|
||||
pub fn kind(&self) -> AstKind<'a> {
|
||||
|
|
|
|||
|
|
@ -1,15 +1,17 @@
|
|||
use petgraph::{stable_graph::NodeIndex, visit::EdgeRef, Direction, Graph};
|
||||
use petgraph::{visit::EdgeRef, Direction, Graph};
|
||||
|
||||
use crate::BasicBlockId;
|
||||
|
||||
/// # Panics
|
||||
pub fn neighbors_filtered_by_edge_weight<State: Default + Clone, NodeWeight, EdgeWeight, F, G>(
|
||||
graph: &Graph<NodeWeight, EdgeWeight>,
|
||||
node: NodeIndex,
|
||||
node: BasicBlockId,
|
||||
edge_filter: &F,
|
||||
visitor: &mut G,
|
||||
) -> Vec<State>
|
||||
where
|
||||
F: Fn(&EdgeWeight) -> Option<State>,
|
||||
G: FnMut(&NodeIndex, State) -> (State, bool),
|
||||
G: FnMut(&BasicBlockId, State) -> (State, bool),
|
||||
{
|
||||
let mut q = vec![];
|
||||
let mut final_states = vec![];
|
||||
|
|
|
|||
Loading…
Reference in a new issue