Stabilise exhaustive patterns feature #110105
Conversation
|
r? @lcnr (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
The Miri subtree was changed cc @rust-lang/miri Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
Noratrieb
added
the
T-lang
This comment has been minimized.
This comment has been minimized.
cjgillot
force-pushed
the
exhaustive-patterns
branch
from
507a45e
to
fef31b6
Compare
Not sure if it still relevant but as far as I am aware the concern raised in http://smallcultfollowing.com/babysteps/blog/2018/08/13/never-patterns-exhaustive-matching-and-uninhabited-types-oh-my/ hasn't been addressed. |
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
The rules around UB have matured since that blog post has been written, so I'm not sure where the concern lies any more. @nikomatsakis does this concern still stand?
The logic here is purely type-based, so there should not be any interaction. What do you have in mind? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
|
Could you leave a comment on the tracking issue responding to the blocking issues mentioned in #51085 (comment)? cc @Nadrieril |
Nadrieril
added
the
A-exhaustiveness-checking
As far as I know, the questions raised in that post (referenced by this comment) are still relevant and have not been resolved. @rust-lang/opsem is probably better positioned to judge whether this stabilization makes those questions any easier or harder. I can try to bring that stuff back into cache if they don't weigh in. =) |
|
Right, specifically the interesting questions involve pointer indirections: given Similar with |
|
It references are not considered uninhabited, so the 'x: &!' case is not allowed. As for the "what does the looking", MIR building introduces a fake read before the unreachable terminator for precisely this reason. So the '!' arm is the whitespace between the last explicit arm and the closing brace. https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/Soundness.20of.20.60exhaustive_patterns.60 might be an easier place to discuss this. |
Oh, okay. That should be documented clearly and covered by tests to ensure this will not be changed with some drive-by PR in the future. If inhabitedness never recurses below pointer indirections, so at least on first thought I think this does not have the issues that were discussed in the blog post.
When and where exactly such a fake read is introduced is not entirely clear though. E.g. arguably |
Well,
With
How do we make sense of "creating a place for a value of type |
That is definitely the case: The former generates an
Why not? It's a regular ZST, layout-wise. In fact eventually we probably want |
|
For For now, this test exists: https://github.com/rust-lang/rust/blob/master/tests/ui/pattern/usefulness/always-inhabited-union-ref.rs This does not apply to
We currently emit a fakeread of the scrutinee in all cases. #103208 would restrict the fakeread to the branch with an |
Then we need to fix the exhaustiveness checker to accept both somehow. Presumably
That's unsettling. I would expect that for consistency we need |
|
All |
|
Furthermore, is is a frequent request to make And having |
|
In addition to the example I just shared in #103208 , @RalfJung's claim that
also seems wrong in at least one other case: union U {
a: (),
b: !,
}
let x: U = U { a: () };
match x.b {
_ if {
println!("Executed match!");
loop {}
} => (),
}This also compiles today and is presumably not UB. I don't understand this PR well enough yet to know if this is a problem. |
|
Ah right, I forgot about union fields. |
|
☔ The latest upstream changes (presumably #111452) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
the
S-waiting-on-author
cjgillot
removed
the
S-waiting-on-fcp
cjgillot
force-pushed
the
exhaustive-patterns
branch
from
bfbf070
to
6004bc6
Compare
@JakobDegen This PR does not change the behaviour of that code, but introduces a spurious "unreachable pattern" warning on the |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #113120) made this pull request unmergeable. Please resolve the merge conflicts. |
cjgillot
force-pushed
the
exhaustive-patterns
branch
from
7f9ef90
to
25c5b94
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #113303) made this pull request unmergeable. Please resolve the merge conflicts. |
It looks like even on stable today, there is already an "unreachable pattern" warning in that case, per the playground link? |
Wait, what do you mean by that? |
|
This playground shows a warning even on stable: |
|
This is indeed a bug, which is tracked in #117119. I am about to submit a PR to fix it |
|
Going through all my github assignments rn. Since this PR has been opened |
Stabilisation report
This feature allows to omit visibly uninhabited patterns from a match.
This omission is only possible when the types are visibly uninhabited at the use point, ie. only if the uninhabitedness comes from a field that is accessible. As a drive-by this PR adds a note explaining this design choice.
Fixes #104034
The relevant tests are in
tests/ui/pattern.I am not aware of any unresolved questions.