From 94f1c4e84ea2504b6475b15ef58020e58c7dc293 Mon Sep 17 00:00:00 2001 From: Lior Goldberg Date: Sat, 13 Sep 2025 18:00:38 +0300 Subject: [PATCH] flow-control: Some cleanup. commit-id:29416fcc --- .../cairo-lang-lowering/src/lower/lower_if.rs | 206 +----------------- .../src/lower/lower_match.rs | 95 +++----- 2 files changed, 29 insertions(+), 272 deletions(-) diff --git a/crates/cairo-lang-lowering/src/lower/lower_if.rs b/crates/cairo-lang-lowering/src/lower/lower_if.rs index 4538cdd4de4..e2813654e6a 100644 --- a/crates/cairo-lang-lowering/src/lower/lower_if.rs +++ b/crates/cairo-lang-lowering/src/lower/lower_if.rs @@ -1,45 +1,10 @@ -use cairo_lang_debug::DebugWithDb; -use cairo_lang_diagnostics::Maybe; use cairo_lang_semantic as semantic; -use cairo_lang_semantic::corelib; use cairo_lang_syntax::node::TypedStablePtr; -use semantic::{Condition, MatchArmSelector}; -use super::block_builder::{BlockBuilder, SealedBlockBuilder}; -use super::context::{LoweredExpr, LoweringContext, LoweringFlowError, LoweringResult}; +use super::block_builder::BlockBuilder; +use super::context::{LoweredExpr, LoweringContext, LoweringResult}; use super::flow_control::create_graph::create_graph_expr_if; use super::flow_control::lower_graph::lower_graph; -use super::lowered_expr_to_block_scope_end; -use crate::diagnostic::LoweringDiagnosticKind::{self}; -use crate::diagnostic::{LoweringDiagnosticsBuilder, MatchDiagnostic, MatchError, MatchKind}; -use crate::ids::LocationId; -use crate::lower::context::VarRequest; -use crate::lower::lower_match::{self, MatchArmWrapper}; -use crate::lower::{create_subscope, lower_block, lower_expr, lower_expr_to_var_usage}; -use crate::{MatchArm, MatchEnumInfo, MatchInfo}; - -/// Represents an expression of the form: -/// -/// `if conditions[0] && conditions[1] && ... && conditions[n] { expr } else { else_block }` -/// -/// where `n` is `conditions.len() - 1`. -/// -/// In particular, note that if `conditions` is empty, there are no conditions and the -/// expression is simply [Self::expr]. -pub struct ConditionedExpr<'db, 'a> { - pub expr: semantic::ExprId, - pub conditions: &'a [Condition], - pub else_block: Option, - /// The location of the `if` expression. - pub if_expr_location: LocationId<'db>, -} - -impl ConditionedExpr<'_, '_> { - /// Returns a copy of self, without the first condition. - pub fn remove_first(&self) -> Self { - Self { conditions: &self.conditions[1..], ..*self } - } -} /// Lowers an expression of type [semantic::ExprIf]. pub fn lower_expr_if<'db>( @@ -50,170 +15,3 @@ pub fn lower_expr_if<'db>( let graph = create_graph_expr_if(ctx, expr); lower_graph(ctx, builder, &graph, ctx.get_location(expr.stable_ptr.untyped())) } - -/// Lowers an expression of type [semantic::ExprIf]. -pub fn lower_if_bool_condition<'db>( - ctx: &mut LoweringContext<'db, '_>, - builder: &mut BlockBuilder<'db>, - condition: semantic::ExprId, - inner_expr: ConditionedExpr<'db, '_>, -) -> LoweringResult<'db, LoweredExpr<'db>> { - // The condition cannot be unit. - let condition_var = lower_expr_to_var_usage(ctx, builder, condition)?; - let db = ctx.db; - let unit_ty = corelib::unit_ty(db); - - let if_expr_location = inner_expr.if_expr_location; - - // Main block. - let subscope_main = create_subscope(ctx, builder); - let block_main_id = subscope_main.block_id; - let main_block_var_id = ctx.new_var(VarRequest { ty: unit_ty, location: if_expr_location }); - let else_block_input_var_id = - ctx.new_var(VarRequest { ty: unit_ty, location: if_expr_location }); - - let block_main = lower_conditioned_expr_and_seal(ctx, subscope_main, &inner_expr) - .map_err(LoweringFlowError::Failed)?; - - // Else block. - let subscope_else = create_subscope(ctx, builder); - let block_else_id = subscope_else.block_id; - - let block_else = - lower_optional_else_block(ctx, subscope_else, inner_expr.else_block, if_expr_location) - .map_err(LoweringFlowError::Failed)?; - - let match_info = MatchInfo::Enum(MatchEnumInfo { - concrete_enum_id: corelib::core_bool_enum(db), - input: condition_var, - arms: vec![ - MatchArm { - arm_selector: MatchArmSelector::VariantId(corelib::false_variant(db)), - block_id: block_else_id, - var_ids: vec![else_block_input_var_id], - }, - MatchArm { - arm_selector: MatchArmSelector::VariantId(corelib::true_variant(db)), - block_id: block_main_id, - var_ids: vec![main_block_var_id], - }, - ], - location: if_expr_location, - }); - builder.merge_and_end_with_match( - ctx, - match_info, - vec![block_main, block_else], - if_expr_location, - ) -} - -/// Lowers an expression of type if where the condition is of type [semantic::Condition::Let]. -pub fn lower_if_let_condition<'db>( - ctx: &mut LoweringContext<'db, '_>, - builder: &mut BlockBuilder<'db>, - matched_expr_id: semantic::ExprId, - patterns: &[semantic::PatternId], - inner_expr: ConditionedExpr<'db, '_>, -) -> LoweringResult<'db, LoweredExpr<'db>> { - let matched_expr = &ctx.function_body.arenas.exprs[matched_expr_id]; - let stable_ptr = matched_expr.stable_ptr().untyped(); - let ty = matched_expr.ty(); - let location = ctx.get_location(stable_ptr); - - let lowered_expr = lower_expr(ctx, builder, matched_expr_id)?; - - if corelib::numeric_upcastable_to_felt252(ctx.db, ty) { - return Err(LoweringFlowError::Failed(ctx.diagnostics.report( - stable_ptr, - LoweringDiagnosticKind::MatchError(MatchError { - kind: MatchKind::IfLet, - error: MatchDiagnostic::UnsupportedNumericInLetCondition, - }), - ))); - } - - let else_arm = inner_expr - .else_block - .map(MatchArmWrapper::ElseClause) - .unwrap_or(MatchArmWrapper::DefaultClause); - let arms = vec![MatchArmWrapper::ConditionedArm(patterns, inner_expr), else_arm]; - - lower_match::lower_match_arms( - ctx, - builder, - matched_expr_id, - lowered_expr, - arms, - location, - MatchKind::IfLet, - ) -} - -/// Lowers a [ConditionedExpr] recursively by iterating over the conditions and calling -/// [lower_if_let_condition] or [lower_if_bool_condition]. -fn lower_conditioned_expr<'db>( - ctx: &mut LoweringContext<'db, '_>, - builder: &mut BlockBuilder<'db>, - expr: &ConditionedExpr<'db, '_>, -) -> LoweringResult<'db, LoweredExpr<'db>> { - log::trace!( - "Lowering a conditioned expression: {:?} (# of conditions: {})", - expr.expr.debug(&ctx.expr_formatter), - expr.conditions.len() - ); - - // If there are no more conditions, we can simply lower the expression. - if expr.conditions.is_empty() { - return lower_expr(ctx, builder, expr.expr); - } - - match &expr.conditions[0] { - Condition::Let(matched_expr_id, patterns) => { - lower_if_let_condition(ctx, builder, *matched_expr_id, patterns, expr.remove_first()) - } - Condition::BoolExpr(condition) => { - lower_if_bool_condition(ctx, builder, *condition, expr.remove_first()) - } - } -} - -/// Lowers a [ConditionedExpr] and seals the block. See [lower_conditioned_expr]. -pub fn lower_conditioned_expr_and_seal<'db>( - ctx: &mut LoweringContext<'db, '_>, - mut builder: BlockBuilder<'db>, - expr: &ConditionedExpr<'db, '_>, -) -> Maybe> { - let lowered_expr = lower_conditioned_expr(ctx, &mut builder, expr); - lowered_expr_to_block_scope_end(ctx, builder, lowered_expr) -} - -/// Lowers an optional else block. If the else block is missing it is replaced with a block -/// returning a unit. -/// Returns the sealed block builder of the else block. -fn lower_optional_else_block<'db>( - ctx: &mut LoweringContext<'db, '_>, - mut builder: BlockBuilder<'db>, - else_expr_opt: Option, - if_location: LocationId<'db>, -) -> Maybe> { - log::trace!("Started lowering of an optional else block."); - match else_expr_opt { - Some(else_expr) => { - let expr = ctx.function_body.arenas.exprs[else_expr].clone(); - match &expr { - semantic::Expr::Block(block) => lower_block(ctx, builder, block), - semantic::Expr::If(if_expr) => { - let lowered_if = lower_expr_if(ctx, &mut builder, if_expr); - lowered_expr_to_block_scope_end(ctx, builder, lowered_if) - } - _ => unreachable!(), - } - } - None => lowered_expr_to_block_scope_end( - ctx, - builder, - Ok(LoweredExpr::Tuple { exprs: vec![], location: if_location }), - ), - } -} diff --git a/crates/cairo-lang-lowering/src/lower/lower_match.rs b/crates/cairo-lang-lowering/src/lower/lower_match.rs index 729d77d29aa..b55ab9ec964 100644 --- a/crates/cairo-lang-lowering/src/lower/lower_match.rs +++ b/crates/cairo-lang-lowering/src/lower/lower_match.rs @@ -24,7 +24,6 @@ use super::context::{ LoweredExpr, LoweredExprExternEnum, LoweringContext, LoweringFlowError, LoweringResult, handle_lowering_flow_error, }; -use super::lower_if::{ConditionedExpr, lower_conditioned_expr_and_seal}; use super::{ alloc_empty_block, generators, lower_expr_block, lower_tail_expr, lowered_expr_to_block_scope_end, recursively_call_loop_func, @@ -52,32 +51,25 @@ struct ExtractedEnumDetails<'db> { /// A wrapper enum to provide a unified interface for handling different types of match arms /// during the lowering phase of the compiler, allowing for consistent pattern matching /// and expression evaluation across different match-like constructs. -pub enum MatchArmWrapper<'db, 'a> { +pub enum MatchArmWrapper<'a> { /// A match arm. Patterns (non-empty) guard the expression to evaluate. Arm(&'a [PatternId], semantic::ExprId), - /// Else clause (no patterns) and it's expression to evaluate (if-let). - ElseClause(semantic::ExprId), /// Default clause when else clause is not provided (if-let/while-let). DefaultClause, - /// Similar to [Self::Arm], except that the expression is a conditioned expression - /// (see [ConditionedExpr]). - ConditionedArm(&'a [PatternId], ConditionedExpr<'db, 'a>), } -impl<'db, 'a> From<&'a semantic::MatchArm> for MatchArmWrapper<'db, 'a> { +impl<'a> From<&'a semantic::MatchArm> for MatchArmWrapper<'a> { fn from(arm: &'a semantic::MatchArm) -> Self { MatchArmWrapper::Arm(&arm.patterns, arm.expression) } } -impl<'db> MatchArmWrapper<'db, '_> { +impl<'db> MatchArmWrapper<'_> { /// Returns the patterns of the match arm. pub fn patterns(&self) -> Option<&[PatternId]> { match self { - MatchArmWrapper::Arm(patterns, _) | MatchArmWrapper::ConditionedArm(patterns, _) => { - Some(patterns) - } - MatchArmWrapper::ElseClause(_) | MatchArmWrapper::DefaultClause => None, + MatchArmWrapper::Arm(patterns, _) => Some(patterns), + MatchArmWrapper::DefaultClause => None, } } @@ -164,7 +156,7 @@ struct PatternPath { /// Returns an option containing the PatternPath of the underscore pattern, if it exists. fn get_underscore_pattern_path_and_mark_unreachable<'db>( ctx: &mut LoweringContext<'db, '_>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], match_type: MatchKind<'db>, ) -> Option { let otherwise_variant = arms @@ -189,7 +181,7 @@ fn get_underscore_pattern_path_and_mark_unreachable<'db>( for arm in arms.iter().skip(otherwise_variant.arm_index + 1) { match arm { - MatchArmWrapper::Arm(patterns, _) | MatchArmWrapper::ConditionedArm(patterns, _) => { + MatchArmWrapper::Arm(patterns, _) => { for pattern in *patterns { let pattern_ptr = ctx.function_body.arenas.patterns[*pattern].stable_ptr(); ctx.diagnostics.report( @@ -202,16 +194,6 @@ fn get_underscore_pattern_path_and_mark_unreachable<'db>( } } MatchArmWrapper::DefaultClause => continue, - MatchArmWrapper::ElseClause(e) => { - let expr_ptr = ctx.function_body.arenas.exprs[*e].stable_ptr(); - ctx.diagnostics.report( - expr_ptr, - MatchError(MatchError { - kind: match_type, - error: MatchDiagnostic::UnreachableMatchArm, - }), - ); - } } } @@ -594,7 +576,7 @@ fn insert_tuple_path_patterns<'db>( /// Returns a map from a matching paths to their corresponding pattern path in a match statement. fn get_variants_to_arm_map_tuple<'db, 'a>( ctx: &mut LoweringContext<'db, '_>, - arms: impl Iterator>, + arms: impl Iterator>, extracted_enums_details: &[ExtractedEnumDetails<'db>], match_type: MatchKind<'db>, ) -> LoweringResult<'db, UnorderedHashMap, PatternPath>> @@ -668,7 +650,7 @@ struct LoweringMatchTupleContext<'db> { fn lower_tuple_match_arm<'db>( ctx: &mut LoweringContext<'db, '_>, mut builder: BlockBuilder<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], match_tuple_ctx: &mut LoweringMatchTupleContext<'db>, leaves_builders: &mut Vec>, match_type: MatchKind<'db>, @@ -759,7 +741,7 @@ fn lower_tuple_match_arm<'db>( fn lower_full_match_tree<'db>( ctx: &mut LoweringContext<'db, '_>, builder: &mut BlockBuilder<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], match_tuple_ctx: &mut LoweringMatchTupleContext<'db>, extracted_enums_details: &[ExtractedEnumDetails<'db>], leaves_builders: &mut Vec>, @@ -849,7 +831,7 @@ pub(crate) fn lower_expr_match_tuple<'db>( expr: LoweredExpr<'db>, matched_expr: semantic::ExprId, tuple_info: &TupleInfo<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], match_type: MatchKind<'db>, ) -> LoweringResult<'db, LoweredExpr<'db>> { let location = expr.location(); @@ -952,7 +934,7 @@ pub(crate) fn lower_match_arms<'db>( builder: &mut BlockBuilder<'db>, matched_expr: semantic::ExprId, lowered_expr: LoweredExpr<'db>, - arms: Vec>, + arms: Vec>, location: LocationId<'db>, match_type: MatchKind<'db>, ) -> LoweringResult<'db, LoweredExpr<'db>> { @@ -989,7 +971,7 @@ pub(crate) fn lower_concrete_enum_match<'db>( builder: &mut BlockBuilder<'db>, matched_expr: semantic::ExprId, lowered_matched_expr: LoweredExpr<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], location: LocationId<'db>, match_type: MatchKind<'db>, ) -> LoweringResult<'db, LoweredExpr<'db>> { @@ -1030,7 +1012,7 @@ pub(crate) fn lower_optimized_extern_match<'db>( ctx: &mut LoweringContext<'db, '_>, builder: &mut BlockBuilder<'db>, extern_enum: LoweredExprExternEnum<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], match_type: MatchKind<'db>, ) -> LoweringResult<'db, LoweredExpr<'db>> { log::trace!("Started lowering of an optimized extern match."); @@ -1103,7 +1085,7 @@ trait EnumVariantScopeBuilder<'db> { fn build_pattern_tree( &self, ctx: &mut LoweringContext<'db, '_>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], concrete_enum_id: semantic::ConcreteEnumId<'db>, match_type: MatchKind<'db>, ) -> LoweringResult<'db, PatternTree<'db>> { @@ -1139,20 +1121,7 @@ trait EnumVariantScopeBuilder<'db> { ); continue; } - MatchArmWrapper::ElseClause(e) => { - let ptr = ctx.function_body.arenas.exprs[*e].stable_ptr(); - try_push( - ctx, - match_type, - ptr, - PatternPath { arm_index, pattern_index: None }, - &mut pattern_tree, - None, - ); - continue; - } - MatchArmWrapper::Arm(patterns, _) - | MatchArmWrapper::ConditionedArm(patterns, _) => patterns, + MatchArmWrapper::Arm(patterns, _) => patterns, }; for (pattern_index, pattern) in patterns.iter().copied().enumerate() { let pattern_path = PatternPath { arm_index, pattern_index: Some(pattern_index) }; @@ -1343,10 +1312,7 @@ trait EnumVariantScopeBuilder<'db> { if concrete_variants.is_empty() { for arm in builder_context.arms { match arm { - MatchArmWrapper::Arm(_, expr) | - MatchArmWrapper::ConditionedArm(_, ConditionedExpr{expr, ..}) | - // Should actually never happen, as we can't if-let, but careful anyway. - MatchArmWrapper::ElseClause(expr) => { + MatchArmWrapper::Arm(_, expr) => { ctx.diagnostics.report( ctx.function_body.arenas.exprs[*expr].stable_ptr(), MatchError(MatchError { @@ -1354,7 +1320,7 @@ trait EnumVariantScopeBuilder<'db> { error: MatchDiagnostic::UnreachableMatchArm, }), ); - }, + } MatchArmWrapper::DefaultClause => (), } } @@ -1621,7 +1587,7 @@ struct MatchArmsLoweringContext<'db, 'a> { /// The match kind. kind: MatchKind<'db>, /// The match arms to lower. - arms: &'a [MatchArmWrapper<'db, 'a>], + arms: &'a [MatchArmWrapper<'a>], /// Empty match info to be used for lowering match arms. empty_match_info: MatchInfo<'db>, /// The location of the match expression. @@ -1633,7 +1599,7 @@ impl<'db, 'a> MatchArmsLoweringContext<'db, 'a> { fn new( builder: &'a mut BlockBuilder<'db>, kind: MatchKind<'db>, - match_arms: &'a [MatchArmWrapper<'db, 'a>], + match_arms: &'a [MatchArmWrapper<'a>], empty_match_info: MatchInfo<'db>, location: LocationId<'db>, ) -> Self { @@ -1664,7 +1630,7 @@ fn group_match_arms<'db>( ctx: &mut LoweringContext<'db, '_>, empty_match_info: &MatchInfo<'db>, location: LocationId<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], variants_block_builders: Vec>, kind: MatchKind<'db>, ) -> LoweringResult<'db, Vec>> { @@ -1694,7 +1660,7 @@ fn lower_match_arm_expr_and_seal_patterns<'db>( ctx: &mut LoweringContext<'db, '_>, empty_match_info: &MatchInfo<'db>, location: LocationId<'db>, - arms: &[MatchArmWrapper<'db, '_>], + arms: &[MatchArmWrapper<'_>], kind: MatchKind<'db>, arm_index: usize, group: impl IntoIterator>, @@ -1753,18 +1719,14 @@ fn lower_match_arm_expr_and_seal_patterns<'db>( fn lower_arm_expr_and_seal<'db>( ctx: &mut LoweringContext<'db, '_>, kind: MatchKind<'db>, - arm: &MatchArmWrapper<'db, '_>, + arm: &MatchArmWrapper<'_>, mut subscope: BlockBuilder<'db>, ) -> Maybe> { match (arm, kind) { - ( - MatchArmWrapper::Arm(_, expr) | MatchArmWrapper::ElseClause(expr), - MatchKind::IfLet | MatchKind::Match, - ) => lower_tail_expr(ctx, subscope, *expr), - ( - MatchArmWrapper::Arm(_, expr) | MatchArmWrapper::ElseClause(expr), - MatchKind::WhileLet(loop_expr_id, stable_ptr), - ) => { + (MatchArmWrapper::Arm(_, expr), MatchKind::IfLet | MatchKind::Match) => { + lower_tail_expr(ctx, subscope, *expr) + } + (MatchArmWrapper::Arm(_, expr), MatchKind::WhileLet(loop_expr_id, stable_ptr)) => { let semantic::Expr::Block(expr) = ctx.function_body.arenas.exprs[*expr].clone() else { unreachable!("WhileLet expression should be a block"); }; @@ -1776,9 +1738,6 @@ fn lower_arm_expr_and_seal<'db>( lowered_expr_to_block_scope_end(ctx, subscope, block_expr) } (MatchArmWrapper::DefaultClause, _) => Ok(subscope.goto_callsite(None)), - (MatchArmWrapper::ConditionedArm(_, expr), _) => { - lower_conditioned_expr_and_seal(ctx, subscope, expr) - } } }