safe transmute: support `Single` enums #126358

jswrenn · 2024-06-12T22:14:44Z

Previously, the implementation of Tree::from_enum incorrectly treated enums with Variants::Single and Variants::Multiple identically. This is incorrect for Variants::Single enums, which delegate their layout to that of a variant with a particular index (or no variant at all if the enum is empty).

This flaw manifested first as an ICE. Tree::from_enum attempted to compute the tag of variants other than the one at Variants::Single's index, and fell afoul of a sanity-checking assertion in compiler/rustc_const_eval/src/interpret/discriminant.rs. This assertion is non-load-bearing, and can be removed; the routine its in is well-behaved even without it.

With the assertion removed, the proximate issue becomes apparent: calling Tree::from_variant on a variant that does not exist is ill-defined. A sanity check the given variant has FieldShapes::Arbitrary fails, and the analysis is (correctly) aborted with Err::NotYetSupported.

This commit corrects this chain of failures by ensuring that Tree::from_variant is not called on variants that are, as far as layout is concerned, nonexistent. Specifically, the implementation of Tree::from_enum is now partitioned into three cases:

enums that are uninhabited
enums for which all but one variant is uninhabited
enums with multiple inhabited variants

Tree::from_variant is now only invoked in the third case. In the first case, Tree::uninhabited() is produced. In the second case, the layout is delegated to Variants::Single's index.

Fixes #125811

rustbot · 2024-06-12T22:14:53Z

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-06-12T22:14:54Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

jswrenn · 2024-06-12T22:20:06Z

compiler/rustc_const_eval/src/interpret/discriminant.rs

@@ -241,10 +241,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
        variant_index: VariantIdx,
    ) -> InterpResult<'tcx, Option<(ScalarInt, usize)>> {
        match self.layout_of(ty)?.variants {
-            abi::Variants::Single { index } => {
-                assert_eq!(index, variant_index);


Removing this assertion was recommended by @RalfJung in #125811 (comment), and I agree with his assessment that it isn't a load-bearing assertion. An index mismatch here doesn't suggest a bug in computing the tag.

That said, removing this assertion is irrelevant to fixing the bug — the salient changes are all in compiler/rustc_transmute/src/layout/tree.rs. Those changes fix the ICE even with this assertion intact.

I would suggest to replace the assertion by a comment, like

// The tag of a `Single` enum is like the tag of the niched variant: there's no tag // as the discriminant is encoded entirely implicitly. // If `write_discriminant` ever hits this case, we do a "validation read" to ensure // the the right discriminant is encoded implicitly, so any attempt to write the // wrong discriminant for a `Single` enum will reliably result in UB.

compiler-errors · 2024-06-12T23:30:30Z

@rustbot author

can take a look once this is fixed

compiler/rustc_transmute/src/layout/tree.rs

compiler-errors · 2024-06-12T23:36:03Z

Also, can you explain somewhere what this PR actually does? This PR has like no description -- typically a fix should justify what it's fixing unless the code speaks for itself, and in this case it doesn't really.

Previously, the implementation of `Tree::from_enum` incorrectly treated enums with `Variants::Single` and `Variants::Multiple` identically. This is incorrect for `Variants::Single` enums, which delegate their layout to that of a variant with a particular index (or no variant at all if the enum is empty). This flaw manifested first as an ICE. `Tree::from_enum` attempted to compute the tag of variants other than the one at `Variants::Single`'s `index`, and fell afoul of a sanity-checking assertion in `compiler/rustc_const_eval/src/interpret/discriminant.rs`. This assertion is non-load-bearing, and can be removed; the routine its in is well-behaved even without it. With the assertion removed, the proximate issue becomes apparent: calling `Tree::from_variant` on a variant that does not exist is ill-defined. A sanity check the given variant has `FieldShapes::Arbitrary` fails, and the analysis is (correctly) aborted with `Err::NotYetSupported`. This commit corrects this chain of failures by ensuring that `Tree::from_variant` is not called on variants that are, as far as layout is concerned, nonexistent. Specifically, the implementation of `Tree::from_enum` is now partitioned into three cases: 1. enums that are uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants `Tree::from_variant` is now only invoked in the third case. In the first case, `Tree::uninhabited()` is produced. In the second case, the layout is delegated to `Variants::Single`'s index. Fixes rust-lang#125811

compiler-errors

Thanks!

compiler-errors · 2024-06-13T02:00:42Z

@bors r+

bors · 2024-06-13T02:00:44Z

📌 Commit fb662f2 has been approved by compiler-errors

It is now in the queue for this repository.

…kingjubilee Rollup of 10 pull requests Successful merges: - rust-lang#125674 (Rewrite `symlinked-extern`, `symlinked-rlib` and `symlinked-libraries` `run-make` tests in `rmake.rs` format) - rust-lang#125688 (Walk into alias-eq nested goals even if normalization fails) - rust-lang#126142 (Harmonize using root or leaf obligation in trait error reporting) - rust-lang#126303 (Urls to docs in rust_hir) - rust-lang#126328 (Add Option::is_none_or) - rust-lang#126337 (Add test for walking order dependent opaque type behaviour) - rust-lang#126353 (Move `MatchAgainstFreshVars` to old solver) - rust-lang#126356 (docs(rustc): Improve discoverable of Cargo docs) - rust-lang#126358 (safe transmute: support `Single` enums) - rust-lang#126362 (Make `try_from_target_usize` method public) r? `@ghost` `@rustbot` modify labels: rollup

RalfJung · 2024-06-13T05:19:33Z

PR already got rolled up so the comment should probably be a follow-up.

RalfJung · 2024-06-13T05:20:53Z

compiler/rustc_transmute/src/layout/tree.rs

+                    // the (unlikely?) event that an uninhabited enum is
+                    // non-zero-sized, this assert will trigger an ICE, and this
+                    // code should be modified such that a `layout.size` amount
+                    // of uninhabited bytes is returned instead.


Have you tried an enum like enum E { Variant(!, i32) }? That may give rise to a non-zero-sized uninhabited enum.

Rollup merge of rust-lang#126358 - jswrenn:fix-125811, r=compiler-errors safe transmute: support `Single` enums Previously, the implementation of `Tree::from_enum` incorrectly treated enums with `Variants::Single` and `Variants::Multiple` identically. This is incorrect for `Variants::Single` enums, which delegate their layout to that of a variant with a particular index (or no variant at all if the enum is empty). This flaw manifested first as an ICE. `Tree::from_enum` attempted to compute the tag of variants other than the one at `Variants::Single`'s `index`, and fell afoul of a sanity-checking assertion in `compiler/rustc_const_eval/src/interpret/discriminant.rs`. This assertion is non-load-bearing, and can be removed; the routine its in is well-behaved even without it. With the assertion removed, the proximate issue becomes apparent: calling `Tree::from_variant` on a variant that does not exist is ill-defined. A sanity check the given variant has `FieldShapes::Arbitrary` fails, and the analysis is (correctly) aborted with `Err::NotYetSupported`. This commit corrects this chain of failures by ensuring that `Tree::from_variant` is not called on variants that are, as far as layout is concerned, nonexistent. Specifically, the implementation of `Tree::from_enum` is now partitioned into three cases: 1. enums that are uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants `Tree::from_variant` is now only invoked in the third case. In the first case, `Tree::uninhabited()` is produced. In the second case, the layout is delegated to `Variants::Single`'s index. Fixes rust-lang#125811

@RalfJung

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrower category of "enums that are variantless". These enums, whose layouts are described with `Variants::Single { index }`, are special in that they do not have a variant at `index`. The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` but that do have a variant at `index`. This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

@RalfJung

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrower category of "enums that are variantless". These enums, whose layouts are described with `Variants::Single { index }`, are special in that they do not have a variant at `index`. The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` but that do have a variant at `index`. This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

@RalfJung

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

@RalfJung

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

@RalfJung

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

@RalfJung

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } This PR also adds a comment requested by @RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } This PR also adds a comment requested by RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited, non-ZST enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by RalfJung in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460

safe transmute: support non-ZST, variantful, uninhabited enums Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by `@RalfJung` in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460 r? `@compiler-errors`

safe transmute: support non-ZST, variantful, uninhabited enums Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by ``@RalfJung`` in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460 r? ``@compiler-errors``

safe transmute: support non-ZST, variantful, uninhabited enums Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by ```@RalfJung``` in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460 r? ```@compiler-errors```

safe transmute: support non-ZST, variantful, uninhabited enums Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by ````@RalfJung```` in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460 r? ````@compiler-errors````

Rollup merge of rust-lang#126493 - jswrenn:fix-126460, r=compiler-errors safe transmute: support non-ZST, variantful, uninhabited enums Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by ````@RalfJung```` in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes rust-lang#126460 r? ````@compiler-errors````

rustbot assigned TaKO8Ki Jun 12, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 12, 2024

jswrenn commented Jun 12, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2024

compiler-errors assigned compiler-errors and unassigned TaKO8Ki Jun 12, 2024

compiler-errors reviewed Jun 12, 2024

View reviewed changes

compiler/rustc_transmute/src/layout/tree.rs Show resolved Hide resolved

jswrenn force-pushed the fix-125811 branch from 9978521 to fb662f2 Compare June 13, 2024 01:40

compiler-errors approved these changes Jun 13, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 13, 2024

workingjubilee mentioned this pull request Jun 13, 2024

Rollup of 10 pull requests #126374

Merged

RalfJung reviewed Jun 13, 2024

View reviewed changes

bors merged commit d33ec8e into rust-lang:master Jun 13, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jun 13, 2024

matthiaskrgr mentioned this pull request Jun 14, 2024

ICE: layout: 🌲 : Size != Size #126460

Closed

jswrenn mentioned this pull request Jun 14, 2024

safe transmute: support non-ZST, variantful, uninhabited enums #126493

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

safe transmute: support `Single` enums #126358

safe transmute: support `Single` enums #126358

jswrenn commented Jun 12, 2024 •

edited

Loading

rustbot commented Jun 12, 2024

rustbot commented Jun 12, 2024

jswrenn Jun 12, 2024 •

edited

Loading

RalfJung Jun 13, 2024

This comment has been minimized.

compiler-errors commented Jun 12, 2024

compiler-errors commented Jun 12, 2024 •

edited

Loading

compiler-errors left a comment

compiler-errors commented Jun 13, 2024

bors commented Jun 13, 2024

RalfJung commented Jun 13, 2024

RalfJung Jun 13, 2024

safe transmute: support Single enums #126358

safe transmute: support Single enums #126358

Conversation

jswrenn commented Jun 12, 2024 • edited Loading

rustbot commented Jun 12, 2024

rustbot commented Jun 12, 2024

jswrenn Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

RalfJung Jun 13, 2024

Choose a reason for hiding this comment

This comment has been minimized.

compiler-errors commented Jun 12, 2024

compiler-errors commented Jun 12, 2024 • edited Loading

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors commented Jun 13, 2024

bors commented Jun 13, 2024

RalfJung commented Jun 13, 2024

RalfJung Jun 13, 2024

Choose a reason for hiding this comment

safe transmute: support `Single` enums #126358

safe transmute: support `Single` enums #126358

jswrenn commented Jun 12, 2024 •

edited

Loading

jswrenn Jun 12, 2024 •

edited

Loading

compiler-errors commented Jun 12, 2024 •

edited

Loading