-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Improve diagnostics and code for exhaustiveness of empty matches #67026
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
Conversation
This is logically equivalent to the previous code.
Actually empty matches are still handled by a different code path
This uses the exact same code path that would be used for `match x { _ if false => {} }`, since in both cases the resulting matrix is empty. Since we think the behaviour in that case is ok, then we can remove the special case and use the default code path.
When the feature is on, the special casing is not needed. That way when we stabilize the feature this `if` can just be removed.
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
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.
This change looks very good overall, thanks!
@@ -9,6 +9,9 @@ LL | let _ = match x { | |||
error[E0004]: non-exhaustive patterns: type `&Void` is non-empty | |||
--> $DIR/uninhabited-matches-feature-gated.rs:12:19 | |||
| | |||
LL | enum Void {} | |||
| ------------ `Void` defined here | |||
... | |||
LL | let _ = match x {}; |
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 we might want a special note for !
types.
Letting @varkor give final stamp, but +1 from me. |
@bors r=varkor,Centril,estebank |
📌 Commit fbd2cd0 has been approved by |
⌛ Testing commit fbd2cd0 with merge a8d1fbfe9401db1c3d3debf889f0c915b92ad61c... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
@bors retry seems spurious cc @alexcrichton @sfackler could you take a look at what is happening with the spurious issue? |
… r=varkor,Centril,estebank Improve diagnostics and code for exhaustiveness of empty matches There was a completely separate check and diagnostics for the case of an empty match. This led to slightly different error messages and duplicated code. This improves code reuse and generally clarifies what happens for empty matches. This also clarifies the action of the `exhaustive_patterns` feature, and ensures that this feature doesn't change diagnostics in places it doesn't need to.
Rollup of 7 pull requests Successful merges: - #67026 (Improve diagnostics and code for exhaustiveness of empty matches) - #67235 (VecDeque: drop remaining items on destructor panic) - #67254 (dont ICE in case of invalid drop fn) - #67256 (Reduce allocs for validation errors) - #67274 (be explicit that mem::uninitialized is the same as MaybeUninit::uninit().assume_init()) - #67278 (`coerce_inner`: use initial `expected_ty`) - #67280 (docs: std::convert::From: Fix typo) Failed merges: r? @ghost
There was a completely separate check and diagnostics for the case of an empty match. This led to slightly different error messages and duplicated code.
This improves code reuse and generally clarifies what happens for empty matches. This also clarifies the action of the
exhaustive_patterns
feature, and ensures that this feature doesn't change diagnostics in places it doesn't need to.