refactor(cfg)!: make BasicBlock::unreachable private (#6321)

Protect `unreachable` property of basic blocks in preparation of upcoming refactors. This is technically a breaking change.
oxc-project · Oct 7, 2024 · 95e018f · 95e018f
1 parent fa4d505
commit 95e018f
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 12 deletions.
diff --git a/crates/oxc_cfg/src/block.rs b/crates/oxc_cfg/src/block.rs
@@ -6,7 +6,7 @@ pub type BasicBlockId = NodeIndex;
 #[derive(Debug, Clone)]
 pub struct BasicBlock {
     pub instructions: Vec<Instruction>,
-    pub unreachable: bool,
+    unreachable: bool,
 }
 
 impl BasicBlock {
@@ -17,6 +17,21 @@ impl BasicBlock {
     pub fn instructions(&self) -> &Vec<Instruction> {
         &self.instructions
     }
+
+    #[inline]
+    pub fn is_unreachable(&self) -> bool {
+        self.unreachable
+    }
+
+    #[inline]
+    pub fn mark_as_unreachable(&mut self) {
+        self.unreachable = true;
+    }
+
+    #[inline]
+    pub fn mark_as_reachable(&mut self) {
+        self.unreachable = false;
+    }
 }
 
 #[derive(Debug, Clone)]

diff --git a/crates/oxc_cfg/src/builder/mod.rs b/crates/oxc_cfg/src/builder/mod.rs
@@ -91,18 +91,18 @@ impl<'a> ControlFlowGraphBuilder<'a> {
 
     pub fn add_edge(&mut self, a: BasicBlockId, b: BasicBlockId, weight: EdgeType) {
         if matches!(weight, EdgeType::NewFunction) {
-            self.basic_block_mut(b).unreachable = false;
-        } else if matches!(weight, EdgeType::Unreachable) || self.basic_block(a).unreachable {
+            self.basic_block_mut(b).mark_as_reachable();
+        } else if matches!(weight, EdgeType::Unreachable) || self.basic_block(a).is_unreachable() {
             if self.graph.edges_directed(b, Direction::Incoming).count() == 0 {
-                self.basic_block_mut(b).unreachable = true;
+                self.basic_block_mut(b).mark_as_unreachable();
             }
         } else if !self
             .basic_block(b)
             .instructions()
             .iter()
             .any(|it| matches!(it, Instruction { kind: InstructionKind::Unreachable, .. }))
         {
-            self.basic_block_mut(b).unreachable = false;
+            self.basic_block_mut(b).mark_as_reachable();
         }
         self.graph.add_edge(a, b, weight);
     }
@@ -211,7 +211,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
         let current_node_ix = self.current_node_ix;
         let basic_block_with_unreachable_graph_ix = self.new_basic_block_normal();
         self.push_instruction(InstructionKind::Unreachable, None);
-        self.current_basic_block().unreachable = true;
+        self.current_basic_block().mark_as_unreachable();
         self.add_edge(
             current_node_ix,
             basic_block_with_unreachable_graph_ix,

diff --git a/crates/oxc_cfg/src/dot.rs b/crates/oxc_cfg/src/dot.rs
@@ -29,7 +29,7 @@ impl DisplayDot for ControlFlowGraph {
                     let mut attrs = Attrs::default().with("label", format!("{weight:?}"));
 
                     if matches!(weight, EdgeType::Unreachable)
-                        || self.basic_block(edge.source()).unreachable
+                        || self.basic_block(edge.source()).is_unreachable()
                     {
                         attrs += ("style", "dotted");
                     } else if matches!(weight, EdgeType::Error(_)) {

diff --git a/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs b/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs
@@ -295,7 +295,7 @@ impl Rule for NoFallthrough {
                 if tests.contains_key(&node) {
                     return (last_cond, true);
                 }
-                if cfg.basic_block(node).unreachable {
+                if cfg.basic_block(node).is_unreachable() {
                     return (None, false);
                 }
 

diff --git a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs
@@ -52,7 +52,7 @@ impl Rule for NoUnreachable {
         // prevent other reachable blocks from ever getting executed.
         let _: Control<()> = depth_first_search(graph, Some(root.cfg_id()), |event| {
             if let DfsEvent::Finish(node, _) = event {
-                let unreachable = cfg.basic_block(node).unreachable;
+                let unreachable = cfg.basic_block(node).is_unreachable();
                 unreachables[node.index()] = unreachable;
 
                 if !unreachable {

diff --git a/crates/oxc_semantic/examples/cfg.rs b/crates/oxc_semantic/examples/cfg.rs
@@ -115,7 +115,7 @@ fn main() -> std::io::Result<()> {
                 let weight = edge.weight();
                 let label = format!("label = \"{weight:?}\"");
                 if matches!(weight, EdgeType::Unreachable)
-                    || cfg.basic_block(edge.source()).unreachable
+                    || cfg.basic_block(edge.source()).is_unreachable()
                 {
                     format!("{label}, style = \"dotted\" ")
                 } else {

diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs
@@ -1528,7 +1528,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
                     cfg.add_edge(
                         finally_block_end_ix,
                         after_try_statement_block_ix,
-                        if cfg.basic_block(after_try_block_graph_ix).unreachable {
+                        if cfg.basic_block(after_try_block_graph_ix).is_unreachable() {
                             EdgeType::Unreachable
                         } else {
                             EdgeType::Join

diff --git a/crates/oxc_semantic/src/dot.rs b/crates/oxc_semantic/src/dot.rs
@@ -72,7 +72,7 @@ impl DebugDot for ControlFlowGraph {
                     }
                     let mut attrs = Attrs::from_iter([("label", format!("{weight:?}"))]);
                     if matches!(weight, EdgeType::Unreachable)
-                        || self.basic_block(edge.source()).unreachable
+                        || self.basic_block(edge.source()).is_unreachable()
                     {
                         attrs += ("style", "dotted");
                     }