refactor: update old usages of cfg.

Author: rzvxa

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;
 
@@ -229,49 +229,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 +262,13 @@ 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
+                        | InstructionKind::Jump { ..  } => {}
                     }
                 }
 

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

@@ -3,7 +3,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::{GetSpan, Span};
 
@@ -109,15 +109,11 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b
                 }
             }
 
-            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)
-                    {
-                        return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH);
-                    }
-                } else {
-                    // We don't care about other types of instructions.
+            for entry in cfg.basic_block(*basic_block_id).instructions() {
+                if let InstructionKind::Return(ReturnInstructionKind::NotImplicitUndefined) =
+                    entry.kind
+                {
+                    return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH);
                 }
             }
 

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;
@@ -314,15 +314,16 @@ 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))
+                    .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::Jump { .. } | InstructionKind::Statement => {
+                            FoldWhile::Continue(acc)
                         }
-                        BasicBlockElement::Assignment(_, _) => FoldWhile::Continue(acc),
                     })
                     .into_inner();
 

crates/oxc_semantic/src/builder.rs

@@ -18,7 +18,7 @@ use crate::{
     class::ClassTableBuilder,
     control_flow::{
         ControlFlowGraphBuilder, EdgeType, Instruction, InstructionKind, Register,
-        StatementControlFlowType,
+        ReturnInstructionKind, StatementControlFlowType,
     },
     diagnostics::redeclaration,
     jsdoc::JSDocBuilder,
@@ -1150,11 +1150,20 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
         if let Some(arg) = &stmt.argument {
             self.visit_expression(arg);
-        }
-
-        /* cfg */
-        self.cfg.push_instruction(Instruction::new(InstructionKind::Return, Some(node_id)));
+            /* cfg */
+            self.cfg.push_instruction(Instruction::new(
+                InstructionKind::Return(ReturnInstructionKind::NotImplicitUndefined),
+                Some(node_id),
+            ));
         /* cfg */
+        } else {
+            /* cfg */
+            self.cfg.push_instruction(Instruction::new(
+                InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined),
+                Some(node_id),
+            ));
+            /* cfg */
+        }
 
         /* cfg - put unreachable after return */
         let _ = self.cfg.new_basic_block();

crates/oxc_semantic/src/control_flow/builder.rs

@@ -102,8 +102,8 @@ impl ControlFlowGraphBuilder {
     }
 
     pub fn put_break(&mut self, label: Option<AstNodeId>) {
-        todo!("{:?}", label);
-        // self.push_instruction(Instruction { kind: InstructionKind::Break, node_id: label });
+        self.push_instruction(Instruction { kind: InstructionKind::Break, node_id: label });
+        // todo!("{:?}", label);
         // self.current_basic_block().push(BasicBlockElement::Break(label));
     }
 

crates/oxc_semantic/src/control_flow/mod.rs

@@ -116,6 +116,10 @@ impl BasicBlock {
     fn new() -> Self {
         BasicBlock { instructions: Vec::new() }
     }
+
+    pub fn instructions(&self) -> &Vec<Instruction> {
+        &self.instructions
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -135,8 +139,15 @@ pub enum InstructionKind {
     Unreachable,
     Statement,
     Jump { conditional: bool, block: BasicBlockId },
-    Return,
+    Return(ReturnInstructionKind),
     Throw,
+    Break,
+}
+
+#[derive(Debug, Clone)]
+pub enum ReturnInstructionKind {
+    ImplicitUndefined,
+    NotImplicitUndefined,
 }
 
 #[derive(Debug, Clone)]
@@ -189,40 +200,23 @@ pub struct PreservedExpressionState {
 }
 
 #[must_use]
-fn print_register(register: Register) -> String {
-    match &register {
-        Register::Index(i) => format!("${i}"),
-        Register::Return => "$return".into(),
-    }
-}
-
-#[must_use]
-pub fn print_basic_block(basic_block_elements: &Vec<BasicBlockElement>) -> String {
+pub fn print_basic_block(basic_block: &BasicBlock) -> String {
     let mut output = String::new();
-    for basic_block in basic_block_elements {
-        match basic_block {
-            BasicBlockElement::Unreachable => output.push_str("Unreachable()\n"),
-            BasicBlockElement::Throw(reg) => {
-                output.push_str(&format!("throw {}\n", print_register(*reg)));
+    for instruction in basic_block.instructions() {
+        match instruction.kind {
+            InstructionKind::Statement => output.push_str("statement\n"),
+            InstructionKind::Unreachable => output.push_str("Unreachable()\n"),
+            InstructionKind::Throw => output.push_str("throw\n"),
+            InstructionKind::Break => output.push_str("break\n"),
+            InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined) => {
+                output.push_str("return <implicit undefined>\n");
             }
-
-            BasicBlockElement::Break(Some(reg)) => {
-                output.push_str(&format!("break {}\n", print_register(*reg)));
-            }
-            BasicBlockElement::Break(None) => {
-                output.push_str("break");
+            InstructionKind::Return(ReturnInstructionKind::NotImplicitUndefined) => {
+                output.push_str("return <value>\n");
             }
-            BasicBlockElement::Assignment(to, with) => {
-                output.push_str(&format!("{} = ", print_register(*to)));
-
-                match with {
-                    AssignmentValue::ImplicitUndefined => {
-                        output.push_str("<implicit undefined>");
-                    }
-                    AssignmentValue::NotImplicitUndefined => output.push_str("<value>"),
-                }
-
-                output.push('\n');
+            #[allow(clippy::todo)]
+            InstructionKind::Jump { .. } => {
+                todo!("We haven't switched to use jumps yet");
             }
         }
     }

crates/oxc_semantic/src/lib.rs

@@ -31,11 +31,11 @@ use rustc_hash::FxHashSet;
 
 pub use crate::{
     control_flow::{
-        print_basic_block, AssignmentValue, BasicBlockElement, BasicBlockId, BinaryAssignmentValue,
-        BinaryOp, CallType, CalleeWithArgumentsAssignmentValue, CollectionAssignmentValue,
-        ControlFlowGraph, EdgeType, Instruction, InstructionKind,
-        ObjectPropertyAccessAssignmentValue, Register, UnaryExpressioneAssignmentValue,
-        UpdateAssignmentValue,
+        print_basic_block, AssignmentValue, BasicBlock, BasicBlockElement, BasicBlockId,
+        BinaryAssignmentValue, BinaryOp, CallType, CalleeWithArgumentsAssignmentValue,
+        CollectionAssignmentValue, ControlFlowGraph, EdgeType, Instruction, InstructionKind,
+        ObjectPropertyAccessAssignmentValue, Register, ReturnInstructionKind,
+        UnaryExpressioneAssignmentValue, UpdateAssignmentValue,
     },
     node::{AstNode, AstNodeId, AstNodes},
     reference::{Reference, ReferenceFlag, ReferenceId},

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@break_from_a_label_in_global_scope.js-2.snap

@@ -5,7 +5,7 @@ input_file: crates/oxc_semantic/tests/cfg_fixtures/break_from_a_label_in_global_
 ---
 digraph {
     0 [ label = ""]
-    1 [ label = "Unreachable()\nbreak $0"]
+    1 [ label = "Unreachable()\nbreak"]
     0 -> 1 [ ]
     0 -> 1 [ ]
 }

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@break_from_a_label_in_global_scope.js.snap

@@ -9,5 +9,5 @@ bb0: {
 
 bb1: {
 	Unreachable()
-	break $0
+	break
 }

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@class_extend_super.js-2.snap

@@ -5,12 +5,11 @@ input_file: crates/oxc_semantic/tests/cfg_fixtures/class_extend_super.js
 ---
 digraph {
     0 [ label = ""]
-    1 [ label = "$return = <value>"]
+    1 [ label = "return <value>"]
     2 [ label = ""]
     3 [ label = "Unreachable()"]
     4 [ label = ""]
     0 -> 1 [ ]
     2 -> 3 [ ]
     0 -> 4 [ ]
 }
-

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@class_extend_super.js.snap

@@ -8,7 +8,7 @@ bb0: {
 }
 
 bb1: {
-	$return = <value>
+	return <value>
 }
 
 bb2: {

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@fn_return_obj_expr_with_computed_key.js-2.snap

@@ -5,12 +5,11 @@ input_file: crates/oxc_semantic/tests/cfg_fixtures/fn_return_obj_expr_with_compu
 ---
 digraph {
     0 [ label = ""]
-    1 [ label = "$return = <value>"]
+    1 [ label = "return <value>"]
     2 [ label = ""]
     3 [ label = "Unreachable()"]
     4 [ label = ""]
     0 -> 1 [ ]
     2 -> 3 [ ]
     0 -> 4 [ ]
 }
-

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@fn_return_obj_expr_with_computed_key.js.snap

@@ -8,7 +8,7 @@ bb0: {
 }
 
 bb1: {
-	$return = <value>
+	return <value>
 }
 
 bb2: {

crates/oxc_semantic/tests/snapshots/cfg__cfg_files@if_else.js-2.snap

@@ -8,14 +8,14 @@ digraph {
     1 [ label = ""]
     2 [ label = ""]
     3 [ label = ""]
-    4 [ label = "$return = <value>"]
+    4 [ label = "return <value>"]
     5 [ label = ""]
     6 [ label = "Unreachable()"]
-    7 [ label = "$return = <value>"]
+    7 [ label = "return <value>"]
     8 [ label = ""]
     9 [ label = "Unreachable()"]
     10 [ label = ""]
-    11 [ label = "Unreachable()\n$return = <value>"]
+    11 [ label = "Unreachable()\nreturn <value>"]
     12 [ label = ""]
     13 [ label = "Unreachable()"]
     14 [ label = ""]
@@ -33,4 +33,3 @@ digraph {
     12 -> 13 [ ]
     0 -> 14 [ ]
 }
-