Skip to content

Commit 6ed41a8

Browse files
committed
let_chains: address FIXME in check_match
1 parent 4a7b537 commit 6ed41a8

File tree

1 file changed

+57
-65
lines changed

1 file changed

+57
-65
lines changed

src/librustc_mir_build/hair/pattern/check_match.rs

Lines changed: 57 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,13 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> {
6868
hir::LocalSource::AwaitDesugar => ("`await` future binding", None),
6969
};
7070
self.check_irrefutable(&loc.pat, msg, sp);
71-
self.check_patterns(false, &loc.pat);
71+
self.check_pattern(false, &loc.pat);
7272
}
7373

7474
fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
7575
intravisit::walk_param(self, param);
7676
self.check_irrefutable(&param.pat, "function argument", None);
77-
self.check_patterns(false, &param.pat);
77+
self.check_pattern(false, &param.pat);
7878
}
7979
}
8080

@@ -113,7 +113,7 @@ impl PatCtxt<'_, '_> {
113113
}
114114

115115
impl<'tcx> MatchVisitor<'_, 'tcx> {
116-
fn check_patterns(&mut self, has_guard: bool, pat: &Pat<'_>) {
116+
fn check_pattern(&mut self, has_guard: bool, pat: &Pat<'_>) {
117117
check_legality_of_move_bindings(self, has_guard, pat);
118118
check_borrow_conflicts_in_at_patterns(self, pat);
119119
if !self.tcx.features().bindings_after_at {
@@ -146,20 +146,31 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
146146
}
147147

148148
fn check_let(&mut self, pat: &'tcx hir::Pat<'tcx>, scrut: &hir::Expr<'_>) {
149-
self.check_patterns(false, pat);
150-
// FIXME(let_chains, Centril): Exhaustiveness of `let p = e` is unnecessary for soundness.
151-
// Consider dropping the checks here, at least the non-linting part for perf reasons.
149+
self.check_pattern(false, pat);
150+
151+
// This does *not* perform exhaustiveness checking, as it is not required for soundness.
152152
self.check_in_cx(scrut.hir_id, |ref mut cx| {
153+
// Lower the pattern. Note that `lower_pattern` performs some static analysis
154+
// which may result in errors. As such, this cannot be removed.
153155
let mut have_errors = false;
154156
let pattern = self.lower_pattern(cx, pat, &mut have_errors).0;
155157
if have_errors {
156158
return;
157159
}
160+
161+
let tcx = cx.tcx;
162+
let id = pat.hir_id;
163+
let mut seen = Matrix::empty();
164+
// Lint the branch with the user-specified pattern.
165+
let v = PatStack::from_pattern(&pattern);
166+
check_useful_lint(cx, &seen, &v, id, || unreachable_pattern(tcx, pat.span, id, None));
167+
seen.push(v);
168+
// Lint the logical fallback branch.
158169
let wild = cx.pattern_arena.alloc(super::Pat::wildcard_from_ty(pattern.ty));
159170
wild.span = pattern.span;
160-
let arms = &[(pattern, pat.hir_id, false), (wild, pat.hir_id, false)];
161-
let desugar = hir::MatchSource::IfDesugar { contains_else_clause: true };
162-
self.check_union_irrefutable(cx, scrut, arms, desugar)
171+
check_useful_lint(cx, &seen, &PatStack::from_pattern(wild), id, || {
172+
tcx.lint_hir(IRREFUTABLE_LET_PATTERNS, id, pat.span, "irrefutable `let` pattern");
173+
});
163174
});
164175
}
165176

@@ -171,7 +182,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
171182
) {
172183
for arm in arms {
173184
// Check the arm for some things unrelated to exhaustiveness.
174-
self.check_patterns(arm.guard.is_some(), &arm.pat);
185+
self.check_pattern(arm.guard.is_some(), &arm.pat);
175186
}
176187

177188
self.check_in_cx(scrut.hir_id, |ref mut cx| {
@@ -187,27 +198,16 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
187198
return;
188199
}
189200

190-
self.check_union_irrefutable(cx, scrut, &inlined_arms, source);
201+
// Lint on unreachable arms / patterns and build up matrix for exhaustiveness checking.
202+
let matrix = check_arms(cx, &inlined_arms, source);
203+
// Check if the match is exhaustive.
204+
let scrut_ty = self.tables.node_type(scrut.hir_id);
205+
// Note: An empty match isn't the same as an empty matrix for diagnostics purposes,
206+
// since an empty matrix can occur when there are arms, if those arms all have guards.
207+
check_exhaustive(cx, scrut_ty, scrut.span, &matrix, scrut.hir_id, arms.is_empty());
191208
})
192209
}
193210

194-
fn check_union_irrefutable<'p>(
195-
&self,
196-
cx: &mut MatchCheckCtxt<'p, 'tcx>,
197-
scrut: &hir::Expr<'_>,
198-
arms: &[(&'p super::Pat<'tcx>, HirId, bool)],
199-
source: hir::MatchSource,
200-
) {
201-
// Check for unreachable arms.
202-
let matrix = check_arms(cx, &arms, source);
203-
204-
// Check if the match is exhaustive.
205-
let scrut_ty = self.tables.node_type(scrut.hir_id);
206-
// Note: An empty match isn't the same as an empty matrix for diagnostics purposes,
207-
// since an empty matrix can occur when there are arms, if those arms all have guards.
208-
check_exhaustive(cx, scrut_ty, scrut.span, &matrix, scrut.hir_id, arms.is_empty());
209-
}
210-
211211
fn check_irrefutable(&self, pat: &'tcx Pat<'tcx>, origin: &str, sp: Option<Span>) {
212212
self.check_in_cx(pat.hir_id, |ref mut cx| {
213213
let mut has_errors = false;
@@ -362,56 +362,48 @@ fn unreachable_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, catchall: Option<
362362
err.emit();
363363
}
364364

365-
fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::MatchSource) {
366-
let msg = match source {
367-
hir::MatchSource::IfDesugar { .. } | hir::MatchSource::WhileDesugar => {
368-
"irrefutable `let` pattern"
365+
/// Lint on unreachable patterns and delegate to `handle()` for the useless case.
366+
fn check_useful_lint<'p, 'tcx>(
367+
cx: &mut MatchCheckCtxt<'p, 'tcx>,
368+
matrix: &Matrix<'p, 'tcx>,
369+
v: &PatStack<'p, 'tcx>,
370+
hir_id: HirId,
371+
handle: impl FnOnce(),
372+
) {
373+
match is_useful(cx, &matrix, &v, LeaveOutWitness, hir_id, true) {
374+
NotUseful => handle(),
375+
Useful(unreachable_subpatterns) => {
376+
for pat in unreachable_subpatterns {
377+
unreachable_pattern(cx.tcx, pat.span, hir_id, None);
378+
}
369379
}
370-
_ => bug!(),
371-
};
372-
tcx.lint_hir(IRREFUTABLE_LET_PATTERNS, id, span, msg);
380+
UsefulWithWitness(_) => bug!(),
381+
}
373382
}
374383

375-
/// Check for unreachable patterns.
384+
/// Check for unreachable patterns in `match` and build up the matrix for exhaustiveness checking.
376385
fn check_arms<'p, 'tcx>(
377386
cx: &mut MatchCheckCtxt<'p, 'tcx>,
378387
arms: &[(&'p super::Pat<'tcx>, HirId, bool)],
379388
source: hir::MatchSource,
380389
) -> Matrix<'p, 'tcx> {
390+
let tcx = cx.tcx;
381391
let mut seen = Matrix::empty();
382392
let mut catchall = None;
383-
for (arm_index, (pat, id, has_guard)) in arms.iter().copied().enumerate() {
393+
for (pat, id, has_guard) in arms.iter().copied() {
384394
let v = PatStack::from_pattern(pat);
385-
match is_useful(cx, &seen, &v, LeaveOutWitness, id, true) {
386-
NotUseful => {
387-
match source {
388-
hir::MatchSource::IfDesugar { .. } | hir::MatchSource::WhileDesugar => {
389-
// Check which arm we're on.
390-
match arm_index {
391-
// The arm with the user-specified pattern.
392-
0 => unreachable_pattern(cx.tcx, pat.span, id, None),
393-
// The arm with the wildcard pattern.
394-
1 => irrefutable_let_pattern(cx.tcx, pat.span, id, source),
395-
_ => bug!(),
396-
}
397-
}
398-
399-
hir::MatchSource::ForLoopDesugar | hir::MatchSource::Normal => {
400-
unreachable_pattern(cx.tcx, pat.span, id, catchall);
401-
}
402-
403-
// Unreachable patterns in try and await expressions occur when one of
404-
// the arms are an uninhabited type. Which is OK.
405-
hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar => {}
406-
}
407-
}
408-
Useful(unreachable_subpatterns) => {
409-
for pat in unreachable_subpatterns {
410-
unreachable_pattern(cx.tcx, pat.span, id, None);
395+
check_useful_lint(cx, &seen, &v, id, || {
396+
match source {
397+
hir::MatchSource::ForLoopDesugar | hir::MatchSource::Normal => {
398+
unreachable_pattern(tcx, pat.span, id, catchall);
411399
}
400+
// Unreachable patterns in `?` and `.await` expressions occur
401+
// when one of the arms are an uninhabited type. Which is OK.
402+
hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar => {}
403+
// The fallback in `if` & `while`'s desugaring is always useful.
404+
hir::MatchSource::IfDesugar { .. } | hir::MatchSource::WhileDesugar => bug!(),
412405
}
413-
UsefulWithWitness(_) => bug!(),
414-
}
406+
});
415407
if !has_guard {
416408
seen.push(v);
417409
if catchall.is_none() && pat_is_catchall(pat) {

0 commit comments

Comments
 (0)