Formally implement let chains

Author: c410-f3r

Diff for: compiler/rustc_ast_lowering/src/expr.rs

@@ -392,14 +392,20 @@ impl<'hir> LoweringContext<'_, 'hir> {
     // If `cond` kind is `let`, returns `let`. Otherwise, wraps and returns `cond`
     // in a temporary block.
     fn manage_let_cond(&mut self, cond: &'hir hir::Expr<'hir>) -> &'hir hir::Expr<'hir> {
-        match cond.kind {
-            hir::ExprKind::Let(..) => cond,
-            _ => {
-                let span_block =
-                    self.mark_span_with_reason(DesugaringKind::CondTemporary, cond.span, None);
-                self.expr_drop_temps(span_block, cond, AttrVec::new())
+        fn has_let_expr<'hir>(expr: &'hir hir::Expr<'hir>) -> bool {
+            match expr.kind {
+                hir::ExprKind::Binary(_, lhs, rhs) => has_let_expr(lhs) || has_let_expr(rhs),
+                hir::ExprKind::Let(..) => true,
+                _ => false,
             }
         }
+        if has_let_expr(cond) {
+            cond
+        } else {
+            let reason = DesugaringKind::CondTemporary;
+            let span_block = self.mark_span_with_reason(reason, cond.span, None);
+            self.expr_drop_temps(span_block, cond, AttrVec::new())
+        }
     }
 
     // We desugar: `'label: while $cond $body` into:

Diff for: compiler/rustc_ast_passes/src/feature_gate.rs

@@ -707,11 +707,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session) {
         "`if let` guards are experimental",
         "you can write `if matches!(<expr>, <pattern>)` instead of `if let <pattern> = <expr>`"
     );
-    gate_all!(
-        let_chains,
-        "`let` expressions in this position are experimental",
-        "you can write `matches!(<expr>, <pattern>)` instead of `let <pattern> = <expr>`"
-    );
+    gate_all!(let_chains, "`let` expressions in this position are unstable");
     gate_all!(
         async_closure,
         "async closures are unstable",

Diff for: compiler/rustc_feature/src/active.rs

@@ -415,7 +415,7 @@ declare_features! (
     // Allows setting the threshold for the `large_assignments` lint.
     (active, large_assignments, "1.52.0", Some(83518), None),
     /// Allows `if/while p && let q = r && ...` chains.
-    (incomplete, let_chains, "1.37.0", Some(53667), None),
+    (active, let_chains, "1.37.0", Some(53667), None),
     /// Allows `let...else` statements.
     (active, let_else, "1.56.0", Some(87335), None),
     /// Allows `#[link(..., cfg(..))]`.

Diff for: compiler/rustc_middle/src/thir.rs

@@ -213,7 +213,7 @@ pub struct Expr<'tcx> {
 
 #[derive(Debug, HashStable)]
 pub enum ExprKind<'tcx> {
-    /// `Scope`s are used to explicitely mark destruction scopes,
+    /// `Scope`s are used to explicitly mark destruction scopes,
     /// and to track the `HirId` of the expressions within the scope.
     Scope {
         region_scope: region::Scope,

Diff for: compiler/rustc_mir_build/src/build/expr/into.rs

@@ -90,17 +90,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 };
 
                 let join_block = this.cfg.start_new_block();
-                this.cfg.terminate(
-                    then_blk,
-                    source_info,
-                    TerminatorKind::Goto { target: join_block },
-                );
-                this.cfg.terminate(
-                    else_blk,
-                    source_info,
-                    TerminatorKind::Goto { target: join_block },
-                );
-
+                this.cfg.goto(then_blk, source_info, join_block);
+                this.cfg.goto(else_blk, source_info, join_block);
                 join_block.unit()
             }
             ExprKind::Let { expr, ref pat } => {
@@ -109,8 +100,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     this.lower_let_expr(block, &this.thir[expr], pat, scope, expr_span)
                 });
 
-                let join_block = this.cfg.start_new_block();
-
                 this.cfg.push_assign_constant(
                     true_block,
                     source_info,
@@ -133,6 +122,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     },
                 );
 
+                let join_block = this.cfg.start_new_block();
                 this.cfg.goto(true_block, source_info, join_block);
                 this.cfg.goto(false_block, source_info, join_block);
                 join_block.unit()

Diff for: compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -47,6 +47,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let expr_span = expr.span;
 
         match expr.kind {
+            ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => {
+                let lhs_then_block = unpack!(this.then_else_break(
+                    block,
+                    &this.thir[lhs],
+                    temp_scope_override,
+                    break_scope,
+                    variable_scope_span,
+                ));
+
+                let rhs_then_block = unpack!(this.then_else_break(
+                    lhs_then_block,
+                    &this.thir[rhs],
+                    temp_scope_override,
+                    break_scope,
+                    variable_scope_span,
+                ));
+
+                rhs_then_block.unit()
+            }
             ExprKind::Scope { region_scope, lint_level, value } => {
                 let region_scope = (region_scope, this.source_info(expr_span));
                 this.in_scope(region_scope, lint_level, |this| {

Diff for: compiler/rustc_mir_build/src/build/scope.rs

@@ -498,7 +498,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///
     /// if let Some(x) = a && let Some(y) = b && let Some(z) = c { ... }
     ///
-    /// there are three possible ways the condition can be false and we may have
+    /// There are three possible ways the condition can be false and we may have
     /// to drop `x`, `x` and `y`, or neither depending on which binding fails.
     /// To handle this correctly we use a `DropTree` in a similar way to a
     /// `loop` expression and 'break' out on all of the 'else' paths.

Diff for: compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -315,7 +315,6 @@ impl<'tcx> Cx<'tcx> {
                             lhs: self.mirror_expr(lhs),
                             rhs: self.mirror_expr(rhs),
                         },
-
                         _ => {
                             let op = bin_op(op.node);
                             ExprKind::Binary {

Diff for: compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -17,6 +17,7 @@ use rustc_session::lint::builtin::{
     BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
 };
 use rustc_session::Session;
+use rustc_span::source_map::Spanned;
 use rustc_span::{DesugaringKind, ExpnKind, Span};
 
 crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) {
@@ -445,6 +446,10 @@ fn check_let_reachability<'p, 'tcx>(
     pat: &'p DeconstructedPat<'p, 'tcx>,
     span: Span,
 ) {
+    if is_let_chain(cx.tcx, pat_id) {
+        return;
+    }
+
     let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }];
     let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty());
 
@@ -764,8 +769,11 @@ pub enum LetSource {
 
 fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource {
     let hir = tcx.hir();
+
     let parent = hir.get_parent_node(pat_id);
-    match hir.get(parent) {
+    let parent_node = hir.get(parent);
+
+    match parent_node {
         hir::Node::Arm(hir::Arm {
             guard: Some(hir::Guard::IfLet(&hir::Pat { hir_id, .. }, _)),
             ..
@@ -780,6 +788,7 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource {
         }
         _ => {}
     }
+
     let parent_parent = hir.get_parent_node(parent);
     let parent_parent_node = hir.get(parent_parent);
 
@@ -792,12 +801,30 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource {
         ..
     }) = parent_parent_parent_parent_node
     {
-        LetSource::WhileLet
-    } else if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If { .. }, .. }) =
-        parent_parent_node
-    {
-        LetSource::IfLet
-    } else {
-        LetSource::GenericLet
+        return LetSource::WhileLet;
+    }
+
+    if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If(..), .. }) = parent_parent_node {
+        return LetSource::IfLet;
     }
+
+    LetSource::GenericLet
+}
+
+// Since this function is called within a let context, it is reasonable to assume that any parent
+// `&&` infers a let chain
+fn is_let_chain(tcx: TyCtxt<'_>, pat_id: HirId) -> bool {
+    let hir = tcx.hir();
+    let parent = hir.get_parent_node(pat_id);
+    let parent_parent = hir.get_parent_node(parent);
+    matches!(
+        hir.get(parent_parent),
+        hir::Node::Expr(
+            hir::Expr {
+                kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, ..),
+                ..
+            },
+            ..
+        )
+    )
 }

Diff for: src/test/ui/expr/if/attrs/let-chains-attr.rs

@@ -1,6 +1,6 @@
 // check-pass
 
-#![feature(let_chains)] //~ WARN the feature `let_chains` is incomplete
+#![feature(let_chains)]
 
 #[cfg(FALSE)]
 fn foo() {

Diff for: src/test/ui/expr/if/attrs/let-chains-attr.stderr

@@ -0,0 +1,93 @@
+// run-pass
+// needs-unwind
+// ignore-wasm32-bare compiled with panic=abort by default
+
+// See `mir_drop_order.rs` for more information
+
+#![feature(let_chains)]
+
+use std::cell::RefCell;
+use std::panic;
+
+pub struct DropLogger<'a, T> {
+    extra: T,
+    id: usize,
+    log: &'a panic::AssertUnwindSafe<RefCell<Vec<usize>>>
+}
+
+impl<'a, T> Drop for DropLogger<'a, T> {
+    fn drop(&mut self) {
+        self.log.0.borrow_mut().push(self.id);
+    }
+}
+
+struct InjectedFailure;
+
+#[allow(unreachable_code)]
+fn main() {
+    let log = panic::AssertUnwindSafe(RefCell::new(vec![]));
+    let d = |id, extra| DropLogger { extra, id: id, log: &log };
+    let get = || -> Vec<_> {
+        let mut m = log.0.borrow_mut();
+        let n = m.drain(..);
+        n.collect()
+    };
+
+    {
+        let _x = (
+            d(
+                0,
+                d(
+                    1,
+                    if let Some(_) = d(2, Some(true)).extra && let DropLogger { .. } = d(3, None) {
+                        None
+                    } else {
+                        Some(true)
+                    }
+                ).extra
+            ),
+            d(4, None),
+            &d(5, None),
+            d(6, None),
+            if let DropLogger { .. } = d(7, None) && let DropLogger { .. } = d(8, None) {
+                d(9, None)
+            }
+            else {
+                // 10 is not constructed
+                d(10, None)
+            }
+        );
+        assert_eq!(get(), vec![3, 8, 7, 1, 2]);
+    }
+    assert_eq!(get(), vec![0, 4, 6, 9, 5]);
+
+    let _ = std::panic::catch_unwind(|| {
+        (
+            d(
+                11,
+                d(
+                    12,
+                    if let Some(_) = d(13, Some(true)).extra
+                        && let DropLogger { .. } = d(14, None)
+                    {
+                        None
+                    } else {
+                        Some(true)
+                    }
+                ).extra
+            ),
+            d(15, None),
+            &d(16, None),
+            d(17, None),
+            if let DropLogger { .. } = d(18, None) && let DropLogger { .. } = d(19, None) {
+                d(20, None)
+            }
+            else {
+                // 10 is not constructed
+                d(21, None)
+            },
+            panic::panic_any(InjectedFailure)
+        );
+    });
+    assert_eq!(get(), vec![14, 19, 20, 17, 15, 11, 18, 16, 12, 13]);
+}