refactor: keep if-let native instead of desugaring (#801)

Author: ivov

crates/emit/src/control_flow/propagation.rs

@@ -380,7 +380,10 @@ impl Planner<'_> {
             return Some(statements);
         }
 
-        if matches!(expression, Expression::If { .. } | Expression::Match { .. }) {
+        if matches!(
+            expression,
+            Expression::If { .. } | Expression::IfLet { .. } | Expression::Match { .. }
+        ) {
             let block = self.lower_branching_to_block(expression, &PlacePlan::Return, fx);
             statements.extend(block.statements);
             return Some(statements);

crates/emit/src/expressions/values.rs

@@ -190,7 +190,8 @@ impl Planner<'_> {
             | Expression::Function {
                 params, body, ty, ..
             } => ValuePlan::Operand(self.emit_lambda(params, body, ty, ctx, fx)),
-            Expression::Match { ty, .. }
+            Expression::IfLet { ty, .. }
+            | Expression::Match { ty, .. }
             | Expression::Select { ty, .. }
             | Expression::Block { ty, .. } => self.lower_to_operand_temp(expression, ty, fx),
             Expression::Return {
@@ -208,9 +209,6 @@ impl Planner<'_> {
                 vec![self.lower_assert_statement(expression, fx)],
                 "struct{}{}".to_string(),
             ),
-            Expression::IfLet { .. } => {
-                unreachable!("IfLet should be desugared to Match before emit")
-            }
             _ => unreachable!(
                 "unexpected leaf expression in plan_operand: {:?}",
                 expression

crates/emit/src/plan/lower.rs

@@ -13,9 +13,33 @@ use crate::plan::bodies::{
 use crate::plan::placement::{requires_temp_var, try_elide_tail_let};
 use crate::plan::values::{ValuePlan, value_plan_from_statements};
 use crate::utils::wrap_if_struct_literal;
-use syntax::ast::{BinaryOperator, Expression, Literal};
+use syntax::ast::{BinaryOperator, Expression, Literal, MatchArm, Pattern, TypedPattern};
 use syntax::types::Type;
 
+fn if_let_match_arms(
+    pattern: &Pattern,
+    typed_pattern: &Option<TypedPattern>,
+    consequence: &Expression,
+    alternative: &Expression,
+) -> Vec<MatchArm> {
+    vec![
+        MatchArm {
+            pattern: pattern.clone(),
+            guard: None,
+            typed_pattern: typed_pattern.clone(),
+            expression: Box::new(consequence.clone()),
+        },
+        MatchArm {
+            pattern: Pattern::WildCard {
+                span: alternative.get_span(),
+            },
+            guard: None,
+            typed_pattern: None,
+            expression: Box::new(alternative.clone()),
+        },
+    ]
+}
+
 impl Planner<'_> {
     /// Allocate a fresh operand-temp result var and its `var V T` declaration
     /// as a typed setup leaf. The control-flow that assigns it follows as a
@@ -67,7 +91,7 @@ impl Planner<'_> {
         value_plan_from_statements(vec![declaration, LoweredStatement::If(plan)], result_var)
     }
 
-    /// Lower a value-position `match`/`select` to a fresh operand-temp
+    /// Lower a value-position `if let`/`match`/`select` to a fresh operand-temp
     /// variable. Only valid for non-never result types; never-typed branches
     /// route through `lower_to_operand_temp` as a diverging statement.
     pub(crate) fn plan_branching_as_operand_temp(
@@ -309,6 +333,22 @@ impl Planner<'_> {
                     self.build_assignment_plan(target, value, compound_operator.as_ref(), fx);
                 directed(directive, LoweredStatement::Assign(plan))
             }
+            Expression::IfLet {
+                pattern,
+                scrutinee,
+                consequence,
+                alternative,
+                typed_pattern,
+                ..
+            } => {
+                let directive = self.maybe_line_directive(&expression.get_span());
+                let arms = if_let_match_arms(pattern, typed_pattern, consequence, alternative);
+                let body = self.lower_match_to_block(scrutinee, &arms, &PlacePlan::Statement, fx);
+                directed(
+                    directive,
+                    LoweredStatement::Match(MatchStatementPlan { body }),
+                )
+            }
             Expression::Match { subject, arms, .. } => {
                 let directive = self.maybe_line_directive(&expression.get_span());
                 let body = self.lower_match_to_block(subject, arms, &PlacePlan::Statement, fx);
@@ -701,7 +741,7 @@ impl Planner<'_> {
         LoweredBlock { statements }
     }
 
-    /// Lower a branching expression (`if`, `match`, `select`) into a
+    /// Lower a branching expression (`if`, `if let`, `match`, `select`) into a
     /// `LoweredBlock` targeting `place`. Centralises the dispatch shared by old
     /// emit paths that need to render a branching tail/assignment.
     pub(crate) fn lower_branching_to_block(
@@ -722,13 +762,24 @@ impl Planner<'_> {
                     statements: vec![LoweredStatement::If(plan)],
                 }
             }
+            Expression::IfLet {
+                pattern,
+                scrutinee,
+                consequence,
+                alternative,
+                typed_pattern,
+                ..
+            } => {
+                let arms = if_let_match_arms(pattern, typed_pattern, consequence, alternative);
+                self.lower_match_to_block(scrutinee, &arms, place, fx)
+            }
             Expression::Match { subject, arms, .. } => {
                 self.lower_match_to_block(subject, arms, place, fx)
             }
             Expression::Select { arms, .. } => LoweredBlock {
                 statements: vec![LoweredStatement::Select(self.lower_select(arms, place, fx))],
             },
-            _ => unreachable!("lower_branching_to_block: expected if/match/select"),
+            _ => unreachable!("lower_branching_to_block: expected if/if-let/match/select"),
         }
     }
 
@@ -795,7 +846,7 @@ impl Planner<'_> {
     /// Lower a single tail expression in return position to its return
     /// statements. Shared by branch-arm return lowering and function-body
     /// lowering; only leaf values and lowered-ABI returns become `RawGo`,
-    /// `if`/`match`/`select` tails recurse structurally with a `Return` place.
+    /// `if`/`if let`/`match`/`select` tails recurse structurally with a `Return` place.
     pub(crate) fn lower_return_tail(
         &mut self,
         last: &Expression,
@@ -832,6 +883,21 @@ impl Planner<'_> {
                     self.lower_if(condition, consequence, alternative, &PlacePlan::Return, fx);
                 statements.push(directed(directive, LoweredStatement::If(plan)));
             }
+            Expression::IfLet {
+                pattern,
+                scrutinee,
+                consequence,
+                alternative,
+                typed_pattern,
+                ..
+            } => {
+                if !directive.is_empty() {
+                    statements.push(LoweredStatement::RawGo(directive));
+                }
+                let arms = if_let_match_arms(pattern, typed_pattern, consequence, alternative);
+                let block = self.lower_match_to_block(scrutinee, &arms, &PlacePlan::Return, fx);
+                statements.extend(block.statements);
+            }
             Expression::Match { subject, arms, .. } => {
                 if !directive.is_empty() {
                     statements.push(LoweredStatement::RawGo(directive));

crates/emit/src/plan/placement.rs

@@ -108,12 +108,12 @@ pub(crate) fn try_elide_tail_let(items: &[Expression]) -> Option<(&Expression, &
     if identifier != tail_name {
         return None;
     }
-    // Only `If` and `Match` can be re-emitted at the surrounding place via
-    // branch lowering (`lower_branching_to_block`); other shapes still stage
+    // Only `If`, `IfLet`, and `Match` can be re-emitted at the surrounding place
+    // via branch lowering (`lower_branching_to_block`); other shapes still stage
     // through temps so eliding the let would not save anything.
     if !matches!(
         value.as_ref(),
-        Expression::If { .. } | Expression::Match { .. }
+        Expression::If { .. } | Expression::IfLet { .. } | Expression::Match { .. }
     ) {
         return None;
     }
@@ -152,6 +152,16 @@ pub(crate) fn expression_contains_binding(expression: &Expression, name: &str) -
         }
     }
     match expression {
+        Expression::IfLet {
+            pattern,
+            consequence,
+            alternative,
+            ..
+        } => {
+            pattern_contains_name(pattern, name)
+                || expression_contains_binding(consequence, name)
+                || expression_contains_binding(alternative, name)
+        }
         Expression::Match { arms, .. } => arms
             .iter()
             .any(|arm| pattern_contains_name(&arm.pattern, name)),
@@ -475,7 +485,10 @@ impl Planner<'_> {
         }
         if matches!(
             last,
-            Expression::If { .. } | Expression::Match { .. } | Expression::Select { .. }
+            Expression::If { .. }
+                | Expression::IfLet { .. }
+                | Expression::Match { .. }
+                | Expression::Select { .. }
         ) {
             let place = PlacePlan::Assign {
                 local: var,

crates/emit/src/plan/values.rs

@@ -143,7 +143,11 @@ impl Planner<'_> {
             }
             Expression::If { ty, .. } => self.plan_if_as_operand_temp(expression, ty, fx),
             Expression::Loop { ty, .. } => self.plan_loop_as_operand_temp(expression, ty, fx),
-            Expression::Match { ty, .. } | Expression::Select { ty, .. } if !ty.is_never() => {
+            Expression::IfLet { ty, .. }
+            | Expression::Match { ty, .. }
+            | Expression::Select { ty, .. }
+                if !ty.is_never() =>
+            {
                 self.plan_branching_as_operand_temp(expression, ty, fx)
             }
             Expression::Call { ty, .. } => match self.classify_call(expression) {

crates/emit/src/statements/let_binding.rs

@@ -138,7 +138,10 @@ fn resolve_let_temp_declaration_ty(
     };
     let is_branching = matches!(
         value,
-        Expression::If { .. } | Expression::Match { .. } | Expression::Select { .. }
+        Expression::If { .. }
+            | Expression::IfLet { .. }
+            | Expression::Match { .. }
+            | Expression::Select { .. }
     );
     if is_branching && let Type::Tuple(slots) = &base {
         Type::Tuple(planner.resolve_tuple_slot_types(slots.clone(), false))

crates/passes/src/passes/checks/pattern_analysis/mod.rs

@@ -23,9 +23,7 @@ use std::cell::RefCell;
 use diagnostics::{IssueKind, LocalSink, PatternIssue};
 use semantics::context::AnalysisContext;
 use semantics::store::Store;
-use syntax::ast::{
-    Expression, Literal, MatchOrigin, Pattern, SelectArmPattern, Span, TypedPattern,
-};
+use syntax::ast::{Expression, Literal, Pattern, SelectArmPattern, Span, TypedPattern};
 use syntax::types::Type;
 
 use maranget::is_useful;
@@ -171,14 +169,24 @@ pub fn check(expression: &Expression, ctx: &PatternAnalysisContext, sink: &Local
             check(alternative, ctx, sink);
         }
 
-        Expression::IfLet { .. } => {
-            unreachable!("IfLet should be desugared to Match before pattern analysis")
+        Expression::IfLet {
+            pattern,
+            scrutinee,
+            consequence,
+            alternative,
+            typed_pattern,
+            else_span,
+            ..
+        } => {
+            check(scrutinee, ctx, sink);
+            check_if_let(pattern, typed_pattern, alternative, *else_span, ctx);
+            check(consequence, ctx, sink);
+            check(alternative, ctx, sink);
         }
 
         Expression::Match {
             subject,
             arms,
-            origin,
             span,
             ..
         } => {
@@ -212,9 +220,7 @@ pub fn check(expression: &Expression, ctx: &PatternAnalysisContext, sink: &Local
                 return;
             }
 
-            if let MatchOrigin::IfLet { else_span } = origin {
-                check_desugared_if_let(arms, *else_span, ctx);
-            } else if !check_redundancy_with_guards(arms, &mut unions, &norm_ctx, sink) {
+            if !check_redundancy_with_guards(arms, &mut unions, &norm_ctx, sink) {
                 return;
             }
 
@@ -462,41 +468,33 @@ fn check_redundancy_with_guards(
     true
 }
 
-fn check_desugared_if_let(
-    arms: &[syntax::ast::MatchArm],
+fn check_if_let(
+    pattern: &Pattern,
+    typed_pattern: &Option<TypedPattern>,
+    alternative: &Expression,
     else_span: Option<Span>,
     ctx: &PatternAnalysisContext,
 ) {
-    if arms.len() != 2 {
-        return;
-    }
-
-    let first_arm = &arms[0];
-    let second_arm = &arms[1];
-
-    let Some(tp) = &first_arm.typed_pattern else {
+    let Some(tp) = typed_pattern else {
         return;
     };
 
     // Suppress lints for patterns that already have or-pattern binding errors.
-    if ctx
-        .or_pattern_error_spans
-        .contains(&first_arm.pattern.get_span())
-    {
+    if ctx.or_pattern_error_spans.contains(&pattern.get_span()) {
         return;
     }
 
     if is_pattern_irrefutable(tp, ctx.store) {
-        ctx.add_issue(first_arm.pattern.get_span(), IssueKind::RedundantIfLet);
+        ctx.add_issue(pattern.get_span(), IssueKind::RedundantIfLet);
 
         if let Some(else_span) = else_span
-            && !is_trivial_expression(&second_arm.expression)
+            && !is_trivial_expression(alternative)
         {
             ctx.add_issue(else_span, IssueKind::UnreachableIfLetElse);
         }
     } else if let Some(else_span) = else_span
-        && is_trivial_expression(&second_arm.expression)
-        && !second_arm.expression.is_conditional()
+        && is_trivial_expression(alternative)
+        && !alternative.is_conditional()
     {
         ctx.add_issue(else_span, IssueKind::RedundantIfLetElse);
     }

crates/passes/src/passes/fact_producers/unused_expressions.rs

@@ -159,6 +159,11 @@ fn descend_discarded(
             consequence,
             alternative,
             ..
+        }
+        | Expression::IfLet {
+            consequence,
+            alternative,
+            ..
         } => {
             descend_discarded(consequence, mode, module_id, store, facts);
             descend_discarded(alternative, mode, module_id, store, facts);