Unrolled build for #162468 in rollup 162477

Rollup merge of #162468 - Zalathar:fallible-let, r=dianne

mir_build: Rename `lower_let_expr` to `lower_fallible_let`, and clarify

Calling this method `lower_let_expr` is misleading, as it is also used for lowering let-else statements, which don't contain a `thir::ExprKind::Let`.

This PR also:
- Rewords some related comments.
- Renames the method's `expr` parameter to the less-misleading `scrutinee_id`, since this parameter is the RHS expression being inspected.
- Reorders the `pat` and `scrutinee_id` parameters to match their order in normal Rust syntax.
- Renames the success/failure blocks to `true_block` and `false_block`, to match the convention used by #161861.

There should be no change to compiler behaviour.

Author: bors

compiler/rustc_mir_build/src/builder/block.rs

@@ -207,12 +207,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         let else_block_span = this.thir[*else_block].span;
                         let (true_block, false_block) =
                             this.in_if_then_scope(last_remainder_scope, else_block_span, |this| {
-                                // Bypass `lower_if_condition` and call `lower_let_expr` directly,
+                                // Bypass `lower_if_condition` and call `lower_fallible_let` directly,
                                 // since we don't have an actual THIR let-expression here.
-                                this.lower_let_expr(
+                                this.lower_fallible_let(
                                     block,
-                                    *initializer,
                                     pattern,
+                                    *initializer,
                                     None,
                                     initializer_span,
                                     DeclareLetBindings::No,

compiler/rustc_mir_build/src/builder/matches/mod.rs

@@ -50,7 +50,7 @@ pub(crate) struct LowerIfCondArgs {
     pub(crate) variable_source_info: SourceInfo,
     /// Determines how bindings should be handled when lowering `let` expressions.
     ///
-    /// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
+    /// Forwarded to [`Builder::lower_fallible_let`] when lowering [`ExprKind::Let`].
     pub(crate) declare_let_bindings: DeclareLetBindings,
 }
 
@@ -62,9 +62,9 @@ impl LowerIfCondArgs {
     }
 }
 
-/// Should lowering a `let` expression also declare its bindings?
+/// Should lowering a `let` also declare its bindings?
 ///
-/// Used by [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
+/// Used by [`Builder::lower_fallible_let`].
 #[derive(Clone, Copy)]
 pub(crate) enum DeclareLetBindings {
     /// Yes, declare `let` bindings as normal for `if` conditions.
@@ -169,10 +169,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 })
             }
             ExprKind::ValueExpr { source } => this.lower_if_condition(block, source, args),
-            ExprKind::Let { expr, ref pat } => this.lower_let_expr(
+            ExprKind::Let { ref pat, expr } => this.lower_fallible_let(
                 block,
-                expr,
                 pat,
+                expr,
                 Some(args.variable_source_info.scope),
                 args.variable_source_info.span,
                 args.declare_let_bindings,
@@ -2309,65 +2309,72 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 // Pat binding - used for `let` and function parameters as well.
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
-    /// Lowers a `let` expression that appears in a suitable context
-    /// (e.g. an `if` condition or match guard).
-    ///
-    /// Also used for lowering let-else statements, since they have similar
-    /// needs despite not actually using `let` expressions.
+    /// Lowers a fallible `let`, which is one of:
+    /// - A let-expression inside an `if` condition or match guard.
+    /// - A let-else statement.
     ///
-    /// Use [`DeclareLetBindings`] to control whether the `let` bindings are
-    /// declared or not.
+    /// (Strictly speaking, the underlying pattern might actually be infallible.
+    /// What matters here is that it is _allowed_ to be fallible.)
     ///
     /// Must be called within a [`Builder::in_if_then_scope`], to indicate where
     /// to break to if the `let` fails to match.
-    pub(crate) fn lower_let_expr(
+    pub(crate) fn lower_fallible_let(
         &mut self,
         mut block: BasicBlock,
-        expr_id: ExprId,
         pat: &Pat<'tcx>,
+        scrutinee_id: ExprId,
         source_scope: Option<SourceScope>,
         scope_span: Span,
+        // Controls whether bindings are declared or not, as requested by the caller.
         declare_let_bindings: DeclareLetBindings,
     ) -> BlockAnd<()> {
-        let expr_span = self.thir[expr_id].span;
-        let scrutinee = unpack!(block = self.lower_scrutinee(block, expr_id));
+        let scrutinee_span = self.thir[scrutinee_id].span;
+        let scrutinee_place_builder = unpack!(block = self.lower_scrutinee(block, scrutinee_id));
+
+        // Lower the scrutinee and pattern as though they were desugared to a `match`.
         let built_tree = self.lower_match_tree(
             block,
-            expr_span,
-            &scrutinee,
+            scrutinee_span,
+            &scrutinee_place_builder,
             pat.span,
             vec![(pat, HasMatchGuard::No)],
             Exhaustive::No,
         );
-        let [branch] = built_tree.branches.try_into().unwrap();
+        let [true_branch] = built_tree.branches.try_into().unwrap();
+        let false_block = built_tree.otherwise_block;
 
         // If pattern-matching failed, break out of the enclosing if-then scope.
-        self.break_from_if_then_scope(built_tree.otherwise_block, self.source_info(expr_span));
+        self.break_from_if_then_scope(false_block, self.source_info(scrutinee_span));
 
         match declare_let_bindings {
             DeclareLetBindings::Yes => {
-                let expr_place = scrutinee.try_to_place(self);
-                let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span));
+                let scrutinee_place;
+                let opt_match_place = try {
+                    scrutinee_place = scrutinee_place_builder.try_to_place(self)?;
+                    (Some(&scrutinee_place), scrutinee_span)
+                };
                 self.declare_bindings(
                     source_scope,
                     pat.span.to(scope_span),
                     pat,
                     None,
-                    opt_expr_place,
+                    opt_match_place,
                 );
             }
             DeclareLetBindings::No => {} // Caller is responsible for bindings.
-            DeclareLetBindings::LetNotPermitted => {
-                self.tcx.dcx().span_bug(expr_span, "let expression not expected in this context")
-            }
+            DeclareLetBindings::LetNotPermitted => self
+                .tcx
+                .dcx()
+                .span_bug(scrutinee_span, "let expression not expected in this context"),
         }
 
-        let success = self.bind_pattern(self.source_info(pat.span), branch, &[], expr_span, None);
+        let true_block =
+            self.bind_pattern(self.source_info(pat.span), true_branch, &[], scrutinee_span, None);
 
         // If branch coverage is enabled, record this branch.
-        self.visit_coverage_conditional_let(pat, success, built_tree.otherwise_block);
+        self.visit_coverage_conditional_let(pat, true_block, false_block);
 
-        success.unit()
+        true_block.unit()
     }
 
     /// Initializes each of the bindings from the candidate by