Warn on pattern bindings that have the same name as a variant #19115
Conversation
|
I considered making this a warning instead but ended up going with a hard error. I cannot think of a situation where the code that this change rejects would be intended. On the other hand, making this produce a warning would mean that the compiler would have to proceed to check exhaustiveness which would almost always fail anyway. So at the end as a user you would see the lint warning and an exhaustiveness error, and most likely only pay attention to the latter, which is guaranteed to be rather obscure. |
|
As a language change, I'm not sure I saw a corresponding RFC for this, although I may have missed it as part of a recent one! |
|
FWIW, a lint doesn't work with our current system, since the lint pass occurs too late (errors about unreachable patterns etc. occur long before linting runs). I'm would definitely be in favour of this being changed to a enum Foo { A, B }
fn main() {
match A {
A => {}
Foo::B => {}
}
} |
ghost
changed the title
ghost
changed the title
|
@alexcrichton @huonw Right. This improvement is particularly important now with the enum namespacing change recently landed, although I suppose we're already late. :-) So in the interest of moving this forward I changed the new diagnostic to be a warning instead. I think making it a hard error would still be good and I will consider writing an RFC for it. |
|
I general I would expect |
|
@alexcrichton IMO this sits somewhere in between an error and a warning in that it's not fatal enough that compilation can still proceed (or otherwise halt at exhaustiveness) but in 99% cases it points out a genuine error so the user should know about it. |
|
Yeah I definitely agree that this is basically always an error, I just feel uneasy about baking this into the language as such (hence the warning), but then it's unfortunate to have warnings you can't turn off... I suppose I'm just a little on the fence about this in that I want to see this have a nice error message but I also don't want to bake anything too much into the language about this either... |
|
@alexcrichton I see! Thanks for the feedback. I made it so that warnings will not be printed out if the command-line level lint level for the |
|
Whoa, the solution here had not actually occurred to me, but it seems great, nice idea! (sorry for being so nitpickity) |
...of the type being matched.
This change will result in a better diagnostic for code like the following:
```rust
enum Enum {
Foo,
Bar
}
fn f(x: Enum) {
match x {
Foo => (),
Bar => ()
}
}
```
which would currently simply fail with an unreachable pattern error
on the 2nd arm.
The user is advised to either use a qualified path in the patterns
or import the variants explicitly into the scope.
...of the type being matched.
This change will result in a better diagnostic for code like the following:
```rust
enum Enum {
Foo,
Bar
}
fn f(x: Enum) {
match x {
Foo => (),
Bar => ()
}
}
```
which would currently simply fail with an unreachable pattern error
on the 2nd arm.
The user is advised to either use a qualified path in the patterns
or import the variants explicitly into the scope.
|
On Thu, Nov 20, 2014 at 08:24:25AM -0800, Alex Crichton wrote:
This matter of timing is related to the idea of using a lint for shadowing. |
ghost
deleted the
...of the type being matched.
This change will result in a better diagnostic for code like the following:
which would currently simply fail with an unreachable pattern error
on the 2nd arm.
The user is advised to either use a qualified path in the patterns
or import the variants explicitly into the scope.