refactor(semantic/cfg): cleanup control flow and it's builder. (#3650)

Removed remaining parts of the old CFG implementation and some cleanups here and there.

Author: rzvxa

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

@@ -225,7 +225,7 @@ impl Rule for RulesOfHooks {
             return;
         }
 
-        if !ctx.semantic().cfg().is_reachabale(func_cfg_id, node_cfg_id) {
+        if !ctx.semantic().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
@@ -302,7 +302,7 @@ fn has_conditional_path_accept_throw(
     .into_iter()
     .filter(|(_, val)| *val == 0)
     .any(|(f, _)| {
-        !cfg.is_reachabale_filtered(f, to_graph_id, |it| {
+        !cfg.is_reachable_filtered(f, to_graph_id, |it| {
             if cfg
                 .basic_block(it)
                 .instructions()

crates/oxc_semantic/src/builder.rs

@@ -18,7 +18,7 @@ use crate::{
     class::ClassTableBuilder,
     control_flow::{
         ControlFlowGraphBuilder, CtxCursor, CtxFlags, EdgeType, ErrorEdgeKind,
-        IterationInstructionKind, Register, ReturnInstructionKind,
+        IterationInstructionKind, ReturnInstructionKind,
     },
     diagnostics::redeclaration,
     jsdoc::JSDocBuilder,
@@ -541,13 +541,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
     fn visit_debugger_statement(&mut self, stmt: &DebuggerStatement) {
         let kind = AstKind::DebuggerStatement(self.alloc(stmt));
         self.enter_node(kind);
-
-        /* cfg */
-        // just take the next_label since it should be taken by the next
-        // statement regardless of whether the statement can use it or not
-        self.cfg.next_label.take();
-        /* cfg */
-
         self.leave_node(kind);
     }
 
@@ -1006,10 +999,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         self.visit_label_identifier(&stmt.label);
 
-        /* cfg */
-        self.cfg.next_label = Some(stmt.label.name.to_compact_str());
-        /* cfg */
-
         self.visit_statement(&stmt.body);
 
         /* cfg */
@@ -1373,8 +1362,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         func.scope_id.set(Some(self.current_scope_id));
 
         /* cfg */
-        let preserved = self.cfg.preserve_expression_state();
-
         let before_function_graph_ix = self.cfg.current_node_ix;
         let error_harness = self.cfg.attach_error_harness(ErrorEdgeKind::Implicit);
         let function_graph_ix = self.cfg.new_basic_block_function();
@@ -1398,7 +1385,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         }
 
         /* cfg */
-        self.cfg.restore_expression_state(preserved);
         self.cfg.ctx(None).resolve_expect(CtxFlags::FUNCTION);
         self.cfg.release_error_harness(error_harness);
         let after_function_graph_ix = self.cfg.new_basic_block_normal();
@@ -1435,11 +1421,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         self.enter_node(kind);
 
-        /* cfg */
-        let preserved = self.cfg.preserve_expression_state();
-        self.cfg.store_final_assignments_into_this_array.push(vec![]);
-        /* cfg */
-
         if let Some(id) = &class.id {
             self.visit_binding_identifier(id);
         }
@@ -1455,14 +1436,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         }
         self.visit_class_body(&class.body);
 
-        /* cfg */
-        let _elements = self.cfg.store_final_assignments_into_this_array.pop().expect(
-            "expected there to be atleast one vec in the store_final_assignments_into_this_arrays",
-        );
-        self.cfg.restore_expression_state(preserved);
-        self.cfg.spread_indices.push(vec![]);
-        /* cfg */
-
         self.leave_node(kind);
         if is_class_expr {
             self.leave_scope();
@@ -1485,7 +1458,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         expr.scope_id.set(Some(self.current_scope_id));
 
         /* cfg */
-        let preserved = self.cfg.preserve_expression_state();
         let current_node_ix = self.cfg.current_node_ix;
         let error_harness = self.cfg.attach_error_harness(ErrorEdgeKind::Implicit);
         let function_graph_ix = self.cfg.new_basic_block_function();
@@ -1500,15 +1472,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         /* cfg */
         self.cfg.add_edge(current_node_ix, function_graph_ix, EdgeType::NewFunction);
-        if expr.expression {
-            self.cfg.store_assignments_into_this_array.push(vec![]);
-            self.cfg.use_this_register = Some(Register::Return);
-        }
         /* cfg */
+
         self.visit_function_body(&expr.body);
 
         /* cfg */
-        self.cfg.restore_expression_state(preserved);
         self.cfg.ctx(None).resolve_expect(CtxFlags::FUNCTION);
         self.cfg.release_error_harness(error_harness);
         self.cfg.current_node_ix = current_node_ix;

crates/oxc_semantic/src/control_flow/builder/mod.rs

@@ -7,9 +7,8 @@ pub use context::{CtxCursor, CtxFlags};
 use petgraph::Direction;
 
 use super::{
-    AstNodeId, BasicBlock, BasicBlockId, CompactStr, ControlFlowGraph, EdgeType, ErrorEdgeKind,
-    Graph, Instruction, InstructionKind, IterationInstructionKind, LabeledInstruction,
-    PreservedExpressionState, Register,
+    AstNodeId, BasicBlock, BasicBlockId, ControlFlowGraph, EdgeType, ErrorEdgeKind, Graph,
+    Instruction, InstructionKind, IterationInstructionKind, LabeledInstruction,
 };
 
 #[derive(Debug, Default)]
@@ -25,27 +24,6 @@ pub struct ControlFlowGraphBuilder<'a> {
     error_path: Vec<ErrorHarness>,
     /// Stack of finalizers, the top most element is always the appropriate one for current node.
     finalizers: Vec<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?
-    pub use_this_register: Option<Register>,
-    pub next_free_register: u32,
-    pub store_assignments_into_this_array: Vec<Vec<Register>>,
-    pub store_final_assignments_into_this_array: Vec<Vec<Register>>,
-    // indexes of spreads in the store_assignments_into_this_array
-    pub spread_indices: Vec<Vec<usize>>,
-    // computed member expressions are only executed when we reach
-    // that part of the chain, so we keep this vec to patch them in later
-    pub should_save_stores_for_patching: bool,
-    pub saved_store: Option<usize>,
-    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<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<BasicBlockId>,
 }
 
 impl<'a> ControlFlowGraphBuilder<'a> {
@@ -243,27 +221,6 @@ impl<'a> ControlFlowGraphBuilder<'a> {
         self.basic_block_mut(block).instructions.push(Instruction { kind, node_id });
     }
 
-    #[must_use]
-    pub fn preserve_expression_state(&mut self) -> PreservedExpressionState {
-        let use_this_register = self.use_this_register.take();
-        let mut store_final_assignments_into_this_array = vec![];
-        std::mem::swap(
-            &mut store_final_assignments_into_this_array,
-            &mut self.store_final_assignments_into_this_array,
-        );
-
-        // DO NOT preserve: saved_stores, should_save_stores_for_patching
-        // should_save_stores_for_patching must always be active to catch
-        // all stores, preserving will mess it up.
-        PreservedExpressionState { use_this_register, store_final_assignments_into_this_array }
-    }
-
-    pub fn restore_expression_state(&mut self, mut preserved_state: PreservedExpressionState) {
-        self.use_this_register = preserved_state.use_this_register.take();
-        self.store_final_assignments_into_this_array =
-            preserved_state.store_final_assignments_into_this_array;
-    }
-
     pub fn enter_statement(&mut self, stmt: AstNodeId) {
         self.push_statement(stmt);
     }

crates/oxc_semantic/src/control_flow/mod.rs

@@ -222,11 +222,11 @@ impl ControlFlowGraph {
         self.basic_blocks.get_mut(ix).expect("expected a valid node id in self.basic_blocks")
     }
 
-    pub fn is_reachabale(&self, from: BasicBlockId, to: BasicBlockId) -> bool {
-        self.is_reachabale_filtered(from, to, |_| Control::Continue)
+    pub fn is_reachable(&self, from: BasicBlockId, to: BasicBlockId) -> bool {
+        self.is_reachable_filtered(from, to, |_| Control::Continue)
     }
 
-    pub fn is_reachabale_filtered<F: Fn(BasicBlockId) -> Control<bool>>(
+    pub fn is_reachable_filtered<F: Fn(BasicBlockId) -> Control<bool>>(
         &self,
         from: BasicBlockId,
         to: BasicBlockId,
@@ -352,21 +352,4 @@ 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 | EdgeType::Error(_) | EdgeType::Finalize | EdgeType::Join => 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 {
-    pub use_this_register: Option<Register>,
-    pub store_final_assignments_into_this_array: Vec<Vec<Register>>,
 }