Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement if-let match guards #79051

Merged
merged 7 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1220,6 +1220,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);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this loop could be merged with the one above, that would probably reduce the perf impact

Copy link
Member

@camelid camelid Dec 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try opening a PR. (EDIT: #80367)


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.
LeSeulArtichaut marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -358,6 +358,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 @@ -1029,10 +1032,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, first_merge);
first_merge = false;
Expand Down
Loading