never_patterns: lower a never arm to an unreachable terminator #120758
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred in cc @BoxyUwU |
This comment has been minimized.
This comment has been minimized.
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
|
||
| enum Void {} | ||
|
|
||
| // EMIT_MIR never_patterns.opt1.UninhabitedEnumBranching.before.mir |
There was a problem hiding this comment.
Could this be a MIR building test, outputting the just-built mir ?
There was a problem hiding this comment.
Yes! How may I do that?
There was a problem hiding this comment.
Is it just EMIT_MIR never_patterns.opt1.built.after.mir?
This comment has been minimized.
This comment has been minimized.
Nadrieril
force-pushed
the
lower-never-pats2
branch
from
96a4bf7 to
1915868
Compare
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
Nadrieril
force-pushed
the
lower-never-pats2
branch
from
1915868 to
3717857
Compare
|
I'm a bit disappointed at the |
| @@ -287,6 +287,7 @@ pub enum ExprPrecedence { | |||
| Struct, | |||
| Gen, | |||
| Await, | |||
| Unreachable, | |||
There was a problem hiding this comment.
Can you just give ExprKind::Unreachable a dummy precedence? Like, just give it the same precedence as Lit or something.
| @@ -570,19 +570,9 @@ impl<'hir> LoweringContext<'_, 'hir> { | |||
| self.dcx().emit_err(NeverPatternWithGuard { span: g.span }); | |||
| } | |||
|
|
|||
| // We add a fake `loop {}` arm body so that it typecks to `!`. | |||
| // FIXME(never_patterns): Desugar into a call to `unreachable_unchecked`. | |||
There was a problem hiding this comment.
Your PR description doesn't explain the challenges of just lowering a call to, for example, the unreachable intrinsic. I feel like that's far less invasive than adding a new HIR expr kind.
There was a problem hiding this comment.
Ah yes, we discussed this on Zulip. Intrinsics don't exist at the hir level, they have to be constructed during mir lowering/codegen, so that's essentially what I'm doing.
Alternatives would be to make unreachable_unchecked a lang item, or keep the loop and rely on the UninhabitedEnumBranching mir opt to elide it. I went with Unreachable because that's what everyone seemed to prefer; I don't have a strong opinion.
|
Wait I just realized something: when lowering |
This introduces
ExprKind::Unreachable(in hir and thir), which is lowered to an unreachable terminator in mir. For the non-trivial cases (mir lowering and type checking), I modeled it after the case of a match expression without arms, and the result looks sensible.I then use this to lower never arms. This doesn't change the final optimized mir because the
UninhabitedEnumBranchingpass would detect the empty variant anyway, but this way is cleaner.r? @compiler-errors