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

Inconsistent "pattern doesn't bind x" messages in or patterns #39698

Closed
osa1 opened this issue Feb 9, 2017 · 9 comments
Closed

Inconsistent "pattern doesn't bind x" messages in or patterns #39698

osa1 opened this issue Feb 9, 2017 · 9 comments

Comments

@osa1
Copy link
Contributor

osa1 commented Feb 9, 2017

Compiling this using rustc nightly (tried with 29dece1 2017-02-08)

enum T {
    T1(i32),
    T2(i32),
}

fn main() {
    match T::T1(123) {
        T::T1(a) | T::T2(b) => { println!("{:?}", a); }
    }
}

prints

error[E0408]: variable `a` from pattern #1 is not bound in pattern #2
 --> pat.rs:8:20
  |
8 |         T::T1(a) | T::T2(b) => { println!("{:?}", a); }
  |                    ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
 --> pat.rs:8:26
  |
8 |         T::T1(a) | T::T2(b) => { println!("{:?}", a); }
  |                          ^ pattern doesn't bind `b`

This is confusing, ideally I think second error message would be


error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
 --> pat.rs:8:26
  |
8 |         T::T1(a) | T::T2(b) => { println!("{:?}", a); }
  |         ^^^^^^^^ pattern doesn't bind `b`

At least not pointing at b while saying the pattern doesn't bind b would be better.

@estebank
Copy link
Contributor

estebank commented Feb 9, 2017

There're two error messages because they are pointing out two different problems:

  • the second pattern doesn't bound a variable from the first pattern
  • the second pattern bounds a variable that isn't present in the first pattern

If your code where

enum T {
    T1(i32),
    T2(),
}

fn main() {
    match T::T1(123) {
        T::T1(a) | T::T2() => { println!("{:?}", a); }
    }
}

the output would be

error[E0408]: variable `a` from pattern #1 is not bound in pattern #2
 --> pat.rs:8:20
  |
8 |         T::T1(a) | T::T2() => { println!("{:?}", a); }
  |                    ^^^^^^^ pattern doesn't bind `a`

That being said, I think the output should be changed to something along the lines of

error[E0408]: variable `a` from pattern #1 is not bound in pattern #2
 --> file.rs:8:20
  |
8 |         T::T1(a) | T::T2(b) => { println!("{:?}", a); }
  |               -    ^^^^^^^^ pattern doesn't bind `a`
  |               |
  |               unbound variable

error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
 --> file.rs:8:26
  |
8 |         T::T1(a) | T::T2(b) => { println!("{:?}", a); }
  |         --------         ^ unbound variable
  |         |
  |         pattern doesn't bind `b`

It can probably be improved further for cases with more patterns. The current output is

error[E0408]: variable `a` from pattern #1 is not bound in pattern #2
  --> file.rs:10:20
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |                    ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
  --> file.rs:10:26
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |                          ^ pattern doesn't bind `b`

error[E0408]: variable `a` from pattern #1 is not bound in pattern #3
  --> file.rs:10:31
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |                               ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `c` from pattern #3 is not bound in pattern #1
  --> file.rs:10:37
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |                                     ^ pattern doesn't bind `c`

and I feel it should be closer to

error[E0408]: variable `a` from pattern #1 is not bound in patterns #2 and #3
  --> file.rs:10:20
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |               -    ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `a`
   |               |    |
   |               |    pattern doesn't bind `a`
   |               missing variable

error[E0408]: variable `b` from pattern #2 and variable `c` from pattern #3 are not bound in pattern #1
  --> file.rs:10:26
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |         --------         ^          ^ unused variable
   |         |                |
   |         |                unused variable
   |         pattern doesn't bind `b` or `c`

cc @jonathandturner thoughts?

@estebank
Copy link
Contributor

@osa1, could you check out #39713 and tell me if that wording makes more sense to you?

Colored output for that PR:

@sophiajt
Copy link
Contributor

I think I like the general idea, but I'm not sure about the specific approach. For example, in this one:

error[E0408]: variable `a` from pattern #1 is not bound in patterns #2 and #3
  --> file.rs:10:20
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |               -    ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `a`
   |               |    |
   |               |    pattern doesn't bind `a`
   |               missing variable

You put missing variable on places where that variable isn't missing, which is a bit confusing. Maybe we can work say something like variable not in all patterns?

Also, does it really say this?

error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
  --> file.rs:10:26
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |                          ^ pattern doesn't bind `b`

If so, that sounds like a bug unto itself.

@estebank
Copy link
Contributor

You put missing variable on places where that variable isn't missing, which is a bit confusing. Maybe we can work say something like variable not in all patterns?

I'm ok with changing the wording to that. Do you want me to set you as reviewer?

Also, does it really say this? If so, that sounds like a bug unto itself.

Yeah, and I agree. That's the reason I actually started the PR (the grouping was merely gold plating).

@estebank
Copy link
Contributor

estebank commented Feb 10, 2017

Another option is to do

error[E0408]: multiple variables not present in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ------^--^-   ---------^-   ------^-
   |               |  |             |          |
   |               |  |             |          variable not in all patterns
   |               |  |             variable not in all patterns
   |               |  variable not in all patterns
   |               variable not in all patterns

error: aborting due to 1 previous errors

which has less vertical clutter, but loses clarity on which variable is missing on which patterns, as well as a more generic error message.

@osa1
Copy link
Contributor Author

osa1 commented Feb 10, 2017

I added a comment in #39713.

I should add, maybe I'm unfamiliar with Rust's error messages and this is actually consistent in Rust's other messages. I just realized this while trying some language features and wanted to report as it looked weird. The PR doesn't actually solve the problem I tried to describe, maybe because I did a bad job at that. I tried to explain further in the PR.

@sophiajt
Copy link
Contributor

sophiajt commented Feb 12, 2017

@estebank - having all of them in the same error seems a bit confusing. I don't think we do that with many errors, so it'd be something users wouldn't expect. Maybe in the future we can work on more compact errors?

For now, I think combining our ideas together, something like this would be a decent solution for the time being:

error[E0408]: variable `a` from pattern #1 is not bound in patterns #2 and #3
  --> file.rs:10:20
   |
10 |         T::T1(a) | T::T2(b) | T::T3(c) | T::T4(a)=> { println!("{:?}", a); }
   |               ^    --------   -------- pattern doesn't bind `a`
   |               |    |
   |               |    pattern doesn't bind `a`
   |               variable not in all patterns

I switched primary and secondary, also, from what you had. I'm not 100% sure this is right, but it looked better this morning.

@estebank
Copy link
Contributor

@osa1 does the new output in #39713 make sense to you?

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 17, 2017
…rner

Clean up "pattern doesn't bind x" messages

Group "missing variable bind" spans in `or` matches and clarify wording
for the two possible cases: when a variable from the first pattern is
not in any of the subsequent patterns, and when a variable in any of the
other patterns is not in the first one.

Before:

```rust
error[E0408]: variable `a` from pattern #1 is not bound in pattern rust-lang#2
  --> file.rs:10:23
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                       ^^^^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` from pattern rust-lang#2 is not bound in pattern #1
  --> file.rs:10:32
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                ^ pattern doesn't bind `b`

error[E0408]: variable `a` from pattern #1 is not bound in pattern rust-lang#3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `d` from pattern #1 is not bound in pattern rust-lang#3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `d`

error[E0408]: variable `c` from pattern rust-lang#3 is not bound in pattern #1
  --> file.rs:10:43
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                           ^ pattern doesn't bind `c`

error[E0408]: variable `d` from pattern #1 is not bound in pattern rust-lang#4
  --> file.rs:10:48
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                                ^^^^^^^^ pattern doesn't bind `d`

error: aborting due to 6 previous errors
```

After:

```rust
error[E0408]: variable `d` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                  -          -       ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `d`
   |                  |          |       |
   |                  |          |       pattern doesn't bind `d`
   |                  |          variable not in all patterns
   |                  variable not in all patterns

error[E0408]: variable `c` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:48
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ^^^^^^^^^^^   ^^^^^^^^^^^         -    ^^^^^^^^ pattern doesn't bind `c`
   |         |             |                   |
   |         |             |                   variable not in all patterns
   |         |             pattern doesn't bind `c`
   |         pattern doesn't bind `c`

error[E0408]: variable `a` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |               -       ^^^^^^^^^^^   ^^^^^^^^         - variable not in all patterns
   |               |       |             |
   |               |       |             pattern doesn't bind `a`
   |               |       pattern doesn't bind `a`
   |               variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ^^^^^^^^^^^            -    ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `b`
   |         |                      |    |
   |         |                      |    pattern doesn't bind `b`
   |         |                      variable not in all patterns
   |         pattern doesn't bind `b`

error: aborting due to 4 previous errors
```

Fixes rust-lang#39698.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 6, 2017
…rner

Clean up "pattern doesn't bind x" messages

Group "missing variable bind" spans in `or` matches and clarify wording
for the two possible cases: when a variable from the first pattern is
not in any of the subsequent patterns, and when a variable in any of the
other patterns is not in the first one.

Before:

```rust
error[E0408]: variable `a` from pattern #1 is not bound in pattern rust-lang#2
  --> file.rs:10:23
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                       ^^^^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` from pattern rust-lang#2 is not bound in pattern #1
  --> file.rs:10:32
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                ^ pattern doesn't bind `b`

error[E0408]: variable `a` from pattern #1 is not bound in pattern rust-lang#3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `d` from pattern #1 is not bound in pattern rust-lang#3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `d`

error[E0408]: variable `c` from pattern rust-lang#3 is not bound in pattern #1
  --> file.rs:10:43
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                           ^ pattern doesn't bind `c`

error[E0408]: variable `d` from pattern #1 is not bound in pattern rust-lang#4
  --> file.rs:10:48
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                                ^^^^^^^^ pattern doesn't bind `d`

error: aborting due to 6 previous errors
```

After:

```rust
error[E0408]: variable `d` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                  -          -       ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `d`
   |                  |          |       |
   |                  |          |       pattern doesn't bind `d`
   |                  |          variable not in all patterns
   |                  variable not in all patterns

error[E0408]: variable `c` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:48
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ^^^^^^^^^^^   ^^^^^^^^^^^         -    ^^^^^^^^ pattern doesn't bind `c`
   |         |             |                   |
   |         |             |                   variable not in all patterns
   |         |             pattern doesn't bind `c`
   |         pattern doesn't bind `c`

error[E0408]: variable `a` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |               -       ^^^^^^^^^^^   ^^^^^^^^         - variable not in all patterns
   |               |       |             |
   |               |       |             pattern doesn't bind `a`
   |               |       pattern doesn't bind `a`
   |               variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ^^^^^^^^^^^            -    ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `b`
   |         |                      |    |
   |         |                      |    pattern doesn't bind `b`
   |         |                      variable not in all patterns
   |         pattern doesn't bind `b`

error: aborting due to 4 previous errors
```

Fixes rust-lang#39698.
bors added a commit that referenced this issue Mar 8, 2017
Clean up "pattern doesn't bind x" messages

Group "missing variable bind" spans in `or` matches and clarify wording
for the two possible cases: when a variable from the first pattern is
not in any of the subsequent patterns, and when a variable in any of the
other patterns is not in the first one.

Before:

```rust
error[E0408]: variable `a` from pattern #1 is not bound in pattern #2
  --> file.rs:10:23
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                       ^^^^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
  --> file.rs:10:32
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                ^ pattern doesn't bind `b`

error[E0408]: variable `a` from pattern #1 is not bound in pattern #3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `d` from pattern #1 is not bound in pattern #3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `d`

error[E0408]: variable `c` from pattern #3 is not bound in pattern #1
  --> file.rs:10:43
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                           ^ pattern doesn't bind `c`

error[E0408]: variable `d` from pattern #1 is not bound in pattern #4
  --> file.rs:10:48
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                                ^^^^^^^^ pattern doesn't bind `d`

error: aborting due to 6 previous errors
```

After:

```rust
error[E0408]: variable `d` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                  -          -       ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `d`
   |                  |          |       |
   |                  |          |       pattern doesn't bind `d`
   |                  |          variable not in all patterns
   |                  variable not in all patterns

error[E0408]: variable `c` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:48
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ^^^^^^^^^^^   ^^^^^^^^^^^         -    ^^^^^^^^ pattern doesn't bind `c`
   |         |             |                   |
   |         |             |                   variable not in all patterns
   |         |             pattern doesn't bind `c`
   |         pattern doesn't bind `c`

error[E0408]: variable `a` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |               -       ^^^^^^^^^^^   ^^^^^^^^         - variable not in all patterns
   |               |       |             |
   |               |       |             pattern doesn't bind `a`
   |               |       pattern doesn't bind `a`
   |               variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |         ^^^^^^^^^^^            -    ^^^^^^^^   ^^^^^^^^ pattern doesn't bind `b`
   |         |                      |    |
   |         |                      |    pattern doesn't bind `b`
   |         |                      variable not in all patterns
   |         pattern doesn't bind `b`

error: aborting due to 4 previous errors
```

Fixes #39698.
@osa1
Copy link
Contributor Author

osa1 commented Mar 8, 2017

@estebank this looks amazing, thanks for all the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants