improvement(semantic/cfg): rework basic blocks.

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

@@ -9,7 +9,7 @@ use oxc_diagnostics::OxcDiagnostic;
 
 use oxc_macros::declare_oxc_lint;
 use oxc_semantic::{
-    pg::neighbors_filtered_by_edge_weight, AssignmentValue, BasicBlockElement, EdgeType, Register,
+    pg::neighbors_filtered_by_edge_weight, EdgeType, InstructionKind, ReturnInstructionKind,
 };
 use oxc_span::Span;
 
@@ -191,6 +191,8 @@ impl GetterReturn {
                 // We don't need to visit NewFunction edges because it's not going to be evaluated
                 // immediately, and we are only doing a pass over things that will be immediately evaluated
                 | EdgeType::NewFunction
+                // Unreachable nodes aren't reachable so we don't follow them.
+                | EdgeType::Unreachable
                 // By returning Some(X),
                 // we signal that we don't walk to this path any farther.
                 //
@@ -229,49 +231,28 @@ impl GetterReturn {
                 }
 
                 // Scan through the values in this basic block.
-                for entry in cfg.basic_block(*basic_block_id) {
-                    match entry {
-                        // If the element is an assignment.
-                        //
-                        // Everything you can write in javascript that would have
-                        // the function continue are expressed as assignments in the cfg.
-                        BasicBlockElement::Assignment(to_reg, val) => {
-                            // If the assignment is to the return register.
-                            //
-                            // The return register is a special register that return statements
-                            // assign the returned value to.
-                            if matches!(to_reg, Register::Return) {
+                for entry in cfg.basic_block(*basic_block_id).instructions() {
+                    match entry.kind {
+                        // If the element is a return.
                                 // `allow_implicit` allows returning without a value to not
-                                // fail the rule. We check for this by checking if the value
-                                // being returned in the cfg this is expressed as
-                                // `AssignmentValue::ImplicitUndefined`.
-                                //
-                                // There is an assumption being made here that returning an
-                                // `undefined` will put the `undefined` directly into the
-                                // return and will not put the `undefined` into an immediate
-                                // register and return the register. However, the tests for
-                                // this rule enforce that this invariant is not broken.
-                                if !self.allow_implicit
-                                    && matches!(val, AssignmentValue::ImplicitUndefined)
-                                {
-                                    // Return false as the second argument to signify we should
-                                    // not continue walking this branch, as we know a return
-                                    // is the end of this path.
-                                    return (DefinitelyReturnsOrThrowsOrUnreachable::No, false);
-                                }
+                                // fail the rule.
+                                // Return false as the second argument to signify we should
+                                // not continue walking this branch, as we know a return
+                                // is the end of this path.
+                        InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined) if !self.allow_implicit => {
+                                return (DefinitelyReturnsOrThrowsOrUnreachable::No, false);
+                        }
                                 // Otherwise, we definitely returned since we assigned
                                 // to the return register.
                                 //
                                 // Return false as the second argument to signify we should
                                 // not continue walking this branch, as we know a return
                                 // is the end of this path.
-                                return (DefinitelyReturnsOrThrowsOrUnreachable::Yes, false);
-                            }
-                        }
+                        | InstructionKind::Return(_)
                         // Throws are classified as returning.
                         //
                         // todo: test with catching...
-                        BasicBlockElement::Throw(_) |
+                        | InstructionKind::Throw
                         // Although the unreachable code is not returned, it will never be executed.
                         // There is no point in checking it for return.
                         //
@@ -283,12 +264,12 @@ impl GetterReturn {
                         // return -1;
                         // ```
                         // Make return useless.
-                        BasicBlockElement::Unreachable => {
-
-                            return (DefinitelyReturnsOrThrowsOrUnreachable::Yes, false);
+                        | InstructionKind::Unreachable =>{
+                                return (DefinitelyReturnsOrThrowsOrUnreachable::Yes, false);
                         }
                         // Ignore irrelevant elements.
-                        BasicBlockElement::Break(_) => {}
+                        | InstructionKind::Break(_)
+                        | InstructionKind::Statement => {}
                     }
                 }
 

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

@@ -104,7 +104,7 @@ impl Rule for NoThisBeforeSuper {
                 node.cfg_id(),
                 &|edge| match edge {
                     EdgeType::Normal => None,
-                    EdgeType::Backedge | EdgeType::NewFunction => {
+                    EdgeType::Unreachable | EdgeType::Backedge | EdgeType::NewFunction => {
                         Some(DefinitelyCallsThisBeforeSuper::No)
                     }
                 },

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

@@ -3,7 +3,8 @@ use oxc_diagnostics::OxcDiagnostic;
 
 use oxc_macros::declare_oxc_lint;
 use oxc_semantic::{
-    pg::neighbors_filtered_by_edge_weight, AssignmentValue, BasicBlockElement, EdgeType, Register,
+    pg::neighbors_filtered_by_edge_weight, EdgeType, Instruction, InstructionKind,
+    ReturnInstructionKind,
 };
 use oxc_span::{GetSpan, Span};
 
@@ -99,7 +100,9 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b
             // We only care about normal edges having a return statement.
             EdgeType::Normal => None,
             // For these two type, we flag it as not found.
-            EdgeType::NewFunction | EdgeType::Backedge => Some(FoundReturn::No),
+            EdgeType::Unreachable | EdgeType::NewFunction | EdgeType::Backedge => {
+                Some(FoundReturn::No)
+            }
         },
         &mut |basic_block_id, _state_going_into_this_rule| {
             // If its an arrow function with an expression, marked as founded and stop walking.
@@ -109,19 +112,17 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b
                 }
             }
 
-            for entry in cfg.basic_block(*basic_block_id) {
-                match entry {
-                    BasicBlockElement::Assignment(to_reg, val) => {
-                        if matches!(to_reg, Register::Return)
-                            && matches!(val, AssignmentValue::NotImplicitUndefined)
-                        {
-                            return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH);
-                        }
+            for Instruction { kind, .. } in cfg.basic_block(*basic_block_id).instructions() {
+                match kind {
+                    InstructionKind::Return(ReturnInstructionKind::NotImplicitUndefined) => {
+                        return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH);
                     }
-                    BasicBlockElement::Unreachable | BasicBlockElement::Throw(_) => {
+                    InstructionKind::Unreachable | InstructionKind::Throw => {
                         return (FoundReturn::No, STOP_WALKING_ON_THIS_PATH);
                     }
-                    BasicBlockElement::Break(_) => {}
+                    InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined)
+                    | InstructionKind::Break(_)
+                    | InstructionKind::Statement => {}
                 }
             }
 

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

@@ -7,7 +7,7 @@ use oxc_macros::declare_oxc_lint;
 use oxc_semantic::{
     petgraph::{self},
     pg::neighbors_filtered_by_edge_weight,
-    AstNodeId, AstNodes, BasicBlockElement, BasicBlockId, EdgeType, Register,
+    AstNodeId, AstNodes, BasicBlockId, EdgeType, Instruction, InstructionKind,
 };
 use oxc_span::{Atom, CompactStr};
 use oxc_syntax::operator::AssignmentOperator;
@@ -281,15 +281,16 @@ impl RulesOfHooks {
         func_cfg_id: BasicBlockId,
         node_cfg_id: BasicBlockId,
     ) -> bool {
-        let graph = &ctx.semantic().cfg().graph;
+        let cfg = ctx.semantic().cfg();
+        let graph = &cfg.graph;
         // All nodes should be reachable from our hook, Otherwise we have a conditional/branching flow.
         petgraph::algo::dijkstra(graph, func_cfg_id, Some(node_cfg_id), |e| match e.weight() {
             EdgeType::NewFunction => 1,
-            EdgeType::Backedge | EdgeType::Normal => 0,
+            EdgeType::Unreachable | EdgeType::Backedge | EdgeType::Normal => 0,
         })
         .into_iter()
         .filter(|(_, val)| *val == 0)
-        .any(|(f, _)| !petgraph::algo::has_path_connecting(graph, f, node_cfg_id, None))
+        .any(|(f, _)| !cfg.is_reachabale(f, node_cfg_id))
     }
 
     #[inline(always)]
@@ -305,7 +306,9 @@ impl RulesOfHooks {
             func_cfg_id,
             &|e| match e {
                 EdgeType::Normal => None,
-                EdgeType::Backedge | EdgeType::NewFunction => Some(State::default()),
+                EdgeType::Unreachable | EdgeType::Backedge | EdgeType::NewFunction => {
+                    Some(State::default())
+                }
             },
             &mut |id: &BasicBlockId, mut state: State| {
                 if node_cfg_id == *id {
@@ -314,15 +317,14 @@ impl RulesOfHooks {
 
                 let (push, keep_walking) = cfg
                     .basic_block(*id)
+                    .instructions
                     .iter()
-                    .fold_while((false, true), |acc, it| match it {
-                        BasicBlockElement::Break(_) => FoldWhile::Done((true, false)),
-                        BasicBlockElement::Unreachable
-                        | BasicBlockElement::Throw(_)
-                        | BasicBlockElement::Assignment(Register::Return, _) => {
-                            FoldWhile::Continue((acc.0, false))
-                        }
-                        BasicBlockElement::Assignment(_, _) => FoldWhile::Continue(acc),
+                    .fold_while((false, true), |acc, Instruction { kind, .. }| match kind {
+                        InstructionKind::Break(_) => FoldWhile::Done((true, false)),
+                        InstructionKind::Unreachable
+                        | InstructionKind::Throw
+                        | InstructionKind::Return(_) => FoldWhile::Continue((acc.0, false)),
+                        InstructionKind::Statement => FoldWhile::Continue(acc),
                     })
                     .into_inner();
 

crates/oxc_semantic/Cargo.toml

@@ -31,14 +31,14 @@ phf        = { workspace = true, features = ["macros"] }
 rustc-hash = { workspace = true }
 serde      = { workspace = true, features = ["derive"], optional = true }
 petgraph   = { workspace = true }
+itertools  = { workspace = true }
 
 tsify        = { workspace = true, optional = true }
 wasm-bindgen = { workspace = true, optional = true }
 
 [dev-dependencies]
 oxc_parser = { workspace = true }
 
-itertools  = { workspace = true }
 indexmap   = { workspace = true }
 insta      = { workspace = true, features = ["glob"] }
 phf        = { workspace = true, features = ["macros"] }

crates/oxc_semantic/examples/cfg.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, env, path::Path, sync::Arc};
 use itertools::Itertools;
 use oxc_allocator::Allocator;
 use oxc_parser::Parser;
-use oxc_semantic::{print_basic_block, SemanticBuilder};
+use oxc_semantic::{DebugDot, DisplayDot, EdgeType, SemanticBuilder};
 use oxc_span::SourceType;
 use petgraph::dot::{Config, Dot};
 
@@ -64,7 +64,7 @@ fn main() -> std::io::Result<()> {
         .cfg()
         .basic_blocks
         .iter()
-        .map(print_basic_block)
+        .map(DisplayDot::display_dot)
         .enumerate()
         .map(|(i, it)| {
             format!(
@@ -88,21 +88,37 @@ fn main() -> std::io::Result<()> {
         Dot::with_attr_getters(
             &semantic.semantic.cfg().graph,
             &[Config::EdgeNoLabel, Config::NodeNoLabel],
-            &|_graph, edge| format!("label = {:?}", edge.weight()),
-            &|_graph, node| format!(
-                "xlabel = {:?}, label = {:?}",
+            &|_graph, edge| {
+                let weight = edge.weight();
+                let label = format!("label = {weight:?}");
+                if matches!(weight, EdgeType::Unreachable) {
+                    format!("{label}, style = \"dotted\"")
+                } else {
+                    label
+                }
+            },
+            &|_graph, node| {
+                let nodes = ast_nodes_by_block.get(node.1).map_or("None".to_string(), |nodes| {
+                    let nodes: Vec<_> =
+                        nodes.iter().map(|node| format!("{}", node.kind().debug_name())).collect();
+                    if nodes.len() > 1 {
+                        format!(
+                            "{}\\l",
+                            nodes.into_iter().map(|it| format!("\\l    {it}")).join("")
+                        )
+                    } else {
+                        nodes.into_iter().join("")
+                    }
+                });
                 format!(
-                    "bb{} [{}]",
+                    "xlabel = \"nodes [{}]\\l\", label = \"bb{}\n{}\"",
+                    nodes,
                     node.1,
-                    print_basic_block(&semantic.semantic.cfg().basic_blocks[*node.1],).trim()
-                ),
-                ast_nodes_by_block
-                    .get(node.1)
-                    .map(|nodes| {
-                        nodes.iter().map(|node| format!("{}", node.kind().debug_name())).join("\n")
-                    })
-                    .unwrap_or_default()
-            )
+                    semantic.semantic.cfg().basic_blocks[*node.1]
+                        .debug_dot(semantic.semantic.nodes().into())
+                        .trim()
+                )
+            }
         )
     );
     std::fs::write(dot_file_path, cfg_dot_diagram)?;