fix: const eval cast of single-variant enum - #23185
Conversation
rustbot
added
the
S-waiting-on-review
There was a problem hiding this comment.
This isn't the correct way to fix that. It should be fixed during MIR lowering, see the corresponding code in rustc: https://github.com/rust-lang/rust/blob/e71c0f1e3395b10a8c331317be1a5c107bdf7b2e/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs#L125-L163.
6e61250 to
5627c1c
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
d50b7b5 to
b36d82e
Compare
b36d82e to
b32e18a
Compare
| let ty = self.place_ty(p, locals)?; | ||
| let bytes = self.eval_place(p, locals)?.get(self)?; | ||
| let result = self.compute_discriminant(ty, bytes)?; | ||
| let result = if let Some(f) = locals.body.owner.as_variant() |
There was a problem hiding this comment.
I said you do not need to change eval. Why are you doing it?
There was a problem hiding this comment.
Sorry, I've tried to understand how it works and how it should be solved. I made the eval changes because the updated lower changes introduce Rvalue::Discriminant in the cast path. Then one of the enums test was failing with RecursiveTypeWithoutIndirection when evaluating discriminant layout for enums with self-referencing expressions.
So, the eval change was basically workaround for that. I'll try to look into a better approach that keeps the fix contained in specifically in lower.
There was a problem hiding this comment.
The eval change is not correct. If you point me at the failing test and provide the full error, I could maybe assist with it.
There was a problem hiding this comment.
Without the eval changes tests fail at https://github.com/rust-lang/rust-analyzer/blob/master/crates/hir-ty/src/consteval/tests.rs#L2494.
failures:
---- consteval::tests::enums stdout ----
thread 'consteval::tests::enums' (29899) panicked at crates/hir-ty/src/consteval/tests.rs:97:17:
Error in evaluating goal: Mir eval error:
Layout for type `E` is not available due RecursiveTypeWithoutIndirection
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
consteval::tests::enums
There was a problem hiding this comment.
Okay I discovered the cause: when computing the discriminant we need to compute the enum's layout which compute all discriminants, creating a cycle. The question is why it doesn't happen in rustc, I'll keep investigating.
There was a problem hiding this comment.
I see: rustc has specific logic for that in THIR lowering (which you should replicate in MIR lowering). See:
3 participants
Single-variant enum incorrectly evaluated to 0.
Previously enum-to-int casts were lowered as a plain
Rvalue::Cast, causing single-variant enums to evaluate to 0 instead of their corresponding discriminant value.Fixes enum discriminant handling in MIR lowering by extracting the discriminant before performing the integer cast. Prevents recursion in MIR evaluation by reading discriminant bytes directly when the owner is a variant of the same enum in
Rvalue::Discriminantbranch.Fixes #23148