Lior/flow control

crates/cairo-lang-lowering/src/borrow_check/test_data/borrow_check

@@ -995,20 +995,41 @@ End:
 
 blk1:
 Statements:
-  (v2: ()) <- test::foo(v1{`x`})
+  (v2: core::option::Option::<test::NonDrop>) <- Option::Some(v1{`maybe_consume(x)`})
 End:
-  Goto(blk3, {v2 -> v3})
+  Goto(blk3, {v2 -> v5})
 
 blk2:
 Statements:
-  (v4: ()) <- struct_construct()
+  (v3: ()) <- struct_construct()
+  (v4: core::option::Option::<test::NonDrop>) <- Option::None(v3{`maybe_consume(x)`})
 End:
-  Goto(blk3, {v4 -> v3})
+  Goto(blk3, {v4 -> v5})
 
 blk3:
 Statements:
 End:
-  Return(v3)
+  Match(match_enum(v5{`maybe_consume(x)`}) {
+    Option::Some(v6) => blk4,
+    Option::None(v7) => blk5,
+  })
+
+blk4:
+Statements:
+  (v8: ()) <- test::foo(v6{`x`})
+End:
+  Goto(blk6, {v8 -> v9})
+
+blk5:
+Statements:
+  (v10: ()) <- struct_construct()
+End:
+  Goto(blk6, {v10 -> v9})
+
+blk6:
+Statements:
+End:
+  Return(v9)
 
 //! > lowering_diagnostics
 error: Variable not dropped.

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,6 +9,7 @@ use super::graph::{
     ArmExpr, BooleanIf, EvaluateExpr, FlowControlGraph, FlowControlGraphBuilder, FlowControlNode,
     NodeId,
 };
+use crate::diagnostic::MatchKind;
 use crate::lower::context::LoweringContext;
 
 mod cache;
@@ -17,10 +18,10 @@ mod patterns;
 
 /// Creates a graph node for [semantic::ExprIf].
 pub fn create_graph_expr_if<'db>(
-    ctx: &LoweringContext<'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 }));
@@ -102,16 +103,16 @@ pub fn create_graph_expr_if<'db>(
         }
     }
 
-    graph.finalize(current_node)
+    graph.finalize(current_node, ctx)
 }
 
 /// Creates a graph node for [semantic::ExprMatch].
 #[allow(dead_code)]
 pub fn create_graph_expr_match<'db>(
-    ctx: &LoweringContext<'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());
@@ -164,5 +165,5 @@ pub fn create_graph_expr_match<'db>(
         next: match_node_id,
     }));
 
-    graph.finalize(root)
+    graph.finalize(root, ctx)
 }

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

@@ -302,7 +302,9 @@ fn create_node_for_tuple_inner<'db>(
     )
 }
 
-/// Creates a node for matching over numeric values, using [EqualsLiteral] nodes.
+/// Creates a node for matching over numeric values using a combination of [EnumMatch] and
+/// [BooleanIf] nodes.
+// TODO(lior): Fix doc if needed.
 fn create_node_for_value<'db>(
     params: CreateNodeParams<'db, '_, '_>,
     input_var: FlowControlVar,

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

@@ -21,12 +21,18 @@
 
 use std::fmt::Debug;
 
+use cairo_lang_diagnostics::DiagnosticAdded;
 use cairo_lang_semantic::{self as semantic, ConcreteVariant, PatternVariable};
 use cairo_lang_syntax::node::ast::ExprPtr;
+use cairo_lang_syntax::node::ids::SyntaxStablePtrId;
 use cairo_lang_utils::unordered_hash_set::UnorderedHashSet;
 use itertools::Itertools;
 
+use crate::diagnostic::{
+    LoweringDiagnosticKind, LoweringDiagnostics, LoweringDiagnosticsBuilder, MatchKind,
+};
 use crate::ids::LocationId;
+use crate::lower::context::LoweringContext;
 
 /// Represents a variable in the flow control graph.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
@@ -228,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.
@@ -245,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> {
@@ -260,9 +274,27 @@ impl<'db> Debug for FlowControlGraph<'db> {
 pub struct FlowControlGraphBuilder<'db> {
     graph: FlowControlGraph<'db>,
     used_vars: UnorderedHashSet<FlowControlVar>,
+    /// Current emitted diagnostics.
+    diagnostics: LoweringDiagnostics<'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.
@@ -280,8 +312,15 @@ impl<'db> FlowControlGraphBuilder<'db> {
     }
 
     /// Finalizes the graph and returns the final [FlowControlGraph].
-    pub fn finalize(self, root: NodeId) -> FlowControlGraph<'db> {
+    ///
+    /// Adds the reported diagnostics to the context.
+    pub fn finalize(
+        self,
+        root: NodeId,
+        ctx: &mut LoweringContext<'db, '_>,
+    ) -> FlowControlGraph<'db> {
         assert_eq!(root.0, self.graph.size() - 1, "The root must be the last node.");
+        ctx.diagnostics.extend(self.diagnostics.build());
         self.graph
     }
 
@@ -308,16 +347,14 @@ impl<'db> FlowControlGraphBuilder<'db> {
     pub fn var_ty(&self, input_var: FlowControlVar) -> semantic::TypeId<'db> {
         self.graph.var_types[input_var.0]
     }
-}
 
-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() }
+    /// Reports a diagnostic.
+    #[expect(dead_code)]
+    pub fn report(
+        &mut self,
+        stable_ptr: impl Into<SyntaxStablePtrId<'db>>,
+        kind: LoweringDiagnosticKind<'db>,
+    ) -> DiagnosticAdded {
+        self.diagnostics.report(stable_ptr, kind)
     }
 }

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

@@ -59,35 +59,40 @@ fn test_create_graph(
     let mut encapsulating_ctx =
         create_encapsulating_ctx(db, test_function.function_id, &test_function.signature);
 
-    let ctx = create_lowering_context(
+    let mut ctx = create_lowering_context(
         db,
         test_function.function_id,
         &test_function.signature,
         &mut encapsulating_ctx,
     );
 
     let graph = match &expr {
-        semantic::Expr::If(expr) => create_graph_expr_if(&ctx, expr),
-        semantic::Expr::Match(expr) => create_graph_expr_match(&ctx, expr),
+        semantic::Expr::If(expr) => create_graph_expr_if(&mut ctx, expr),
+        semantic::Expr::Match(expr) => create_graph_expr_match(&mut ctx, expr),
         _ => {
             panic!("Unsupported expression: {:?}", expr.debug(&expr_formatter));
         }
     };
 
-    let error = verify_diagnostics_expectation(args, &semantic_diagnostics);
-
     // Lower the graph.
-    let lowered_str = if args.get("skip_lowering").unwrap_or(&"false".into()) == "true" {
-        "".into()
-    } else {
-        let lowered = lower_graph_as_function(ctx, expr_id, &graph);
-        formatted_lowered(db, Some(&lowered))
-    };
+    let (lowered_str, lowering_diagnostics) =
+        if args.get("skip_lowering").unwrap_or(&"false".into()) == "true" {
+            ("".into(), ctx.diagnostics.build().format(db))
+        } else {
+            let lowered = lower_graph_as_function(ctx, expr_id, &graph);
+            (formatted_lowered(db, Some(&lowered)), lowered.diagnostics.format(db))
+        };
+
+    let error = verify_diagnostics_expectation(
+        args,
+        &format!("{semantic_diagnostics}{lowering_diagnostics}"),
+    );
 
     TestRunnerResult {
         outputs: OrderedHashMap::from([
             ("graph".into(), format!("{graph:?}")),
             ("semantic_diagnostics".into(), semantic_diagnostics),
+            ("lowering_diagnostics".into(), lowering_diagnostics),
             ("lowered".into(), lowered_str),
         ]),
         error,

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

@@ -6,12 +6,12 @@ use cairo_lang_utils::unordered_hash_map::UnorderedHashMap;
 use super::graph::{FlowControlGraph, FlowControlVar, NodeId};
 use crate::ids::LocationId;
 use crate::lower::block_builder::{
-    BlockBuilder, SealedBlockBuilder, SealedGotoCallsite, merge_block_builders,
-    merge_sealed_block_builders,
+    BlockBuilder, SealedGotoCallsite, merge_block_builders, merge_sealed_block_builders,
 };
 use crate::lower::context::{
     LoweredExpr, LoweringContext, LoweringFlowError, LoweringResult, handle_lowering_flow_error,
 };
+use crate::lower::lowered_expr_to_block_scope_end;
 use crate::{BlockEnd, BlockId, MatchInfo, VarUsage};
 
 mod lower_node;
@@ -42,6 +42,18 @@ pub fn lower_graph<'db, 'mt>(
     res
 }
 
+/// Information about the final result of the lowering of the graph.
+enum LowerGraphResult<'db> {
+    /// The result is the given expression.
+    Expr(LoweredExpr<'db>),
+    /// The graph is finalized using the given [MatchInfo] on the sealed blocks from all the arms.
+    /// In this case, the final [LoweringResult] may be `Ok` or [LoweringFlowError::Match] (if
+    /// all arms panic or return).
+    Match(MatchInfo<'db>),
+    /// The flow does not continue to the callsite (for example, if all the arms panic or return).
+    Error(LoweringFlowError<'db>),
+}
+
 /// Helper struct for the lowering of a flow control graph.
 struct LowerGraphContext<'db, 'mt, 'a> {
     /// The lowering context.
@@ -50,7 +62,7 @@ struct LowerGraphContext<'db, 'mt, 'a> {
     graph: &'a FlowControlGraph<'db>,
     /// The [BlockBuilder] for the result of the lowering, and the [MatchInfo] for its
     /// finalization.
-    result: Option<(BlockBuilder<'db>, LoweringResult<'db, MatchInfo<'db>>)>,
+    result: Option<(BlockBuilder<'db>, LowerGraphResult<'db>)>,
     /// A map from [NodeId] to all the [BlockBuilder]s that lead to it.
     /// When a node is visited, it creates a [BlockBuilder] for each of its child nodes
     /// (see [Self::assign_child_block_id]) and adds it to the map.
@@ -160,7 +172,7 @@ impl<'mt, 'db, 'a> LowerGraphContext<'db, 'mt, 'a> {
         info: MatchInfo<'db>,
     ) {
         if id == self.effective_root {
-            self.result = Some((builder, Ok(info)));
+            self.result = Some((builder, LowerGraphResult::Match(info)));
         } else {
             builder.finalize(self.ctx, BlockEnd::Match { info });
         }
@@ -175,23 +187,34 @@ impl<'mt, 'db, 'a> LowerGraphContext<'db, 'mt, 'a> {
         err: LoweringFlowError<'db>,
     ) -> Maybe<()> {
         if id == self.effective_root {
-            self.result = Some((builder, Err(err)));
+            self.result = Some((builder, LowerGraphResult::Error(err)));
         } else {
             handle_lowering_flow_error(self.ctx, builder, err)?;
         }
         Ok(())
     }
 
-    /// Adds a sealed block to the context.
-    ///
-    /// The block is an arm's block that was already sealed and should be merged with the other
-    /// arms.
-    fn add_sealed_block(&mut self, sealed_block: SealedBlockBuilder<'db>) {
-        // If sealed_block is `None`, ignore it as it doesn't return to the callsite and therefore
-        // should not be merged.
-        if let Some(sealed_block) = sealed_block {
-            self.sealed_blocks.push(sealed_block);
+    /// Finalizes an arm block that ends with the given `lowering_result`.
+    fn finalize_with_arm(
+        &mut self,
+        id: NodeId,
+        builder: BlockBuilder<'db>,
+        lowering_result: LoweringResult<'db, LoweredExpr<'db>>,
+    ) -> Maybe<()> {
+        if id == self.effective_root {
+            let lower_graph_result = match lowering_result {
+                Ok(lowered_expr) => LowerGraphResult::Expr(lowered_expr),
+                Err(err) => LowerGraphResult::Error(err),
+            };
+            self.result = Some((builder, lower_graph_result));
+        } else {
+            let sealed_block = lowered_expr_to_block_scope_end(self.ctx, builder, lowering_result)?;
+            if let Some(sealed_block) = sealed_block {
+                self.sealed_blocks.push(sealed_block);
+            }
         }
+
+        Ok(())
     }
 
     /// Finalizes the lowering of the graph.
@@ -202,7 +225,7 @@ impl<'mt, 'db, 'a> LowerGraphContext<'db, 'mt, 'a> {
         let (builder, match_info) = self.result.take().unwrap();
 
         match match_info {
-            Ok(match_info) => {
+            LowerGraphResult::Match(match_info) => {
                 if let Some((new_builder, lowered_expr)) = merge_sealed_block_builders(
                     self.ctx,
                     self.sealed_blocks,
@@ -215,7 +238,8 @@ impl<'mt, 'db, 'a> LowerGraphContext<'db, 'mt, 'a> {
                     (Err(LoweringFlowError::Match(match_info)), builder)
                 }
             }
-            Err(err) => (Err(err), builder),
+            LowerGraphResult::Error(err) => (Err(err), builder),
+            LowerGraphResult::Expr(lowered_expr) => (Ok(lowered_expr), builder),
         }
     }
 }