Skip to content

Using a tuple-like data type with a record-like pattern (or vice versa) produces an opaque diagnostic #41314

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

Closed
whitequark opened this issue Apr 15, 2017 · 11 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@whitequark
Copy link
Member

whitequark commented Apr 15, 2017

One of the ways to reproduce:

enum X {
    Y(u32)
}

fn main() {
    match X::Y(0) {
        X::Y { data } => ()
    }
}

This results in:

error[E0026]: variant `X::Y` does not have a field named `data`
 --> <anon>:7:16
  |
7 |         X::Y { data } => ()
  |                ^^^^ variant `X::Y` does not have field `data`

error[E0027]: pattern does not mention field `0`
 --> <anon>:7:9
  |
7 |         X::Y { data } => ()
  |         ^^^^^^^^^^^^^ missing field `0`

I think the error message would have been much better if it just said outright that variant X::Y requires a tuple pattern or something like that (I'm not sure what the proper terminology is...)

This happens fairly often during refactoring.

@whitequark
Copy link
Member Author

Here's a much better error message, from a similar case:

error[E0532]: expected unit struct/variant or constant, found tuple variant `kern::DmaRecordStart`
   --> /home/whitequark/Work/artiq-dev/artiq/artiq/firmware/runtime/session.rs:397:14
    |
397 |             &kern::DmaRecordStart => {
    |              ^^^^^^^^^^^^^^^^^^^^ not a unit struct/variant or constant

@petrochenkov
Copy link
Contributor

@whitequark

I think the error message would have been much better if it just said outright that variant X::Y requires a tuple pattern or something like that

The problem is that X::Y doesn't require a tuple pattern, fields of tuple-like things can be accessed by their number.

match X::Y(0) {
    X::Y { 0: data } => ()
}

is legal, as well as

match X::Y(0) {
    X::Y { .. } => ()
}

@whitequark
Copy link
Member Author

The problem is that X::Y doesn't require a tuple pattern, fields of tuple-like things can be accessed by their number.

Then a note could be added.

@petrochenkov
Copy link
Contributor

@whitequark

Then a note could be added.

Could you give a concrete example? (How the note looks and when it is triggered.)

@whitequark
Copy link
Member Author

whitequark commented Apr 16, 2017

Something like this...

error[E0026]: variant `X::Y` does not have a field named `data`
 --> <anon>:7:16
  |
7 |         X::Y { data } => ()
  |                ^^^^ variant `X::Y` does not have field `data`

error[E0027]: pattern does not mention field `0`
 --> <anon>:7:9
  |
7 |         X::Y { data } => ()
  |         ^^^^^^^^^^^^^ missing field `0`
note[E????]: trying to match a tuple variant with a struct variant pattern

triggered when the syntactic form of the variant does not match the syntactic form of the pattern

@Mark-Simulacrum Mark-Simulacrum added the A-diagnostics Area: Messages for errors, warnings, and lints label Jun 20, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@estebank
Copy link
Contributor

check_struct_pat_fields should check the variant::ctor_kind and provide different output depending on wether it is CtorKind::Fn (Enum::A(1)), CtorKind::Const (Enum::B) or CtorKind::Fictive (Enum::C { a: 1 }). Both errors are emitted from that same method, so specializing the output should be straightforward.

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Sep 21, 2017
@thombles
Copy link
Contributor

Hi, I'm going to try to fix this now. First time working on the rust code but it makes sense so far!

@estebank
Copy link
Contributor

@thombles, feel free to reach out with any questions you might have!

@thombles
Copy link
Contributor

thombles commented Sep 23, 2017

I've made the change and it looks like this. thombles@8a66362

error[E0026]: variant `X::Y` does not have a field named `data`
  --> src/main.rs:18:16
   |
18 |         X::Y { data } => println!("The data is {}", data)
   |                ^^^^ variant `X::Y` does not have field `data`

error[E0027]: pattern does not mention field `0`
  --> src/main.rs:18:9
   |
18 |         X::Y { data } => println!("The data is {}", data)
   |         ^^^^^^^^^^^^^ missing field `0`
   |
   = note: trying to match a tuple variant with a struct variant pattern

I just have a couple of questions before I put up a PR...

  1. Does this cover all the cases that we're worried about?

I see that we already have E0532 for the opposite direction:

error[E0532]: expected tuple struct/variant, found struct variant `A::B`
  --> src/main.rs:23:9
   |
23 |         A::B(n) => println!("The data is {}", n)
   |         ^^^^ did you mean `A::B { /* fields */ }`?
  1. Whitequark's mockup had a [E????] suffix on the note. I think this note is associated with the existing error and has no number of its own - is that right?

  2. Is this something that should be unit tested? I searched for diagnostic unit tests but couldn't find much.

  3. I am getting huge amounts of compilation running ./x.py build src/rustc --stage 1. I've tried playing with optimize true, setting codegen-units to 0 and also --keep-stage 0. The best build time I got after a tiny change in _match.rs was 13 minutes on a ~6yo i7 CPU. Is that normal?

Thanks!

@petrochenkov
Copy link
Contributor

@thombles

Does this cover all the cases that we're worried about?

It should cover this issue.

I think this note is associated with the existing error and has no number of its own - is that right

Correct.

The best build time I got after a tiny change in _match.rs was 13 minutes on a ~6yo i7 CPU. Is that normal?

Yes :(

Is this something that should be unit tested? I searched for diagnostic unit tests but couldn't find much.

I recommend to find a test reporting this error in compile-fail tests and move it into ui test directory (user-interface tests).
The sequence for adding an UI test is

  • add an my_test.rs file
  • run ./x.py test --stage 1 src/test/ui
  • run ./src/test/ui/update-all-references.sh 'build/x86_64-pc-windows-gnu/test/ui/' (using your platform of course, if you are not on windows-gnu), this will generate my_test.stderr files with expected output.

I am getting huge amounts of compilation running ./x.py build src/rustc --stage 1.

./x.py build src/rustc --stage 1 builds more than necessary, ./x.py test --stage 1 src/test/<test-directory> should do the job a bit more quickly.

@thombles
Copy link
Contributor

Thanks @petrochenkov! The existing compile-fail test covered matching a struct so I made a new UI test to exercise this scenario. I've made a PR now.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 23, 2017
Improve diagnostics when attempting to match tuple enum variant with struct pattern

Adds an extra note as below to explain that a tuple pattern was probably intended.

```
error[E0026]: variant `X::Y` does not have a field named `data`
  --> src/main.rs:18:16
   |
18 |         X::Y { data } => println!("The data is {}", data)
   |                ^^^^ variant `X::Y` does not have field `data`

error[E0027]: pattern does not mention field `0`
  --> src/main.rs:18:9
   |
18 |         X::Y { data } => println!("The data is {}", data)
   |         ^^^^^^^^^^^^^ missing field `0`
   |
   = note: trying to match a tuple variant with a struct variant pattern
```

Solution for rust-lang#41314.
bors added a commit that referenced this issue Sep 24, 2017
Improve diagnostics when attempting to match tuple enum variant with struct pattern

Adds an extra note as below to explain that a tuple pattern was probably intended.

```
error[E0026]: variant `X::Y` does not have a field named `data`
  --> src/main.rs:18:16
   |
18 |         X::Y { data } => println!("The data is {}", data)
   |                ^^^^ variant `X::Y` does not have field `data`

error[E0027]: pattern does not mention field `0`
  --> src/main.rs:18:9
   |
18 |         X::Y { data } => println!("The data is {}", data)
   |         ^^^^^^^^^^^^^ missing field `0`
   |
   = note: trying to match a tuple variant with a struct variant pattern
```

Fixes #41314.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

No branches or pull requests

5 participants