Skip to content

Commit

Permalink
Rollup merge of rust-lang#79051 - LeSeulArtichaut:if-let-guard, r=mat…
Browse files Browse the repository at this point in the history
…thewjasper

Implement if-let match guards

Implements rust-lang/rfcs#2294 (tracking issue: rust-lang#51114).

I probably should do a few more things before this can be merged:
- [x] Add tests (added basic tests, more advanced tests could be done in the future?)
- [x] Add lint for exhaustive if-let guard (comparable to normal if-let statements)
- [x] Fix clippy

However since this is a nightly feature maybe it's fine to land this and do those steps in follow-up PRs.

Thanks a lot `@matthewjasper` ❤️ for helping me with lowering to MIR! Would you be interested in reviewing this?
r? `@ghost` for now
  • Loading branch information
Dylan-DPC authored Dec 16, 2020
2 parents 2ba7ca2 + 0917260 commit cec4573
Show file tree
Hide file tree
Showing 27 changed files with 360 additions and 81 deletions.
15 changes: 10 additions & 5 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,19 @@ impl<'hir> LoweringContext<'_, 'hir> {
}

fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> {
let pat = self.lower_pat(&arm.pat);
let guard = arm.guard.as_ref().map(|cond| {
if let ExprKind::Let(ref pat, ref scrutinee) = cond.kind {
hir::Guard::IfLet(self.lower_pat(pat), self.lower_expr(scrutinee))
} else {
hir::Guard::If(self.lower_expr(cond))
}
});
hir::Arm {
hir_id: self.next_id(),
attrs: self.lower_attrs(&arm.attrs),
pat: self.lower_pat(&arm.pat),
guard: match arm.guard {
Some(ref x) => Some(hir::Guard::If(self.lower_expr(x))),
_ => None,
},
pat,
guard,
body: self.lower_expr(&arm.body),
span: arm.span,
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,7 @@ pub struct Arm<'hir> {
#[derive(Debug, HashStable_Generic)]
pub enum Guard<'hir> {
If(&'hir Expr<'hir>),
IfLet(&'hir Pat<'hir>, &'hir Expr<'hir>),
}

#[derive(Debug, HashStable_Generic)]
Expand Down Expand Up @@ -1721,6 +1722,8 @@ pub enum MatchSource {
IfDesugar { contains_else_clause: bool },
/// An `if let _ = _ { .. }` (optionally with `else { .. }`).
IfLetDesugar { contains_else_clause: bool },
/// An `if let _ = _ => { .. }` match guard.
IfLetGuardDesugar,
/// A `while _ { .. }` (which was desugared to a `loop { match _ { .. } }`).
WhileDesugar,
/// A `while let _ = _ { .. }` (which was desugared to a
Expand All @@ -1739,7 +1742,7 @@ impl MatchSource {
use MatchSource::*;
match self {
Normal => "match",
IfDesugar { .. } | IfLetDesugar { .. } => "if",
IfDesugar { .. } | IfLetDesugar { .. } | IfLetGuardDesugar => "if",
WhileDesugar | WhileLetDesugar => "while",
ForLoopDesugar => "for",
TryDesugar => "?",
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,10 @@ pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm<'v>) {
if let Some(ref g) = arm.guard {
match g {
Guard::If(ref e) => visitor.visit_expr(e),
Guard::IfLet(ref pat, ref e) => {
visitor.visit_pat(pat);
visitor.visit_expr(e);
}
}
}
visitor.visit_expr(&arm.body);
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2002,6 +2002,15 @@ impl<'a> State<'a> {
self.print_expr(&e);
self.s.space();
}
hir::Guard::IfLet(pat, e) => {
self.word_nbsp("if");
self.word_nbsp("let");
self.print_pat(&pat);
self.s.space();
self.word_space("=");
self.print_expr(&e);
self.s.space();
}
}
}
self.word_space("=>");
Expand Down
54 changes: 45 additions & 9 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
guard: Option<&Guard<'tcx>>,
fake_borrow_temps: &Vec<(Place<'tcx>, Local)>,
scrutinee_span: Span,
arm_span: Option<Span>,
arm_scope: Option<region::Scope>,
) -> BasicBlock {
if candidate.subcandidates.is_empty() {
Expand All @@ -239,6 +240,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
guard,
fake_borrow_temps,
scrutinee_span,
arm_span,
true,
)
} else {
Expand Down Expand Up @@ -274,6 +276,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
guard,
&fake_borrow_temps,
scrutinee_span,
arm_span,
schedule_drops,
);
if arm_scope.is_none() {
Expand Down Expand Up @@ -436,6 +439,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&fake_borrow_temps,
irrefutable_pat.span,
None,
None,
)
.unit()
}
Expand Down Expand Up @@ -817,11 +821,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// For an example of a case where we set `otherwise_block`, even for an
/// exhaustive match consider:
///
/// ```rust
/// match x {
/// (true, true) => (),
/// (_, false) => (),
/// (false, true) => (),
/// }
/// ```
///
/// For this match, we check if `x.0` matches `true` (for the first
/// arm). If that's false, we check `x.1`. If it's `true` we check if
Expand Down Expand Up @@ -935,11 +941,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Link up matched candidates. For example, if we have something like
/// this:
///
/// ```rust
/// ...
/// Some(x) if cond => ...
/// Some(x) => ...
/// Some(x) if cond => ...
/// ...
/// ```
///
/// We generate real edges from:
/// * `start_block` to the `prebinding_block` of the first pattern,
Expand Down Expand Up @@ -1517,7 +1525,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Initializes each of the bindings from the candidate by
/// moving/copying/ref'ing the source as appropriate. Tests the guard, if
/// any, and then branches to the arm. Returns the block for the case where
/// the guard fails.
/// the guard succeeds.
///
/// Note: we do not check earlier that if there is a guard,
/// there cannot be move bindings. We avoid a use-after-move by only
Expand All @@ -1529,6 +1537,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
guard: Option<&Guard<'tcx>>,
fake_borrows: &Vec<(Place<'tcx>, Local)>,
scrutinee_span: Span,
arm_span: Option<Span>,
schedule_drops: bool,
) -> BasicBlock {
debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate);
Expand Down Expand Up @@ -1659,15 +1668,42 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.push_assign(block, scrutinee_source_info, Place::from(temp), borrow);
}

// the block to branch to if the guard fails; if there is no
// guard, this block is simply unreachable
let guard = match guard {
Guard::If(e) => self.hir.mirror(e.clone()),
let (guard_span, (post_guard_block, otherwise_post_guard_block)) = match guard {
Guard::If(e) => {
let e = self.hir.mirror(e.clone());
let source_info = self.source_info(e.span);
(e.span, self.test_bool(block, e, source_info))
},
Guard::IfLet(pat, scrutinee) => {
let scrutinee_span = scrutinee.span();
let scrutinee_place = unpack!(block = self.lower_scrutinee(block, scrutinee.clone(), scrutinee_span));
let mut guard_candidate = Candidate::new(scrutinee_place, &pat, false);
let wildcard = Pat::wildcard_from_ty(pat.ty);
let mut otherwise_candidate = Candidate::new(scrutinee_place, &wildcard, false);
let fake_borrow_temps =
self.lower_match_tree(block, pat.span, false, &mut [&mut guard_candidate, &mut otherwise_candidate]);
self.declare_bindings(
None,
pat.span.to(arm_span.unwrap()),
pat,
ArmHasGuard(false),
Some((Some(&scrutinee_place), scrutinee.span())),
);
let post_guard_block = self.bind_pattern(
self.source_info(pat.span),
guard_candidate,
None,
&fake_borrow_temps,
scrutinee.span(),
None,
None,
);
let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap();
(scrutinee_span, (post_guard_block, otherwise_post_guard_block))
}
};
let source_info = self.source_info(guard.span);
let guard_end = self.source_info(tcx.sess.source_map().end_point(guard.span));
let (post_guard_block, otherwise_post_guard_block) =
self.test_bool(block, guard, source_info);
let source_info = self.source_info(guard_span);
let guard_end = self.source_info(tcx.sess.source_map().end_point(guard_span));
let guard_frame = self.guard_context.pop().unwrap();
debug!("Exiting guard building context with locals: {:?}", guard_frame);

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm.guard.as_ref(),
&fake_borrow_temps,
scrutinee_span,
Some(arm.span),
Some(arm.scope),
);

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,10 +776,10 @@ impl ToBorrowKind for hir::Mutability {
fn convert_arm<'tcx>(cx: &mut Cx<'_, 'tcx>, arm: &'tcx hir::Arm<'tcx>) -> Arm<'tcx> {
Arm {
pattern: cx.pattern_from_hir(&arm.pat),
guard: match arm.guard {
Some(hir::Guard::If(ref e)) => Some(Guard::If(e.to_ref())),
_ => None,
},
guard: arm.guard.as_ref().map(|g| match g {
hir::Guard::If(ref e) => Guard::If(e.to_ref()),
hir::Guard::IfLet(ref pat, ref e) => Guard::IfLet(cx.pattern_from_hir(pat), e.to_ref()),
}),
body: arm.body.to_ref(),
lint_level: LintLevel::Explicit(arm.hir_id),
scope: region::Scope { id: arm.hir_id.local_id, data: region::ScopeData::Node },
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/thir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ crate struct Arm<'tcx> {
#[derive(Clone, Debug)]
crate enum Guard<'tcx> {
If(ExprRef<'tcx>),
IfLet(Pat<'tcx>, ExprRef<'tcx>),
}

#[derive(Copy, Clone, Debug)]
Expand Down
31 changes: 31 additions & 0 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,20 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
for arm in arms {
// Check the arm for some things unrelated to exhaustiveness.
self.check_patterns(&arm.pat);
if let Some(hir::Guard::IfLet(ref pat, _)) = arm.guard {
self.check_patterns(pat);
}
}

let mut cx = self.new_cx(scrut.hir_id);

for arm in arms {
if let Some(hir::Guard::IfLet(ref pat, _)) = arm.guard {
let tpat = self.lower_pattern(&mut cx, pat, &mut false).0;
check_if_let_guard(&mut cx, &tpat, pat.hir_id);
}
}

let mut have_errors = false;

let arms: Vec<_> = arms
Expand Down Expand Up @@ -360,12 +370,28 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::
let msg = match source {
hir::MatchSource::IfLetDesugar { .. } => "irrefutable if-let pattern",
hir::MatchSource::WhileLetDesugar => "irrefutable while-let pattern",
hir::MatchSource::IfLetGuardDesugar => "irrefutable if-let guard",
_ => bug!(),
};
lint.build(msg).emit()
});
}

fn check_if_let_guard<'p, 'tcx>(
cx: &mut MatchCheckCtxt<'p, 'tcx>,
pat: &'p super::Pat<'tcx>,
pat_id: HirId,
) {
let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }];
let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty);
report_arm_reachability(&cx, &report, hir::MatchSource::IfLetGuardDesugar);

if report.non_exhaustiveness_witnesses.is_empty() {
// The match is exhaustive, i.e. the if let pattern is irrefutable.
irrefutable_let_pattern(cx.tcx, pat.span, pat_id, hir::MatchSource::IfLetGuardDesugar)
}
}

/// Report unreachable arms, if any.
fn report_arm_reachability<'p, 'tcx>(
cx: &MatchCheckCtxt<'p, 'tcx>,
Expand All @@ -390,6 +416,11 @@ fn report_arm_reachability<'p, 'tcx>(
}
}

hir::MatchSource::IfLetGuardDesugar => {
assert_eq!(arm_index, 0);
unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, None);
}

hir::MatchSource::ForLoopDesugar | hir::MatchSource::Normal => {
unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, catchall);
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_passes/src/check_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ impl NonConstExpr {
return None;
}

Self::Match(IfLetGuardDesugar) => bug!("if-let guard outside a `match` expression"),

// All other expressions are allowed.
Self::Loop(Loop | While | WhileLet)
| Self::Match(
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {

fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) {
self.add_from_pat(&arm.pat);
if let Some(hir::Guard::IfLet(ref pat, _)) = arm.guard {
self.add_from_pat(pat);
}
intravisit::walk_arm(self, arm);
}

Expand Down Expand Up @@ -866,10 +869,13 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
for arm in arms {
let body_succ = self.propagate_through_expr(&arm.body, succ);

let guard_succ = self.propagate_through_opt_expr(
arm.guard.as_ref().map(|hir::Guard::If(e)| *e),
body_succ,
);
let guard_succ = arm.guard.as_ref().map_or(body_succ, |g| match g {
hir::Guard::If(e) => self.propagate_through_expr(e, body_succ),
hir::Guard::IfLet(pat, e) => {
let let_bind = self.define_bindings_in_pat(pat, body_succ);
self.propagate_through_expr(e, let_bind)
}
});
let arm_succ = self.define_bindings_in_pat(&arm.pat, guard_succ);
self.merge_from_succ(ln, arm_succ);
}
Expand Down
Loading

0 comments on commit cec4573

Please sign in to comment.