-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
add note for non-exhaustive matches with guards #113019
add note for non-exhaustive matches with guards #113019
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @fee1-dead (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Some changes might have occurred in exhaustiveness checking cc @Nadrieril |
|
||
let all_arms_have_guards = arms.iter().all(|arm_id| thir[*arm_id].guard.is_some()); | ||
if !is_empty_match && all_arms_have_guards { | ||
err.note("match arms with guards don't count towards exhaustivity"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please use translatable diagnostics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better if the message pointed at the specific match arm that had guards and prevented the match from being exhaustive, but this is a step.
@@ -25,10 +25,14 @@ fn main() { | |||
let ed = ElementData { kind: Box::new(ElementKind::HTMLImageElement(id)) }; | |||
let n = NodeData { kind: Box::new(NodeKind::Element(ed)) }; | |||
|
|||
// n.b. span could be better |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you remove this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that was in reference to the lack of the comments annotating the expected diagnostic in the compiletest. Is that not right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was about the span that the diagnostic highlighted?
How would you determine which match arm is preventing the match from being exhaustive? For instance in fn foo(i: i32) {
match i {
i if i == 0 => {},
i if i != 0 => {},
}
} removing either guard will cause the match to be exhaustive, and I feel like pointing to one of them in particular might be a bit confusing. I also have some thoughts about how the error message could be improved---I mostly put this up so I could get some feedback there, though maybe Zulip is the better place to do that. For instance, the current wording of the note makes it sound like something that would show up on any non-exhaustive match with an unguarded arm. I think this narrow concern could be addressed by explicitly mentioning that all arms have guards, like ocamlc does: let foo (i : int) =
match i with
| i when i == 0 -> ()
| i when i <> 0 -> ()
|
Obviously if we have the same pattern then they all don't contribute to exhaustiveness. I was thinking about how for specific cases like such: let x = Some(1);
match x {
Some(x) if x == 5 => {}
None => {}
} we can then point at match x {
Foo(x) if x == 5 => {}
Foo(x) => {}
Bar(x) if y == 42 => {}
None => {}
} For the above we need to point at the |
3f029ab
to
2017a17
Compare
Thanks. @bors r+ rollup |
Rollup of 6 pull requests Successful merges: - rust-lang#111571 (Implement proposed API for `proc_macro_span`) - rust-lang#112236 (Simplify computation of killed borrows) - rust-lang#112867 (More `ImplSource` nits) - rust-lang#113019 (add note for non-exhaustive matches with guards) - rust-lang#113094 (Fix invalid HTML DIV tag used in HEAD) - rust-lang#113111 (add myself to review for t-types stuff) r? `@ghost` `@rustbot` modify labels: rollup
Associated issue: #92197
When a match statement includes guards on every match arm (and is therefore necessarily non-exhaustive), add a note to the error E0004 diagnostic noting this.