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

Tweak output for non-exhaustive match expression #91993

Merged
merged 5 commits into from
Mar 8, 2022
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
143 changes: 123 additions & 20 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_session::lint::builtin::{
};
use rustc_session::Session;
use rustc_span::source_map::Spanned;
use rustc_span::{DesugaringKind, ExpnKind, Span};
use rustc_span::{DesugaringKind, ExpnKind, MultiSpan, Span};

crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) {
let body_id = match def_id.as_local() {
Expand Down Expand Up @@ -64,7 +64,9 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, '_, 'tcx> {
fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) {
intravisit::walk_expr(self, ex);
match &ex.kind {
hir::ExprKind::Match(scrut, arms, source) => self.check_match(scrut, arms, *source),
hir::ExprKind::Match(scrut, arms, source) => {
self.check_match(scrut, arms, *source, ex.span)
}
hir::ExprKind::Let(hir::Let { pat, init, span, .. }) => {
self.check_let(pat, init, *span)
}
Expand Down Expand Up @@ -163,6 +165,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
scrut: &hir::Expr<'_>,
hir_arms: &'tcx [hir::Arm<'tcx>],
source: hir::MatchSource,
expr_span: Span,
) {
let mut cx = self.new_cx(scrut.hir_id);

Expand Down Expand Up @@ -208,15 +211,14 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
}

// Check if the match is exhaustive.
let is_empty_match = arms.is_empty();
let witnesses = report.non_exhaustiveness_witnesses;
if !witnesses.is_empty() {
if source == hir::MatchSource::ForLoopDesugar && hir_arms.len() == 2 {
// the for loop pattern is not irrefutable
let pat = hir_arms[1].pat.for_loop_some().unwrap();
self.check_irrefutable(pat, "`for` loop binding", None);
} else {
non_exhaustive_match(&cx, scrut_ty, scrut.span, witnesses, is_empty_match);
non_exhaustive_match(&cx, scrut_ty, scrut.span, witnesses, hir_arms, expr_span);
}
}
}
Expand Down Expand Up @@ -334,7 +336,7 @@ fn check_for_bindings_named_same_as_variants(
let ty_path = cx.tcx.def_path_str(edef.did);
let mut err = lint.build(&format!(
"pattern binding `{}` is named the same as one \
of the variants of the type `{}`",
of the variants of the type `{}`",
ident, ty_path
));
err.code(error_code!(E0170));
Expand Down Expand Up @@ -494,21 +496,26 @@ fn non_exhaustive_match<'p, 'tcx>(
scrut_ty: Ty<'tcx>,
sp: Span,
witnesses: Vec<DeconstructedPat<'p, 'tcx>>,
is_empty_match: bool,
arms: &[hir::Arm<'tcx>],
expr_span: Span,
) {
let is_empty_match = arms.is_empty();
let non_empty_enum = match scrut_ty.kind() {
ty::Adt(def, _) => def.is_enum() && !def.variants.is_empty(),
_ => false,
};
// In the case of an empty match, replace the '`_` not covered' diagnostic with something more
// informative.
let mut err;
let pattern;
let mut patterns_len = 0;
if is_empty_match && !non_empty_enum {
err = create_e0004(
cx.tcx.sess,
sp,
format!("non-exhaustive patterns: type `{}` is non-empty", scrut_ty),
);
pattern = "_".to_string();
} else {
let joined_patterns = joined_uncovered_patterns(cx, &witnesses);
err = create_e0004(
Expand All @@ -517,6 +524,16 @@ fn non_exhaustive_match<'p, 'tcx>(
format!("non-exhaustive patterns: {} not covered", joined_patterns),
);
err.span_label(sp, pattern_not_covered_label(&witnesses, &joined_patterns));
patterns_len = witnesses.len();
pattern = if witnesses.len() < 4 {
witnesses
.iter()
.map(|witness| witness.to_pat(cx).to_string())
.collect::<Vec<String>>()
.join(" | ")
} else {
"_".to_string()
};
};

let is_variant_list_non_exhaustive = match scrut_ty.kind() {
Expand All @@ -525,10 +542,6 @@ fn non_exhaustive_match<'p, 'tcx>(
};

adt_defined_here(cx, &mut err, scrut_ty, &witnesses);
err.help(
"ensure that all possible cases are being handled, \
possibly by adding wildcards or more match arms",
);
err.note(&format!(
"the matched value is of type `{}`{}",
scrut_ty,
Expand All @@ -540,14 +553,14 @@ fn non_exhaustive_match<'p, 'tcx>(
&& matches!(witnesses[0].ctor(), Constructor::NonExhaustive)
{
err.note(&format!(
"`{}` does not have a fixed maximum value, \
so a wildcard `_` is necessary to match exhaustively",
"`{}` does not have a fixed maximum value, so a wildcard `_` is necessary to match \
exhaustively",
scrut_ty,
));
if cx.tcx.sess.is_nightly_build() {
err.help(&format!(
"add `#![feature(precise_pointer_size_matching)]` \
to the crate attributes to enable precise `{}` matching",
"add `#![feature(precise_pointer_size_matching)]` to the crate attributes to \
enable precise `{}` matching",
scrut_ty,
));
}
Expand All @@ -557,6 +570,84 @@ fn non_exhaustive_match<'p, 'tcx>(
err.note("references are always considered inhabited");
}
}

let mut suggestion = None;
let sm = cx.tcx.sess.source_map();
match arms {
[] if sp.ctxt() == expr_span.ctxt() => {
// Get the span for the empty match body `{}`.
let (indentation, more) = if let Some(snippet) = sm.indentation_before(sp) {
(format!("\n{}", snippet), " ")
} else {
(" ".to_string(), "")
};
suggestion = Some((
sp.shrink_to_hi().with_hi(expr_span.hi()),
format!(
" {{{indentation}{more}{pattern} => todo!(),{indentation}}}",
indentation = indentation,
more = more,
pattern = pattern,
),
));
}
[only] => {
let pre_indentation = if let (Some(snippet), true) = (
sm.indentation_before(only.span),
sm.is_multiline(sp.shrink_to_hi().with_hi(only.span.lo())),
) {
format!("\n{}", snippet)
} else {
" ".to_string()
};
let comma = if matches!(only.body.kind, hir::ExprKind::Block(..)) { "" } else { "," };
suggestion = Some((
only.span.shrink_to_hi(),
format!("{}{}{} => todo!()", comma, pre_indentation, pattern),
));
}
[.., prev, last] if prev.span.ctxt() == last.span.ctxt() => {
if let Ok(snippet) = sm.span_to_snippet(prev.span.between(last.span)) {
let comma =
if matches!(last.body.kind, hir::ExprKind::Block(..)) { "" } else { "," };
suggestion = Some((
last.span.shrink_to_hi(),
format!(
"{}{}{} => todo!()",
comma,
snippet.strip_prefix(",").unwrap_or(&snippet),
pattern
),
));
}
}
_ => {}
}

let msg = format!(
"ensure that all possible cases are being handled by adding a match arm with a wildcard \
pattern{}{}",
if patterns_len > 1 && patterns_len < 4 && suggestion.is_some() {
", a match arm with multiple or-patterns"
} else {
// we are either not suggesting anything, or suggesting `_`
""
},
match patterns_len {
// non-exhaustive enum case
0 if suggestion.is_some() => " as shown",
0 => "",
1 if suggestion.is_some() => " or an explicit pattern as shown",
1 => " or an explicit pattern",
_ if suggestion.is_some() => " as shown, or multiple match arms",
_ => " or multiple match arms",
},
);
if let Some((span, sugg)) = suggestion {
err.span_suggestion_verbose(span, &msg, sugg, Applicability::HasPlaceholders);
} else {
err.help(&msg);
}
err.emit();
}

Expand Down Expand Up @@ -597,15 +688,27 @@ fn adt_defined_here<'p, 'tcx>(
) {
let ty = ty.peel_refs();
if let ty::Adt(def, _) = ty.kind() {
if let Some(sp) = cx.tcx.hir().span_if_local(def.did) {
err.span_label(sp, format!("`{}` defined here", ty));
}

if witnesses.len() < 4 {
let mut spans = vec![];
if witnesses.len() < 5 {
for sp in maybe_point_at_variant(cx, def, witnesses.iter()) {
err.span_label(sp, "not covered");
spans.push(sp);
}
}
let def_span = cx
.tcx
.hir()
.get_if_local(def.did)
.and_then(|node| node.ident())
.map(|ident| ident.span)
.unwrap_or_else(|| cx.tcx.def_span(def.did));
let mut span: MultiSpan =
if spans.is_empty() { def_span.into() } else { spans.clone().into() };

span.push_span_label(def_span, String::new());
for pat in spans {
span.push_span_label(pat, "not covered".to_string());
}
err.span_note(span, &format!("`{}` defined here", ty));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,38 @@
error[E0004]: non-exhaustive patterns: `Opcode(0_u8)` and `Opcode(2_u8..=u8::MAX)` not covered
--> $DIR/issue-88331.rs:11:20
|
LL | pub struct Opcode(pub u8);
| -------------------------- `Opcode` defined here
...
LL | move |i| match msg_type {
| ^^^^^^^^ patterns `Opcode(0_u8)` and `Opcode(2_u8..=u8::MAX)` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
note: `Opcode` defined here
--> $DIR/issue-88331.rs:4:12
|
LL | pub struct Opcode(pub u8);
| ^^^^^^
= note: the matched value is of type `Opcode`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
|
LL ~ Opcode::OP1 => unimplemented!(),
LL ~ Opcode(0_u8) | Opcode(2_u8..=u8::MAX) => todo!(),
|

error[E0004]: non-exhaustive patterns: `Opcode2(Opcode(0_u8))` and `Opcode2(Opcode(2_u8..=u8::MAX))` not covered
--> $DIR/issue-88331.rs:27:20
|
LL | pub struct Opcode2(Opcode);
| --------------------------- `Opcode2` defined here
...
LL | move |i| match msg_type {
| ^^^^^^^^ patterns `Opcode2(Opcode(0_u8))` and `Opcode2(Opcode(2_u8..=u8::MAX))` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
note: `Opcode2` defined here
--> $DIR/issue-88331.rs:18:12
|
LL | pub struct Opcode2(Opcode);
| ^^^^^^^
= note: the matched value is of type `Opcode2`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
|
LL ~ Opcode2::OP2=> unimplemented!(),
LL ~ Opcode2(Opcode(0_u8)) | Opcode2(Opcode(2_u8..=u8::MAX)) => todo!(),
|

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
@@ -1,35 +1,55 @@
error[E0004]: non-exhaustive patterns: `B` not covered
--> $DIR/non-exhaustive-match.rs:26:25
|
LL | enum L1 { A, B }
| ----------------
| | |
| | not covered
| `L1` defined here
...
LL | let _b = || { match l1 { L1::A => () } };
| ^^ pattern `B` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
note: `L1` defined here
--> $DIR/non-exhaustive-match.rs:12:14
|
LL | enum L1 { A, B }
| -- ^ not covered
= note: the matched value is of type `L1`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
|
LL | let _b = || { match l1 { L1::A => (), B => todo!() } };
| ++++++++++++++

error[E0004]: non-exhaustive patterns: type `E1` is non-empty
--> $DIR/non-exhaustive-match.rs:37:25
|
LL | let _d = || { match e1 {} };
| ^^
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
note: `E1` defined here
--> $DIR/auxiliary/match_non_exhaustive_lib.rs:2:1
|
LL | pub enum E1 {}
| ^^^^^^^^^^^^^^
= note: the matched value is of type `E1`, which is marked as non-exhaustive
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
|
LL ~ let _d = || { match e1 {
LL + _ => todo!(),
LL ~ } };
|

error[E0004]: non-exhaustive patterns: `_` not covered
--> $DIR/non-exhaustive-match.rs:39:25
|
LL | let _e = || { match e2 { E2::A => (), E2::B => () } };
| ^^ pattern `_` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
note: `E2` defined here
--> $DIR/auxiliary/match_non_exhaustive_lib.rs:5:1
|
LL | pub enum E2 { A, B }
| ^^^^^^^^^^^^^^^^^^^^
= note: the matched value is of type `E2`, which is marked as non-exhaustive
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
|
LL | let _e = || { match e2 { E2::A => (), E2::B => (), _ => todo!() } };
| ++++++++++++++

error[E0505]: cannot move out of `e3` because it is borrowed
--> $DIR/non-exhaustive-match.rs:46:22
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ error[E0004]: non-exhaustive patterns: type `u8` is non-empty
LL | let c1 = || match x { };
| ^
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
= note: the matched value is of type `u8`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
|
LL ~ let c1 = || match x {
LL + _ => todo!(),
LL ~ };
|

error[E0381]: use of possibly-uninitialized variable: `x`
--> $DIR/pattern-matching-should-fail.rs:8:23
Expand Down
Loading