Skip to content

Commit

Permalink
Auto merge of rust-lang#58380 - estebank:missing-match-pats, r=zackmd…
Browse files Browse the repository at this point in the history
…avis

Point at enum definition when match patterns are not exhaustive

```
error[E0004]: non-exhaustive patterns: type `X` is non-empty
 --> file.rs:9:11
  |
1 | / enum X {
2 | |     A,
  | |     - variant not covered
3 | |     B,
  | |     - variant not covered
4 | |     C,
  | |     - variant not covered
5 | | }
  | |_- `X` defined here
...
9 |       match x {
  |             ^
  |
  = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `B` and `C` not covered
  --> file.rs:11:11
   |
1  | / enum X {
2  | |     A,
3  | |     B,
4  | |     C,
   | |     - not covered
5  | | }
   | |_- `X` defined here
...
11 |       match x {
   |             ^ patterns `C` not covered
```

When a match expression doesn't have patterns covering every variant,
point at the enum's definition span. On a best effort basis, point at the
variant(s) that are missing. This does not handle the case when the missing
pattern is due to a field's enum variants:

```
enum E1 {
    A,
    B,
    C,
}
enum E2 {
    A(E1),
    B,
}
fn foo() {
    match E2::A(E1::A) {
        E2::A(E1::B) => {}
        E2::B => {}
    }
    //~^ ERROR `E2::A(E1::A)` and `E2::A(E1::C)` not handled
}
```

Unify look between match with no arms and match with some missing patterns.

Fix rust-lang#37518.
  • Loading branch information
bors committed Mar 4, 2019
2 parents 9261088 + d651281 commit a9da8fc
Show file tree
Hide file tree
Showing 36 changed files with 401 additions and 96 deletions.
1 change: 0 additions & 1 deletion src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,6 @@ impl<'tcx> Witness<'tcx> {
self.apply_constructor(cx, ctor, ty)
}


/// Constructs a partial witness for a pattern given a list of
/// patterns expanded by the specialization step.
///
Expand Down
144 changes: 116 additions & 28 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc::middle::expr_use_visitor as euv;
use rustc::middle::mem_categorization::cmt_;
use rustc::middle::region;
use rustc::session::Session;
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::{self, Ty, TyCtxt, TyKind};
use rustc::ty::subst::{InternalSubsts, SubstsRef};
use rustc::lint;
use rustc_errors::{Applicability, DiagnosticBuilder};
Expand Down Expand Up @@ -203,25 +203,51 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
// is uninhabited.
let pat_ty = self.tables.node_type(scrut.hir_id);
let module = self.tcx.hir().get_module_parent_by_hir_id(scrut.hir_id);
let mut def_span = None;
let mut missing_variants = vec![];
if inlined_arms.is_empty() {
let scrutinee_is_uninhabited = if self.tcx.features().exhaustive_patterns {
self.tcx.is_ty_uninhabited_from(module, pat_ty)
} else {
match pat_ty.sty {
ty::Never => true,
ty::Adt(def, _) => def.variants.is_empty(),
ty::Adt(def, _) => {
def_span = self.tcx.hir().span_if_local(def.did);
if def.variants.len() < 4 && !def.variants.is_empty() {
// keep around to point at the definition of non-covered variants
missing_variants = def.variants.iter()
.map(|variant| variant.ident)
.collect();
}
def.variants.is_empty()
},
_ => false
}
};
if !scrutinee_is_uninhabited {
// We know the type is inhabited, so this must be wrong
let mut err = create_e0004(self.tcx.sess, scrut.span,
format!("non-exhaustive patterns: type `{}` \
is non-empty",
pat_ty));
span_help!(&mut err, scrut.span,
"ensure that all possible cases are being handled, \
possibly by adding wildcards or more match arms");
let mut err = create_e0004(
self.tcx.sess,
scrut.span,
format!("non-exhaustive patterns: {}", match missing_variants.len() {
0 => format!("type `{}` is non-empty", pat_ty),
1 => format!(
"pattern `{}` of type `{}` is not handled",
missing_variants[0].name,
pat_ty,
),
_ => format!("multiple patterns of type `{}` are not handled", pat_ty),
}),
);
err.help("ensure that all possible cases are being handled, \
possibly by adding wildcards or more match arms");
if let Some(sp) = def_span {
err.span_label(sp, format!("`{}` defined here", pat_ty));
}
// point at the definition of non-covered enum variants
for variant in &missing_variants {
err.span_label(variant.span, "variant not covered");
}
err.emit();
}
// If the type *is* uninhabited, it's vacuously exhaustive
Expand Down Expand Up @@ -263,7 +289,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
};

let pattern_string = witness[0].single_pattern().to_string();
let mut diag = struct_span_err!(
let mut err = struct_span_err!(
self.tcx.sess, pat.span, E0005,
"refutable pattern in {}: `{}` not covered",
origin, pattern_string
Expand All @@ -276,8 +302,13 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
}
_ => format!("pattern `{}` not covered", pattern_string),
};
diag.span_label(pat.span, label_msg);
diag.emit();
err.span_label(pat.span, label_msg);
if let ty::Adt(def, _) = pattern_ty.sty {
if let Some(sp) = self.tcx.hir().span_if_local(def.did){
err.span_label(sp, format!("`{}` defined here", pattern_ty));
}
}
err.emit();
});
}
}
Expand Down Expand Up @@ -331,10 +362,11 @@ fn pat_is_catchall(pat: &Pat) -> bool {
}

// Check for unreachable patterns
fn check_arms<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
arms: &[(Vec<(&'a Pattern<'tcx>, &hir::Pat)>, Option<&hir::Expr>)],
source: hir::MatchSource)
{
fn check_arms<'a, 'tcx>(
cx: &mut MatchCheckCtxt<'a, 'tcx>,
arms: &[(Vec<(&'a Pattern<'tcx>, &hir::Pat)>, Option<&hir::Expr>)],
source: hir::MatchSource,
) {
let mut seen = Matrix::empty();
let mut catchall = None;
for (arm_index, &(ref pats, guard)) in arms.iter().enumerate() {
Expand Down Expand Up @@ -410,10 +442,12 @@ fn check_arms<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
}
}

fn check_exhaustive<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
scrut_ty: Ty<'tcx>,
sp: Span,
matrix: &Matrix<'p, 'tcx>) {
fn check_exhaustive<'p, 'a: 'p, 'tcx: 'a>(
cx: &mut MatchCheckCtxt<'a, 'tcx>,
scrut_ty: Ty<'tcx>,
sp: Span,
matrix: &Matrix<'p, 'tcx>,
) {
let wild_pattern = Pattern {
ty: scrut_ty,
span: DUMMY_SP,
Expand Down Expand Up @@ -447,11 +481,26 @@ fn check_exhaustive<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
1 => format!("pattern {} not covered", joined_patterns),
_ => format!("patterns {} not covered", joined_patterns),
};
create_e0004(cx.tcx.sess, sp,
format!("non-exhaustive patterns: {} not covered",
joined_patterns))
.span_label(sp, label_text)
.emit();
let mut err = create_e0004(cx.tcx.sess, sp, format!(
"non-exhaustive patterns: {} not covered",
joined_patterns,
));
err.span_label(sp, label_text);
// point at the definition of non-covered enum variants
if let ty::Adt(def, _) = scrut_ty.sty {
if let Some(sp) = cx.tcx.hir().span_if_local(def.did){
err.span_label(sp, format!("`{}` defined here", scrut_ty));
}
}
let patterns = witnesses.iter().map(|p| (**p).clone()).collect::<Vec<Pattern<'_>>>();
if patterns.len() < 4 {
for sp in maybe_point_at_variant(cx, &scrut_ty.sty, patterns.as_slice()) {
err.span_label(sp, "not covered");
}
}
err.help("ensure that all possible cases are being handled, \
possibly by adding wildcards or more match arms");
err.emit();
}
NotUseful => {
// This is good, wildcard pattern isn't reachable
Expand All @@ -460,10 +509,49 @@ fn check_exhaustive<'p, 'a: 'p, 'tcx: 'a>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
}
}

fn maybe_point_at_variant(
cx: &mut MatchCheckCtxt<'a, 'tcx>,
sty: &TyKind<'tcx>,
patterns: &[Pattern<'_>],
) -> Vec<Span> {
let mut covered = vec![];
if let ty::Adt(def, _) = sty {
// Don't point at variants that have already been covered due to other patterns to avoid
// visual clutter
for pattern in patterns {
let pk: &PatternKind<'_> = &pattern.kind;
if let PatternKind::Variant { adt_def, variant_index, subpatterns, .. } = pk {
if adt_def.did == def.did {
let sp = def.variants[*variant_index].ident.span;
if covered.contains(&sp) {
continue;
}
covered.push(sp);
let subpatterns = subpatterns.iter()
.map(|field_pattern| field_pattern.pattern.clone())
.collect::<Vec<_>>();
covered.extend(
maybe_point_at_variant(cx, sty, subpatterns.as_slice()),
);
}
}
if let PatternKind::Leaf { subpatterns } = pk {
let subpatterns = subpatterns.iter()
.map(|field_pattern| field_pattern.pattern.clone())
.collect::<Vec<_>>();
covered.extend(maybe_point_at_variant(cx, sty, subpatterns.as_slice()));
}
}
}
covered
}

// Legality of move bindings checking
fn check_legality_of_move_bindings(cx: &MatchVisitor<'_, '_>,
has_guard: bool,
pats: &[P<Pat>]) {
fn check_legality_of_move_bindings(
cx: &MatchVisitor<'_, '_>,
has_guard: bool,
pats: &[P<Pat>],
) {
let mut by_ref_span = None;
for pat in pats {
pat.each_binding(|_, hir_id, span, _path| {
Expand Down
12 changes: 2 additions & 10 deletions src/test/ui/always-inhabited-union-ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,15 @@ error[E0004]: non-exhaustive patterns: type `&'static !` is non-empty
LL | match uninhab_ref() {
| ^^^^^^^^^^^^^
|
help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
--> $DIR/always-inhabited-union-ref.rs:23:11
|
LL | match uninhab_ref() {
| ^^^^^^^^^^^^^
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: type `Foo` is non-empty
--> $DIR/always-inhabited-union-ref.rs:27:11
|
LL | match uninhab_union() {
| ^^^^^^^^^^^^^^^
|
help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
--> $DIR/always-inhabited-union-ref.rs:27:11
|
LL | match uninhab_union() {
| ^^^^^^^^^^^^^^^
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error: aborting due to 2 previous errors

Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/check_match/issue-35609.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,70 @@ error[E0004]: non-exhaustive patterns: `(B, _)`, `(C, _)`, `(D, _)` and 2 more n
|
LL | match (A, ()) { //~ ERROR non-exhaustive
| ^^^^^^^ patterns `(B, _)`, `(C, _)`, `(D, _)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `(_, B)`, `(_, C)`, `(_, D)` and 2 more not covered
--> $DIR/issue-35609.rs:14:11
|
LL | match (A, A) { //~ ERROR non-exhaustive
| ^^^^^^ patterns `(_, B)`, `(_, C)`, `(_, D)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered
--> $DIR/issue-35609.rs:18:11
|
LL | match ((A, ()), ()) { //~ ERROR non-exhaustive
| ^^^^^^^^^^^^^ patterns `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered
--> $DIR/issue-35609.rs:22:11
|
LL | match ((A, ()), A) { //~ ERROR non-exhaustive
| ^^^^^^^^^^^^ patterns `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered
--> $DIR/issue-35609.rs:26:11
|
LL | match ((A, ()), ()) { //~ ERROR non-exhaustive
| ^^^^^^^^^^^^^ patterns `((B, _), _)`, `((C, _), _)`, `((D, _), _)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `S(B, _)`, `S(C, _)`, `S(D, _)` and 2 more not covered
--> $DIR/issue-35609.rs:31:11
|
LL | struct S(Enum, ());
| ------------------- `S` defined here
...
LL | match S(A, ()) { //~ ERROR non-exhaustive
| ^^^^^^^^ patterns `S(B, _)`, `S(C, _)`, `S(D, _)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `Sd { x: B, .. }`, `Sd { x: C, .. }`, `Sd { x: D, .. }` and 2 more not covered
--> $DIR/issue-35609.rs:35:11
|
LL | struct Sd { x: Enum, y: () }
| ---------------------------- `Sd` defined here
...
LL | match (Sd { x: A, y: () }) { //~ ERROR non-exhaustive
| ^^^^^^^^^^^^^^^^^^^^ patterns `Sd { x: B, .. }`, `Sd { x: C, .. }`, `Sd { x: D, .. }` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `Some(B)`, `Some(C)`, `Some(D)` and 2 more not covered
--> $DIR/issue-35609.rs:39:11
|
LL | match Some(A) { //~ ERROR non-exhaustive
| ^^^^^^^ patterns `Some(B)`, `Some(C)`, `Some(D)` and 2 more not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error: aborting due to 8 previous errors

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/consts/match_ice.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0004]: non-exhaustive patterns: `&S` not covered
|
LL | match C { //~ ERROR non-exhaustive
| ^ pattern `&S` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error: aborting due to previous error

Expand Down
11 changes: 9 additions & 2 deletions src/test/ui/empty/empty-never-array.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
error[E0005]: refutable pattern in local binding: `T(_, _)` not covered
--> $DIR/empty-never-array.rs:10:9
|
LL | let Helper::U(u) = Helper::T(t, []);
| ^^^^^^^^^^^^ pattern `T(_, _)` not covered
LL | / enum Helper<T, U> {
LL | | T(T, [!; 0]),
LL | | #[allow(dead_code)]
LL | | U(U),
LL | | }
| |_- `Helper<T, U>` defined here
...
LL | let Helper::U(u) = Helper::T(t, []);
| ^^^^^^^^^^^^ pattern `T(_, _)` not covered

error: aborting due to previous error

Expand Down
8 changes: 2 additions & 6 deletions src/test/ui/error-codes/E0004-2.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
error[E0004]: non-exhaustive patterns: type `std::option::Option<i32>` is non-empty
error[E0004]: non-exhaustive patterns: multiple patterns of type `std::option::Option<i32>` are not handled
--> $DIR/E0004-2.rs:4:11
|
LL | match x { } //~ ERROR E0004
| ^
|
help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
--> $DIR/E0004-2.rs:4:11
|
LL | match x { } //~ ERROR E0004
| ^
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error: aborting due to previous error

Expand Down
13 changes: 11 additions & 2 deletions src/test/ui/error-codes/E0004.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
error[E0004]: non-exhaustive patterns: `HastaLaVistaBaby` not covered
--> $DIR/E0004.rs:9:11
|
LL | match x { //~ ERROR E0004
| ^ pattern `HastaLaVistaBaby` not covered
LL | / enum Terminator {
LL | | HastaLaVistaBaby,
| | ---------------- not covered
LL | | TalkToMyHand,
LL | | }
| |_- `Terminator` defined here
...
LL | match x { //~ ERROR E0004
| ^ pattern `HastaLaVistaBaby` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error: aborting due to previous error

Expand Down
Loading

0 comments on commit a9da8fc

Please sign in to comment.