refactor(semantic): make control flow generation optional. (#3737)

For maximum backward compatibility, we generate CFG by default.

Note: It can't be done with a simple method since lifetimes make it impossible(at least without unsafe trickery) I've tried to do it without a macro but it was just unintuitive.
This commit is contained in:
rzvxa 2024-06-18 15:59:38 +00:00
parent 0537d298db
commit d8ad321687
11 changed files with 551 additions and 412 deletions

View file

@ -124,6 +124,10 @@ pub struct ControlFlowGraph {
}
impl ControlFlowGraph {
pub fn graph(&self) -> &Graph<usize, EdgeType> {
&self.graph
}
/// # Panics
pub fn basic_block(&self, id: BasicBlockId) -> &BasicBlock {
let ix = *self.graph.node_weight(id).expect("expected a valid node id in self.graph");

View file

@ -8,7 +8,7 @@ use oxc_ast::{
use oxc_diagnostics::OxcDiagnostic;
use oxc_cfg::{
graph::visit::neighbors_filtered_by_edge_weight, EdgeType, InstructionKind,
graph::visit::neighbors_filtered_by_edge_weight, ControlFlowGraph, EdgeType, InstructionKind,
ReturnInstructionKind,
};
use oxc_macros::declare_oxc_lint;
@ -55,16 +55,19 @@ declare_oxc_lint!(
impl Rule for GetterReturn {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
// control flow dependant
let Some(cfg) = ctx.semantic().cfg() else { return };
// https://eslint.org/docs/latest/rules/getter-return#handled_by_typescript
if ctx.source_type().is_typescript() {
return;
}
match node.kind() {
AstKind::Function(func) if !func.is_typescript_syntax() => {
self.run_diagnostic(node, ctx, func.span);
self.run_diagnostic(node, ctx, cfg, func.span);
}
AstKind::ArrowFunctionExpression(expr) => {
self.run_diagnostic(node, ctx, expr.span);
self.run_diagnostic(node, ctx, cfg, expr.span);
}
_ => {}
}
@ -178,15 +181,19 @@ impl GetterReturn {
false
}
fn run_diagnostic<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>, span: Span) {
fn run_diagnostic<'a>(
&self,
node: &AstNode<'a>,
ctx: &LintContext<'a>,
cfg: &ControlFlowGraph,
span: Span,
) {
if !Self::is_wanted_node(node, ctx) {
return;
}
let cfg = ctx.semantic().cfg();
let output = neighbors_filtered_by_edge_weight(
&cfg.graph,
cfg.graph(),
node.cfg_id(),
&|edge| match edge {
EdgeType::Jump | EdgeType::Normal => None,

View file

@ -10,7 +10,7 @@ use oxc_cfg::{
visit::{neighbors_filtered_by_edge_weight, EdgeRef},
Direction,
},
BasicBlockId, EdgeType, ErrorEdgeKind, InstructionKind,
BasicBlockId, ControlFlowGraph, EdgeType, ErrorEdgeKind, InstructionKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
@ -89,13 +89,15 @@ impl Rule for NoFallthrough {
}
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
// control flow dependant
let Some(cfg) = ctx.semantic().cfg() else { return };
let AstKind::SwitchStatement(switch) = node.kind() else { return };
let switch_id = node.cfg_id();
let cfg = ctx.semantic().cfg();
let graph = &cfg.graph;
let graph = cfg.graph();
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch);
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, cfg, node, switch);
let Some(default_or_exit) = default.or(exit) else {
// TODO: our `get_switch_semantic_cases` can't evaluate cfg_ids for switch statements
@ -260,6 +262,7 @@ impl NoFallthrough {
// Issue: <https://github.com/oxc-project/oxc/issues/3662>
fn get_switch_semantic_cases(
ctx: &LintContext,
cfg: &ControlFlowGraph,
node: &AstNode,
switch: &SwitchStatement,
) -> (
@ -268,8 +271,7 @@ fn get_switch_semantic_cases(
/* default */ Option<BasicBlockId>,
/* exit */ Option<BasicBlockId>,
) {
let cfg = &ctx.semantic().cfg();
let graph = &cfg.graph;
let graph = cfg.graph();
let has_default = switch.cases.iter().any(SwitchCase::is_default_case);
let (tests, exit) = graph
.edges_directed(node.cfg_id(), Direction::Outgoing)

View file

@ -58,7 +58,8 @@ enum DefinitelyCallsThisBeforeSuper {
impl Rule for NoThisBeforeSuper {
fn run_once(&self, ctx: &LintContext) {
let semantic = ctx.semantic();
let cfg = semantic.cfg();
// control flow dependant
let Some(cfg) = ctx.semantic().cfg() else { return };
// first pass -> find super calls and local violations
let mut wanted_nodes = Vec::new();

View file

@ -31,10 +31,12 @@ declare_oxc_lint!(
impl Rule for NoUnreachable {
fn run_once(&self, ctx: &LintContext) {
// control flow dependant
let Some(cfg) = ctx.semantic().cfg() else { return };
let nodes = ctx.nodes();
let Some(root) = nodes.root_node() else { return };
let cfg = ctx.semantic().cfg();
let graph = &cfg.graph;
let graph = cfg.graph();
// A pre-allocated vector containing the reachability status of all the basic blocks.
// We initialize this vector with all nodes set to `unreachable` since if we don't visit a

View file

@ -2,8 +2,8 @@ use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_cfg::{
graph::visit::neighbors_filtered_by_edge_weight, EdgeType, Instruction, InstructionKind,
ReturnInstructionKind,
graph::visit::neighbors_filtered_by_edge_weight, ControlFlowGraph, EdgeType, Instruction,
InstructionKind, ReturnInstructionKind,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
@ -51,6 +51,9 @@ declare_oxc_lint!(
impl Rule for RequireRenderReturn {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
// control flow dependant
let Some(cfg) = ctx.semantic().cfg() else { return };
if !matches!(node.kind(), AstKind::ArrowFunctionExpression(_) | AstKind::Function(_)) {
return;
}
@ -64,7 +67,7 @@ impl Rule for RequireRenderReturn {
return;
}
if !contains_return_statement(node, ctx) {
if !contains_return_statement(node, cfg) {
match parent.kind() {
AstKind::MethodDefinition(method) => {
ctx.diagnostic(require_render_return_diagnostic(method.key.span()));
@ -91,10 +94,9 @@ enum FoundReturn {
const KEEP_WALKING_ON_THIS_PATH: bool = true;
const STOP_WALKING_ON_THIS_PATH: bool = false;
fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool {
let cfg = ctx.semantic().cfg();
fn contains_return_statement(node: &AstNode, cfg: &ControlFlowGraph) -> bool {
let state = neighbors_filtered_by_edge_weight(
&cfg.graph,
cfg.graph(),
node.cfg_id(),
&|edge| match edge {
// We only care about normal edges having a return statement.

View file

@ -4,7 +4,7 @@ use oxc_ast::{
};
use oxc_cfg::{
graph::{algo, visit::Control},
EdgeType, ErrorEdgeKind, InstructionKind,
ControlFlowGraph, EdgeType, ErrorEdgeKind, InstructionKind,
};
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{AstNodeId, AstNodes};
@ -107,6 +107,9 @@ declare_oxc_lint!(
impl Rule for RulesOfHooks {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
// control flow dependant
let Some(cfg) = ctx.semantic().cfg() else { return };
let AstKind::CallExpression(call) = node.kind() else { return };
if !is_react_hook(&call.callee) {
@ -227,7 +230,7 @@ impl Rule for RulesOfHooks {
return;
}
if !ctx.semantic().cfg().is_reachable(func_cfg_id, node_cfg_id) {
if !cfg.is_reachable(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
@ -236,11 +239,11 @@ impl Rule for RulesOfHooks {
}
// Is this node cyclic?
if semantic.cfg().is_cyclic(node_cfg_id) {
if cfg.is_cyclic(node_cfg_id) {
return ctx.diagnostic(diagnostics::loop_hook(span, hook_name));
}
if has_conditional_path_accept_throw(ctx, parent_func, node) {
if has_conditional_path_accept_throw(cfg, parent_func, node) {
#[allow(clippy::needless_return)]
return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name));
}
@ -248,14 +251,13 @@ impl Rule for RulesOfHooks {
}
fn has_conditional_path_accept_throw(
ctx: &LintContext<'_>,
cfg: &ControlFlowGraph,
from: &AstNode<'_>,
to: &AstNode<'_>,
) -> bool {
let from_graph_id = from.cfg_id();
let to_graph_id = to.cfg_id();
let cfg = ctx.semantic().cfg();
let graph = &cfg.graph;
let graph = cfg.graph();
if graph
.edges(to_graph_id)
.any(|it| matches!(it.weight(), EdgeType::Error(ErrorEdgeKind::Explicit)))

View file

@ -46,6 +46,7 @@ fn main() -> std::io::Result<()> {
let semantic = SemanticBuilder::new(&source_text, source_type)
.with_check_syntax_error(true)
.with_trivias(ret.trivias)
.with_cfg(true)
.build(program);
if !semantic.errors.is_empty() {
@ -59,16 +60,19 @@ fn main() -> std::io::Result<()> {
return Ok(());
}
let cfg = semantic
.semantic
.cfg()
.expect("we set semantic to build the control flow (`with_cfg`) for us so it should always be `Some`");
let mut ast_nodes_by_block = HashMap::<_, Vec<_>>::new();
for node in semantic.semantic.nodes().iter() {
let block = node.cfg_id();
let block_ix = semantic.semantic.cfg().graph.node_weight(block).unwrap();
let block_ix = cfg.graph.node_weight(block).unwrap();
ast_nodes_by_block.entry(*block_ix).or_default().push(node);
}
let basic_blocks_printed = semantic
.semantic
.cfg()
let basic_blocks_printed = cfg
.basic_blocks
.iter()
.map(DisplayDot::display_dot)
@ -93,13 +97,13 @@ fn main() -> std::io::Result<()> {
let cfg_dot_diagram = format!(
"{:?}",
Dot::with_attr_getters(
&semantic.semantic.cfg().graph,
cfg.graph(),
&[Config::EdgeNoLabel, Config::NodeNoLabel],
&|_graph, edge| {
let weight = edge.weight();
let label = format!("label = \"{weight:?}\"");
if matches!(weight, EdgeType::Unreachable)
|| semantic.semantic.cfg().basic_block(edge.source()).unreachable
|| cfg.basic_block(edge.source()).unreachable
{
format!("{label}, style = \"dotted\" ")
} else {
@ -124,9 +128,7 @@ fn main() -> std::io::Result<()> {
node.1,
nodes,
node.1,
semantic.semantic.cfg().basic_blocks[*node.1]
.debug_dot(semantic.semantic.nodes().into())
.trim()
cfg.basic_blocks[*node.1].debug_dot(semantic.semantic.nodes().into()).trim()
)
}
)

File diff suppressed because it is too large Load diff

View file

@ -56,7 +56,7 @@ pub struct Semantic<'a> {
unused_labels: FxHashSet<AstNodeId>,
cfg: ControlFlowGraph,
cfg: Option<ControlFlowGraph>,
}
impl<'a> Semantic<'a> {
@ -108,8 +108,8 @@ impl<'a> Semantic<'a> {
&self.unused_labels
}
pub fn cfg(&self) -> &ControlFlowGraph {
&self.cfg
pub fn cfg(&self) -> Option<&ControlFlowGraph> {
self.cfg.as_ref()
}
pub fn is_unresolved_reference(&self, node_id: AstNodeId) -> bool {

View file

@ -102,29 +102,29 @@ impl<'a> SemanticTester<'a> {
pub fn basic_blocks_count(&self) -> usize {
let built = self.build();
built.cfg().basic_blocks.len()
built.cfg().map_or(0, |cfg| cfg.basic_blocks.len())
}
pub fn basic_blocks_printed(&self) -> String {
let built = self.build();
built
.cfg()
.basic_blocks
.iter()
.map(DisplayDot::display_dot)
.enumerate()
.map(|(i, it)| {
format!(
"bb{i}: {{\n{}\n}}",
it.lines().map(|x| format!("\t{}", x.trim())).join("\n")
)
})
.join("\n\n")
built.cfg().map_or_else(String::default, |cfg| {
cfg.basic_blocks
.iter()
.map(DisplayDot::display_dot)
.enumerate()
.map(|(i, it)| {
format!(
"bb{i}: {{\n{}\n}}",
it.lines().map(|x| format!("\t{}", x.trim())).join("\n")
)
})
.join("\n\n")
})
}
pub fn cfg_dot_diagram(&self) -> String {
let semantic = self.build();
semantic.cfg().debug_dot(semantic.nodes().into())
semantic.cfg().map_or_else(String::default, |cfg| cfg.debug_dot(semantic.nodes().into()))
}
/// Tests that a symbol with the given name exists at the top-level scope and provides a