Point at enum definition when match patterns are not exhaustive #58380

estebank · 2019-02-11T17:57:39Z

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 #37518.

rust-highfive · 2019-02-11T17:57:43Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2019-02-11T17:58:19Z

r? @zackmdavis

zackmdavis

naming/commentary quibbles, otherwise r=me

zackmdavis · 2019-02-16T08:18:35Z

src/librustc_mir/hair/pattern/check_match.rs

@@ -204,25 +204,42 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
            // is uninhabited.
            let pat_ty = self.tables.node_id_to_type(scrut.hir_id);
            let module = self.tcx.hir().get_module_parent(scrut.id);
+            let mut def_span = None;
+            let mut missing_variants = vec![];


I usually write empty vectors as Vec::new() (thinking, "no need to the macro sugar when there aren't any elements to initialize"), but rustfmt doesn't seem to have an opinion. (I guess you could argue that it's technically a semantic change.)

zackmdavis · 2019-02-16T08:26:17Z

src/librustc_mir/hair/pattern/check_match.rs

-                               "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: type `{}` is non-empty",


(I realized that this was already here and not a part of the present changeset, but) do we even want to bother saying "type is non-empty" here? If we don't expect users to think or care about the empty enum very often, this might not be the most helpful time to educate them.

changed to "non-exhaustive patterns: variant Foo of type Bar is not handled"

zackmdavis · 2019-02-16T08:28:21Z

src/librustc_mir/hair/pattern/check_match.rs

+                            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.span)


Can we call the variable missing_variant_spans or similar? (I balked at err.span_label(*variant on line 241.)

collected the variant idents instead, keeping the vector name the same

src/librustc_mir/hair/pattern/check_match.rs

zackmdavis · 2019-02-16T08:42:15Z

src/test/ui/error-codes/E0004.stderr

+LL | | }
+   | |_- `Terminator` defined here
+...
+LL |       match x { //~ ERROR E0004


(possibly a bad idea, probably out of scope for this PR) I find it counterintuitive for the primary span of a "non-exaustive patterns" error to point to the x in match x, rather than the body of match arms. (That could get very visually cluttered, though, especially with the secondary definition-spans introduced here. Also, pointing at the discriminant does seem intuitive when it's a pattern rather than an opaque variable, as in the examples in src/test/ui/check_match/issue-35609.stderr.)

I have the same thoughts you do, but I lay on the side of using the smallest possible primary span, for VSCode's sake.

``` 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.

estebank · 2019-03-04T01:10:56Z

@bors r=zackmdavis

bors · 2019-03-04T01:10:57Z

📌 Commit d651281 has been approved by zackmdavis

bors · 2019-03-04T04:46:45Z

⌛ Testing commit d651281 with merge a9da8fc...

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 #37518.

bors · 2019-03-04T07:40:31Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: zackmdavis
Pushing a9da8fc to master...

rust-highfive assigned eddyb Feb 11, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 11, 2019

rust-highfive assigned zackmdavis and unassigned eddyb Feb 11, 2019

estebank force-pushed the missing-match-pats branch from 516ab7d to 0ba0740 Compare February 11, 2019 17:59

This comment has been minimized.

Sign in to view

zackmdavis approved these changes Feb 16, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

estebank added 3 commits March 2, 2019 16:45

Use anonymous explicit lifetimes

509bb0a

Reword error message

d651281

estebank force-pushed the missing-match-pats branch from acba4dc to d651281 Compare March 3, 2019 01:50

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 4, 2019

bors merged commit d651281 into rust-lang:master Mar 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Point at enum definition when match patterns are not exhaustive #58380

Point at enum definition when match patterns are not exhaustive #58380

estebank commented Feb 11, 2019

rust-highfive commented Feb 11, 2019

estebank commented Feb 11, 2019

This comment has been minimized.

zackmdavis left a comment

zackmdavis Feb 16, 2019

zackmdavis Feb 16, 2019

estebank Mar 3, 2019

zackmdavis Feb 16, 2019

estebank Mar 3, 2019

zackmdavis Feb 16, 2019

estebank Mar 3, 2019

This comment has been minimized.

estebank commented Mar 4, 2019

bors commented Mar 4, 2019

bors commented Mar 4, 2019

bors commented Mar 4, 2019

Point at enum definition when match patterns are not exhaustive #58380

Point at enum definition when match patterns are not exhaustive #58380

Conversation

estebank commented Feb 11, 2019

rust-highfive commented Feb 11, 2019

estebank commented Feb 11, 2019

This comment has been minimized.

zackmdavis left a comment

Choose a reason for hiding this comment

zackmdavis Feb 16, 2019

Choose a reason for hiding this comment

zackmdavis Feb 16, 2019

Choose a reason for hiding this comment

estebank Mar 3, 2019

Choose a reason for hiding this comment

zackmdavis Feb 16, 2019

Choose a reason for hiding this comment

estebank Mar 3, 2019

Choose a reason for hiding this comment

zackmdavis Feb 16, 2019

Choose a reason for hiding this comment

estebank Mar 3, 2019

Choose a reason for hiding this comment

This comment has been minimized.

estebank commented Mar 4, 2019

bors commented Mar 4, 2019

bors commented Mar 4, 2019

bors commented Mar 4, 2019