Skip to content

Commit

Permalink
Rollup merge of rust-lang#88642 - c410-f3r:let_chains_2, r=matthewjasper
Browse files Browse the repository at this point in the history
Formally implement let chains

## Let chains

My longest and hardest contribution since rust-lang#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 rust-lang#53667
  • Loading branch information
matthiaskrgr authored Jan 19, 2022
2 parents 2f004d2 + 5f74ef4 commit 5d2928f
Show file tree
Hide file tree
Showing 31 changed files with 536 additions and 340 deletions.
18 changes: 12 additions & 6 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(..))]`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 3 additions & 13 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 } => {
Expand All @@ -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,
Expand All @@ -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()
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
43 changes: 35 additions & 8 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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, .. }, _)),
..
Expand All @@ -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);

Expand All @@ -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, .. }, ..),
..
},
..
)
)
}
2 changes: 1 addition & 1 deletion src/test/ui/expr/if/attrs/let-chains-attr.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// check-pass

#![feature(let_chains)] //~ WARN the feature `let_chains` is incomplete
#![feature(let_chains)]

#[cfg(FALSE)]
fn foo() {
Expand Down
11 changes: 0 additions & 11 deletions src/test/ui/expr/if/attrs/let-chains-attr.stderr

This file was deleted.

93 changes: 93 additions & 0 deletions src/test/ui/mir/mir_let_chains_drop_order.rs
Original file line number Diff line number Diff line change
@@ -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]);
}
9 changes: 0 additions & 9 deletions src/test/ui/pattern/issue-82290.rs

This file was deleted.

21 changes: 0 additions & 21 deletions src/test/ui/pattern/issue-82290.stderr

This file was deleted.

Loading

0 comments on commit 5d2928f

Please sign in to comment.