mark min_exhaustive_patterns as complete
#120742
Conversation
rustbot
added
S-waiting-on-review
| @@ -271,7 +271,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | |||
| PatKind::Variant { adt_def, args, variant_index, ref subpatterns } => { | |||
| let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| { | |||
| i == variant_index || { | |||
| self.tcx.features().exhaustive_patterns | |||
| (self.tcx.features().exhaustive_patterns | |||
There was a problem hiding this comment.
what does this do specifically?
There was a problem hiding this comment.
The following: if the pattern is SomeEnum::Variant3 and all the other variants are uninhabited, then we skip the discriminant test. I don't fully understand why we only do this when the feature is on, my guess is someone was overly-conservative.
There was a problem hiding this comment.
This isn't just for efficiency, this affects closure captures because let Ok((x, _)) = ...; in a closure knows that it only captures the lhs of the tuple. Which I guess is pretty wild.
There was a problem hiding this comment.
|
If you pull 51ada89, b9a1b41, e1df69a into a separate PR, I can approve that immediately. There's nobody you need to ask permission to use a feature in the compiler except for someone from the compiler, so that doesn't need T-lang approval or anything. I would recommend then 0e76047 landing separately with approval from T-libs after that, along with pointing them to the relevant tracking issues or anything that would increase their confidence that this is a sound subset of the exhaustive patterns feature for use in libcore. As for 464d5e7, I wonder if all of the modified tests should instead be made into ones that have two revisions between |
|
Thank you! I'm thinking I do need approval for marking it as complete because it's a requirement of the T-lang "experimental feature" process I've been using: "This flag has to stay until an RFC is accepted, even if the implementation is in good shape". |
|
oh huh, didn't know that |
|
I've essentially been side-stepping T-lang to make progress on this (mostly for the never patterns stuff tbh), so I gotta check back with them at some point |
|
I've split off what doesn't require T-lang over there: #120775 |
Nadrieril
force-pushed
the
use-min_exh_pats
branch
from
464d5e7
to
700821a
Compare
Nadrieril
removed
the
S-waiting-on-review
…piler-errors Make `min_exhaustive_patterns` match `exhaustive_patterns` better Split off from rust-lang#120742. There remained two edge cases where `min_exhaustive_patterns` wasn't behaving like `exhaustive_patterns`. This fixes them, and tests the feature in a bunch more cases. I essentially went through all uses of `exhaustive_patterns` to see which ones would be interesting to compare between the two features. r? `@compiler-errors`
Rollup merge of rust-lang#120775 - Nadrieril:more-min_exh_pats, r=compiler-errors Make `min_exhaustive_patterns` match `exhaustive_patterns` better Split off from rust-lang#120742. There remained two edge cases where `min_exhaustive_patterns` wasn't behaving like `exhaustive_patterns`. This fixes them, and tests the feature in a bunch more cases. I essentially went through all uses of `exhaustive_patterns` to see which ones would be interesting to compare between the two features. r? `@compiler-errors`
Nadrieril
force-pushed
the
use-min_exh_pats
branch
from
700821a
to
9590e86
Compare
Nadrieril
changed the title
min_exhaustive_patterns where possiblemin_exhaustive_patterns as complete
Nadrieril
force-pushed
the
use-min_exh_pats
branch
from
9590e86
to
8e83d0c
Compare
|
The FCP is in final comment period. So very likely that in 10 days this can be allowed to be merged. I've split off the std/core changes to a later PR. What remains is pretty straightforward. In the meantime reopening this for review so it's ready when the FCP completes. @rustbot ready |
rustbot
added
the
S-waiting-on-review
Nadrieril
removed
the
T-libs
compiler-errors
removed
the
S-waiting-on-review
compiler-errors
added
the
S-waiting-on-author
bors
added
S-waiting-on-bors
…iler-errors mark `min_exhaustive_patterns` as complete This is step 1 and 2 of my [proposal](rust-lang#119612 (comment)) to move `min_exhaustive_patterns` forward. The vast majority of in-tree use cases of `exhaustive_patterns` are covered by `min_exhaustive_patterns`. There are a few cases that still require `exhaustive_patterns` in tests and they're all behind references. r? `@ghost`
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#120742 (mark `min_exhaustive_patterns` as complete) - rust-lang#121470 (Don't ICE on anonymous struct in enum variant) - rust-lang#121492 (coverage: Rename `is_closure` to `is_hole`) - rust-lang#121495 (remove repetitive words) - rust-lang#121498 (Make QNX/NTO specific "timespec capping" public to crate::sys) - rust-lang#121510 (lint-overflowing-ops: unify cases and remove redundancy) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#120742 - Nadrieril:use-min_exh_pats, r=compiler-errors mark `min_exhaustive_patterns` as complete This is step 1 and 2 of my [proposal](rust-lang#119612 (comment)) to move `min_exhaustive_patterns` forward. The vast majority of in-tree use cases of `exhaustive_patterns` are covered by `min_exhaustive_patterns`. There are a few cases that still require `exhaustive_patterns` in tests and they're all behind references. r? ``@ghost``
This is step 1 and 2 of my proposal to move
min_exhaustive_patternsforward. The vast majority of in-tree use cases ofexhaustive_patternsare covered bymin_exhaustive_patterns. There are a few cases that still requireexhaustive_patternsin tests and they're all behind references.r? @ghost