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.

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

improvement(semantic/cfg): better CFG API #3472

Merged
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
7 changes: 1 addition & 6 deletions crates/oxc_linter/src/rules/react/rules_of_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,7 @@ impl Rule for RulesOfHooks {
return;
}

if !petgraph::algo::has_path_connecting(
&semantic.cfg().graph,
func_cfg_id,
node_cfg_id,
None,
) {
if !ctx.semantic().cfg().is_reachabale(func_cfg_id, node_cfg_id) {
// There should always be a control flow path between a parent and child node.
// If there is none it means we always do an early exit before reaching our hook call.
// In some cases it might mean that we are operating on an invalid `cfg` but in either
Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_semantic/examples/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ fn main() -> std::io::Result<()> {
}
});
format!(
"xlabel = \"nodes [{}]\\l\", label = \"bb{}\n{}\"",
"xlabel = \"nodes{} [{}]\\l\", label = \"bb{}\n{}\"",
node.1,
nodes,
node.1,
semantic.semantic.cfg().basic_blocks[*node.1]
Expand Down
48 changes: 34 additions & 14 deletions crates/oxc_semantic/src/control_flow/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
mod builder;
mod dot;

use itertools::Itertools;
use oxc_span::CompactStr;
use oxc_syntax::operator::{
AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator, UpdateOperator,
};
use petgraph::{
stable_graph::NodeIndex,
visit::{depth_first_search, Dfs, DfsEvent, Walker},
visit::{depth_first_search, Control, DfsEvent},
Graph,
};

Expand Down Expand Up @@ -192,19 +191,28 @@ impl ControlFlowGraph {
}

pub fn is_reachabale(&self, from: BasicBlockId, to: BasicBlockId) -> bool {
if from == to {
return true;
}
let graph = &self.graph;
let mut dfs = Dfs::empty(graph);
dfs.reset(graph);
dfs.move_to(from);
dfs.iter(graph)
.take_while_inclusive(|it| {
!self
.basic_block(*it)
.instructions()
.iter()
.any(|it| matches!(it, Instruction { kind: InstructionKind::Unreachable, .. }))
})
.any(|x| x == to)
depth_first_search(&self.graph, Some(from), |event| match event {
DfsEvent::TreeEdge(a, b) => {
let unreachable = graph.edges_connecting(a, b).all(|edge| {
matches!(edge.weight(), EdgeType::NewFunction | EdgeType::Unreachable)
});

if unreachable {
Control::Prune
} else if b == to {
return Control::Break(true);
} else {
Control::Continue
}
}
_ => Control::Continue,
})
.break_value()
.unwrap_or(false)
}

pub fn is_cyclic(&self, node: BasicBlockId) -> bool {
Expand All @@ -214,6 +222,18 @@ impl ControlFlowGraph {
})
.is_err()
}

pub fn has_conditional_path(&self, from: BasicBlockId, to: BasicBlockId) -> bool {
let graph = &self.graph;
// All nodes should be able to reach the `to` node, Otherwise we have a conditional/branching flow.
petgraph::algo::dijkstra(graph, from, Some(to), |e| match e.weight() {
EdgeType::NewFunction => 1,
EdgeType::Jump | EdgeType::Unreachable | EdgeType::Backedge | EdgeType::Normal => 0,
})
.into_iter()
.filter(|(_, val)| *val == 0)
.any(|(f, _)| !self.is_reachabale(f, to))
}
}

pub struct PreservedExpressionState {
Expand Down
Loading