flow_control: Add diagnostics for unreachable arms.

Author: liorgold2

crates/cairo-lang-lowering/src/diagnostic.rs

@@ -173,7 +173,7 @@ impl<'db> MatchError<'db> {
                 "Unreachable pattern arm.".into()
             }
             (MatchDiagnostic::UnreachableMatchArm, MatchKind::IfLet) => {
-                "Unreachable else clause.".into()
+                "Unreachable clause.".into()
             }
             (MatchDiagnostic::UnreachableMatchArm, MatchKind::WhileLet(_, _)) => {
                 unreachable!("While-let does not have two arms.")

crates/cairo-lang-lowering/src/lower/flow_control/create_graph.rs

@@ -9,7 +9,7 @@ use super::graph::{
     ArmExpr, BooleanIf, EvaluateExpr, FlowControlGraph, FlowControlGraphBuilder, FlowControlNode,
     NodeId,
 };
-use crate::lower::context::LoweringContext;
+use crate::{diagnostic::MatchKind, lower::context::LoweringContext};
 
 mod cache;
 mod filtered_patterns;
@@ -20,7 +20,7 @@ pub fn create_graph_expr_if<'db>(
     ctx: &mut LoweringContext<'db, '_>,
     expr: &semantic::ExprIf<'db>,
 ) -> FlowControlGraph<'db> {
-    let mut graph = FlowControlGraphBuilder::default();
+    let mut graph = FlowControlGraphBuilder::new(MatchKind::IfLet);
 
     // Add the `true` branch (the `if` block).
     let true_branch = graph.add_node(FlowControlNode::ArmExpr(ArmExpr { expr: expr.if_block }));
@@ -111,7 +111,7 @@ pub fn create_graph_expr_match<'db>(
     ctx: &mut LoweringContext<'db, '_>,
     expr: &semantic::ExprMatch<'db>,
 ) -> FlowControlGraph<'db> {
-    let mut graph = FlowControlGraphBuilder::default();
+    let mut graph = FlowControlGraphBuilder::new(MatchKind::Match);
 
     let matched_expr = &ctx.function_body.arenas.exprs[expr.matched_expr];
     let matched_expr_location = ctx.get_location(matched_expr.stable_ptr().untyped());

crates/cairo-lang-lowering/src/lower/flow_control/graph.rs

@@ -29,7 +29,7 @@ use cairo_lang_utils::unordered_hash_set::UnorderedHashSet;
 use itertools::Itertools;
 
 use crate::diagnostic::{
-    LoweringDiagnosticKind, LoweringDiagnostics, LoweringDiagnosticsBuilder,
+    LoweringDiagnosticKind, LoweringDiagnostics, LoweringDiagnosticsBuilder, MatchKind,
 };
 use crate::ids::LocationId;
 use crate::lower::context::LoweringContext;
@@ -234,6 +234,9 @@ pub struct FlowControlGraph<'db> {
     var_locations: Vec<LocationId<'db>>,
     /// The pattern variables used by the [BindVar] nodes in the graph.
     pattern_vars: Vec<PatternVariable<'db>>,
+    /// The kind of the expression being lowered.
+    /// This is used for diagnostic reporting.
+    kind: MatchKind<'db>,
 }
 impl<'db> FlowControlGraph<'db> {
     /// Returns the root node of the graph.
@@ -251,6 +254,11 @@ impl<'db> FlowControlGraph<'db> {
     pub fn node(&self, id: NodeId) -> &FlowControlNode<'db> {
         &self.nodes[id.0]
     }
+
+    /// Returns the kind of the expression being lowered.
+    pub fn kind(&self) -> MatchKind<'db> {
+        self.kind
+    }
 }
 
 impl<'db> Debug for FlowControlGraph<'db> {
@@ -271,6 +279,22 @@ pub struct FlowControlGraphBuilder<'db> {
 }
 
 impl<'db> FlowControlGraphBuilder<'db> {
+    /// Constructs a new [FlowControlGraphBuilder].
+    pub fn new(kind: MatchKind<'db>) -> Self {
+        let graph = FlowControlGraph {
+            nodes: Vec::new(),
+            var_types: Vec::new(),
+            var_locations: Vec::new(),
+            pattern_vars: Vec::new(),
+            kind,
+        };
+        Self {
+            graph,
+            used_vars: UnorderedHashSet::default(),
+            diagnostics: LoweringDiagnostics::default(),
+        }
+    }
+
     /// Adds a new node to the graph. Returns the new node's id.
     pub fn add_node(&mut self, node: FlowControlNode<'db>) -> NodeId {
         // Mark the input variable (if exists) as used.
@@ -334,19 +358,3 @@ impl<'db> FlowControlGraphBuilder<'db> {
         self.diagnostics.report(stable_ptr, kind)
     }
 }
-
-impl<'db> Default for FlowControlGraphBuilder<'db> {
-    fn default() -> Self {
-        let graph = FlowControlGraph {
-            nodes: Vec::new(),
-            var_types: Vec::new(),
-            var_locations: Vec::new(),
-            pattern_vars: Vec::new(),
-        };
-        Self {
-            graph,
-            used_vars: UnorderedHashSet::default(),
-            diagnostics: LoweringDiagnostics::default(),
-        }
-    }
-}

crates/cairo-lang-lowering/src/lower/flow_control/lower_graph/lower_node.rs

@@ -6,6 +6,7 @@ use cairo_lang_syntax::node::TypedStablePtr;
 use itertools::zip_eq;
 
 use super::LowerGraphContext;
+use crate::diagnostic::{LoweringDiagnosticKind, LoweringDiagnosticsBuilder, MatchDiagnostic, MatchError};
 use crate::ids::SemanticFunctionIdEx;
 use crate::lower::block_builder::BlockBuilder;
 use crate::lower::context::{LoweredExpr, VarRequest};
@@ -22,6 +23,18 @@ use crate::{MatchArm, MatchEnumInfo, MatchExternInfo, MatchInfo, VarUsage};
 /// Lowers the node with the given [NodeId].
 pub fn lower_node(ctx: &mut LowerGraphContext<'_, '_, '_>, id: NodeId) -> Maybe<()> {
     let Some(builder) = ctx.get_builder_if_reachable(id) else {
+        // If an [ArmExpr] node is unreachable, report an error.
+        // TODO(lior): If the main branch is unreachable, report an proper error.
+        if let FlowControlNode::ArmExpr(node) = ctx.graph.node(id) {
+            let stable_ptr = ctx.ctx.function_body.arenas.exprs[node.expr].stable_ptr();
+
+            let match_error = LoweringDiagnosticKind::MatchError(MatchError{
+                        kind: ctx.graph.kind(),
+                        error: MatchDiagnostic::UnreachableMatchArm,
+                    });
+            ctx.ctx.diagnostics.report(stable_ptr, match_error);
+        }
+
         return Ok(());
     };
 

crates/cairo-lang-lowering/src/lower/flow_control/test_data/if

@@ -306,7 +306,7 @@ End:
 //! > Test if with panic in condition
 
 //! > test_runner_name
-test_create_graph(expect_diagnostics: false)
+test_create_graph(expect_diagnostics: true)
 
 //! > function_code
 fn foo() -> felt252 {
@@ -329,6 +329,21 @@ Root: 3
 //! > semantic_diagnostics
 
 //! > lowering_diagnostics
+warning: Unreachable clause.
+ --> lib.cairo:4:12-6:5
+      } else {
+ ____________^
+|         1
+|     }
+|_____^
+
+warning: Unreachable clause.
+ --> lib.cairo:2:17-4:5
+      if panic!() {
+ _________________^
+|         0
+|     } else {
+|_____^
 
 //! > lowered
 Parameters:
@@ -547,7 +562,7 @@ End:
 //! > Test if let-chain with panic in condition
 
 //! > test_runner_name
-test_create_graph(expect_diagnostics: false)
+test_create_graph(expect_diagnostics: true)
 
 //! > function_code
 fn foo() -> felt252 {
@@ -572,6 +587,13 @@ Root: 5
 //! > semantic_diagnostics
 
 //! > lowering_diagnostics
+warning: Unreachable clause.
+ --> lib.cairo:2:42-4:5
+      if let Some(_) = Some(0) && panic!() {
+ __________________________________________^
+|         0
+|     } else {
+|_____^
 
 //! > lowered
 Parameters:
@@ -607,3 +629,46 @@ blk3:
 Statements:
 End:
   Return(v12)
+
+//! > ==========================================================================
+
+//! > Unreachable else clause.
+
+//! > test_runner_name
+test_create_graph(expect_diagnostics: true)
+
+//! > function_code
+fn foo(x: Option<felt252>) -> felt252 {
+    if let Some(_) | None = x {
+        0
+    } else {
+        1
+    }
+}
+
+//! > module_code
+
+//! > graph
+Root: 2
+0 ArmExpr { expr: ExprId(2) }
+1 ArmExpr { expr: ExprId(4) }
+2 EvaluateExpr { expr: ExprId(0), var_id: v0, next: NodeId(0) }
+
+//! > semantic_diagnostics
+
+//! > lowering_diagnostics
+warning: Unreachable clause.
+ --> lib.cairo:4:12-6:5
+      } else {
+ ____________^
+|         1
+|     }
+|_____^
+
+//! > lowered
+Parameters: v0: core::option::Option::<core::felt252>
+blk0 (root):
+Statements:
+  (v1: core::felt252) <- 0
+End:
+  Return(v1)

crates/cairo-lang-lowering/src/lower/flow_control/test_data/match

@@ -1,14 +1,15 @@
 //! > Simple match
 
 //! > test_runner_name
-test_create_graph(expect_diagnostics: false)
+test_create_graph(expect_diagnostics: true)
 
 //! > function_code
 fn foo(color: Color) -> felt252 {
     match color {
         Color::Red | Color::Green(_) => 1,
         Color::Red(_) | Color::Blue => 2,
-        _ => 3,
+        Color::Blue | Color::Green => 3,
+        _ => 4,
     }
 }
 
@@ -22,16 +23,21 @@ enum Color {
 }
 
 //! > graph
-Root: 4
+Root: 5
 0 ArmExpr { expr: ExprId(1) }
 1 ArmExpr { expr: ExprId(2) }
 2 ArmExpr { expr: ExprId(3) }
-3 EnumMatch { matched_var: v0, variants: (NodeId(0), v1), (NodeId(0), v2), (NodeId(1), v3), (NodeId(2), v4), (NodeId(2), v5)}
-4 EvaluateExpr { expr: ExprId(0), var_id: v0, next: NodeId(3) }
+3 ArmExpr { expr: ExprId(4) }
+4 EnumMatch { matched_var: v0, variants: (NodeId(0), v1), (NodeId(0), v2), (NodeId(1), v3), (NodeId(3), v4), (NodeId(3), v5)}
+5 EvaluateExpr { expr: ExprId(0), var_id: v0, next: NodeId(4) }
 
 //! > semantic_diagnostics
 
 //! > lowering_diagnostics
+warning: Unreachable pattern arm.
+ --> lib.cairo:12:39
+        Color::Blue | Color::Green => 3,
+                                      ^
 
 //! > lowered
 Parameters: v0: test::Color
@@ -74,7 +80,7 @@ End:
 
 blk6:
 Statements:
-  (v6: core::felt252) <- 3
+  (v6: core::felt252) <- 4
 End:
   Goto(blk8, {v6 -> v9})