Skip to content

Commit 5d2928f

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #88642 - c410-f3r:let_chains_2, r=matthewjasper
Formally implement let chains ## Let chains My longest and hardest contribution since #64010. Thanks to `@Centril` for creating the RFC and special thanks to `@matthewjasper` for helping me since the beginning of this journey. In fact, `@matthewjasper` did much of the complicated MIR stuff so it's true to say that this feature wouldn't be possible without him. Thanks again `@matthewjasper!` With the changes proposed in this PR, it will be possible to chain let expressions along side local variable declarations or ordinary conditional expressions. In other words, do much of what the `if_chain` crate already does. ## Other considerations * `if let guard` and `let ... else` features need special care and should be handled in a following PR. * Irrefutable patterns are allowed within a let chain context * ~~Three Clippy lints were already converted to start dogfooding and help detect possible corner cases~~ cc #53667
2 parents 2f004d2 + 5f74ef4 commit 5d2928f

31 files changed

+536
-340
lines changed

‎compiler/rustc_ast_lowering/src/expr.rs

+12-6
Original file line numberDiff line numberDiff line change
@@ -392,14 +392,20 @@ impl<'hir> LoweringContext<'_, 'hir> {
392392
// If `cond` kind is `let`, returns `let`. Otherwise, wraps and returns `cond`
393393
// in a temporary block.
394394
fn manage_let_cond(&mut self, cond: &'hir hir::Expr<'hir>) -> &'hir hir::Expr<'hir> {
395-
match cond.kind {
396-
hir::ExprKind::Let(..) => cond,
397-
_ => {
398-
let span_block =
399-
self.mark_span_with_reason(DesugaringKind::CondTemporary, cond.span, None);
400-
self.expr_drop_temps(span_block, cond, AttrVec::new())
395+
fn has_let_expr<'hir>(expr: &'hir hir::Expr<'hir>) -> bool {
396+
match expr.kind {
397+
hir::ExprKind::Binary(_, lhs, rhs) => has_let_expr(lhs) || has_let_expr(rhs),
398+
hir::ExprKind::Let(..) => true,
399+
_ => false,
401400
}
402401
}
402+
if has_let_expr(cond) {
403+
cond
404+
} else {
405+
let reason = DesugaringKind::CondTemporary;
406+
let span_block = self.mark_span_with_reason(reason, cond.span, None);
407+
self.expr_drop_temps(span_block, cond, AttrVec::new())
408+
}
403409
}
404410

405411
// We desugar: `'label: while $cond $body` into:

‎compiler/rustc_ast_passes/src/feature_gate.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -707,11 +707,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session) {
707707
"`if let` guards are experimental",
708708
"you can write `if matches!(<expr>, <pattern>)` instead of `if let <pattern> = <expr>`"
709709
);
710-
gate_all!(
711-
let_chains,
712-
"`let` expressions in this position are experimental",
713-
"you can write `matches!(<expr>, <pattern>)` instead of `let <pattern> = <expr>`"
714-
);
710+
gate_all!(let_chains, "`let` expressions in this position are unstable");
715711
gate_all!(
716712
async_closure,
717713
"async closures are unstable",

‎compiler/rustc_feature/src/active.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ declare_features! (
415415
// Allows setting the threshold for the `large_assignments` lint.
416416
(active, large_assignments, "1.52.0", Some(83518), None),
417417
/// Allows `if/while p && let q = r && ...` chains.
418-
(incomplete, let_chains, "1.37.0", Some(53667), None),
418+
(active, let_chains, "1.37.0", Some(53667), None),
419419
/// Allows `let...else` statements.
420420
(active, let_else, "1.56.0", Some(87335), None),
421421
/// Allows `#[link(..., cfg(..))]`.

‎compiler/rustc_middle/src/thir.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ pub struct Expr<'tcx> {
213213

214214
#[derive(Debug, HashStable)]
215215
pub enum ExprKind<'tcx> {
216-
/// `Scope`s are used to explicitely mark destruction scopes,
216+
/// `Scope`s are used to explicitly mark destruction scopes,
217217
/// and to track the `HirId` of the expressions within the scope.
218218
Scope {
219219
region_scope: region::Scope,

‎compiler/rustc_mir_build/src/build/expr/into.rs

+3-13
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9090
};
9191

9292
let join_block = this.cfg.start_new_block();
93-
this.cfg.terminate(
94-
then_blk,
95-
source_info,
96-
TerminatorKind::Goto { target: join_block },
97-
);
98-
this.cfg.terminate(
99-
else_blk,
100-
source_info,
101-
TerminatorKind::Goto { target: join_block },
102-
);
103-
93+
this.cfg.goto(then_blk, source_info, join_block);
94+
this.cfg.goto(else_blk, source_info, join_block);
10495
join_block.unit()
10596
}
10697
ExprKind::Let { expr, ref pat } => {
@@ -109,8 +100,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
109100
this.lower_let_expr(block, &this.thir[expr], pat, scope, expr_span)
110101
});
111102

112-
let join_block = this.cfg.start_new_block();
113-
114103
this.cfg.push_assign_constant(
115104
true_block,
116105
source_info,
@@ -133,6 +122,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
133122
},
134123
);
135124

125+
let join_block = this.cfg.start_new_block();
136126
this.cfg.goto(true_block, source_info, join_block);
137127
this.cfg.goto(false_block, source_info, join_block);
138128
join_block.unit()

‎compiler/rustc_mir_build/src/build/matches/mod.rs

+19
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
4747
let expr_span = expr.span;
4848

4949
match expr.kind {
50+
ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => {
51+
let lhs_then_block = unpack!(this.then_else_break(
52+
block,
53+
&this.thir[lhs],
54+
temp_scope_override,
55+
break_scope,
56+
variable_scope_span,
57+
));
58+
59+
let rhs_then_block = unpack!(this.then_else_break(
60+
lhs_then_block,
61+
&this.thir[rhs],
62+
temp_scope_override,
63+
break_scope,
64+
variable_scope_span,
65+
));
66+
67+
rhs_then_block.unit()
68+
}
5069
ExprKind::Scope { region_scope, lint_level, value } => {
5170
let region_scope = (region_scope, this.source_info(expr_span));
5271
this.in_scope(region_scope, lint_level, |this| {

‎compiler/rustc_mir_build/src/build/scope.rs

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

‎compiler/rustc_mir_build/src/thir/cx/expr.rs

-1
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,6 @@ impl<'tcx> Cx<'tcx> {
315315
lhs: self.mirror_expr(lhs),
316316
rhs: self.mirror_expr(rhs),
317317
},
318-
319318
_ => {
320319
let op = bin_op(op.node);
321320
ExprKind::Binary {

‎compiler/rustc_mir_build/src/thir/pattern/check_match.rs

+35-8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use rustc_session::lint::builtin::{
1717
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
1818
};
1919
use rustc_session::Session;
20+
use rustc_span::source_map::Spanned;
2021
use rustc_span::{DesugaringKind, ExpnKind, Span};
2122

2223
crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) {
@@ -445,6 +446,10 @@ fn check_let_reachability<'p, 'tcx>(
445446
pat: &'p DeconstructedPat<'p, 'tcx>,
446447
span: Span,
447448
) {
449+
if is_let_chain(cx.tcx, pat_id) {
450+
return;
451+
}
452+
448453
let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }];
449454
let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty());
450455

@@ -764,8 +769,11 @@ pub enum LetSource {
764769

765770
fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource {
766771
let hir = tcx.hir();
772+
767773
let parent = hir.get_parent_node(pat_id);
768-
match hir.get(parent) {
774+
let parent_node = hir.get(parent);
775+
776+
match parent_node {
769777
hir::Node::Arm(hir::Arm {
770778
guard: Some(hir::Guard::IfLet(&hir::Pat { hir_id, .. }, _)),
771779
..
@@ -780,6 +788,7 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource {
780788
}
781789
_ => {}
782790
}
791+
783792
let parent_parent = hir.get_parent_node(parent);
784793
let parent_parent_node = hir.get(parent_parent);
785794

@@ -792,12 +801,30 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource {
792801
..
793802
}) = parent_parent_parent_parent_node
794803
{
795-
LetSource::WhileLet
796-
} else if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If { .. }, .. }) =
797-
parent_parent_node
798-
{
799-
LetSource::IfLet
800-
} else {
801-
LetSource::GenericLet
804+
return LetSource::WhileLet;
805+
}
806+
807+
if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If(..), .. }) = parent_parent_node {
808+
return LetSource::IfLet;
802809
}
810+
811+
LetSource::GenericLet
812+
}
813+
814+
// Since this function is called within a let context, it is reasonable to assume that any parent
815+
// `&&` infers a let chain
816+
fn is_let_chain(tcx: TyCtxt<'_>, pat_id: HirId) -> bool {
817+
let hir = tcx.hir();
818+
let parent = hir.get_parent_node(pat_id);
819+
let parent_parent = hir.get_parent_node(parent);
820+
matches!(
821+
hir.get(parent_parent),
822+
hir::Node::Expr(
823+
hir::Expr {
824+
kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, ..),
825+
..
826+
},
827+
..
828+
)
829+
)
803830
}

‎src/test/ui/expr/if/attrs/let-chains-attr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// check-pass
22

3-
#![feature(let_chains)] //~ WARN the feature `let_chains` is incomplete
3+
#![feature(let_chains)]
44

55
#[cfg(FALSE)]
66
fn foo() {

‎src/test/ui/expr/if/attrs/let-chains-attr.stderr

-11
This file was deleted.

‎src/test/ui/mir/mir_let_chains_drop_order.rs

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// run-pass
2+
// needs-unwind
3+
// ignore-wasm32-bare compiled with panic=abort by default
4+
5+
// See `mir_drop_order.rs` for more information
6+
7+
#![feature(let_chains)]
8+
9+
use std::cell::RefCell;
10+
use std::panic;
11+
12+
pub struct DropLogger<'a, T> {
13+
extra: T,
14+
id: usize,
15+
log: &'a panic::AssertUnwindSafe<RefCell<Vec<usize>>>
16+
}
17+
18+
impl<'a, T> Drop for DropLogger<'a, T> {
19+
fn drop(&mut self) {
20+
self.log.0.borrow_mut().push(self.id);
21+
}
22+
}
23+
24+
struct InjectedFailure;
25+
26+
#[allow(unreachable_code)]
27+
fn main() {
28+
let log = panic::AssertUnwindSafe(RefCell::new(vec![]));
29+
let d = |id, extra| DropLogger { extra, id: id, log: &log };
30+
let get = || -> Vec<_> {
31+
let mut m = log.0.borrow_mut();
32+
let n = m.drain(..);
33+
n.collect()
34+
};
35+
36+
{
37+
let _x = (
38+
d(
39+
0,
40+
d(
41+
1,
42+
if let Some(_) = d(2, Some(true)).extra && let DropLogger { .. } = d(3, None) {
43+
None
44+
} else {
45+
Some(true)
46+
}
47+
).extra
48+
),
49+
d(4, None),
50+
&d(5, None),
51+
d(6, None),
52+
if let DropLogger { .. } = d(7, None) && let DropLogger { .. } = d(8, None) {
53+
d(9, None)
54+
}
55+
else {
56+
// 10 is not constructed
57+
d(10, None)
58+
}
59+
);
60+
assert_eq!(get(), vec![3, 8, 7, 1, 2]);
61+
}
62+
assert_eq!(get(), vec![0, 4, 6, 9, 5]);
63+
64+
let _ = std::panic::catch_unwind(|| {
65+
(
66+
d(
67+
11,
68+
d(
69+
12,
70+
if let Some(_) = d(13, Some(true)).extra
71+
&& let DropLogger { .. } = d(14, None)
72+
{
73+
None
74+
} else {
75+
Some(true)
76+
}
77+
).extra
78+
),
79+
d(15, None),
80+
&d(16, None),
81+
d(17, None),
82+
if let DropLogger { .. } = d(18, None) && let DropLogger { .. } = d(19, None) {
83+
d(20, None)
84+
}
85+
else {
86+
// 10 is not constructed
87+
d(21, None)
88+
},
89+
panic::panic_any(InjectedFailure)
90+
);
91+
});
92+
assert_eq!(get(), vec![14, 19, 20, 17, 15, 11, 18, 16, 12, 13]);
93+
}

‎src/test/ui/pattern/issue-82290.rs

-9
This file was deleted.

‎src/test/ui/pattern/issue-82290.stderr

-21
This file was deleted.

0 commit comments

Comments
 (0)