[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337

zachs18 · 2025-08-13T06:25:57Z

PR description is outdated. I'll update it if/when the MCP is accepted.

This PR contains 3 or 4 changes which could probably be split up:

First (first commit): split out into #145387

Second (second commit) (needed for the actual layout changes to be sound with offset_of! on uninhabited variants' fields): Keep track of uninhabited non-abset variants' layouts, even if we use an untagged (Variants::Single) or uninhabited (Variants::Empty) layout. This is necessary as otherwise offset_of! will "conjure" a layout for uninhabited variants where all fields are at offset 0, which conflicts with partial initialization¹.

Third (sixth commit): Do not make space for the enum tag in uninhabited variants in the tagged enum layout calculation. This optimizes enums where the uninhabited variants are the largest, and reduces their size by up to the size of the tag. e.g. enum Foo { A, B(i32, !) } was 8 bytes A: (tag, uninit), B: (tag, i32, !) and is now 4 bytes A: (tag,), B: (i32, !). Additionally, enums with only zero-sized uninhabited variants are special-cased to return an uninhabited layout of the correct size and alignment without a tag at all, as otherwise we get weird edge cases².

Fourth (seventh commit): Add an additional enum layout that does not encode a tag. This is similar to the existing "single-present-variant" optimization, but works even if some of the uninhabited variants are "present" (have non-1-ZST fields). This works by calculating the layout of the inhabited variant as a struct, and padding it to the maximum size and alignment necessary to fit each uninhabited variant.

The layout changes probably need an MCP and I haven't squashed fully, so draft for now.

r? ghost

This isn't an issue before this PR because the layout algorithm only produces Variants::Empty or Variants::Single when all uninhabited variants are "absent", i.e. have only 1-ZST fields ↩
Specifically, without the special-case, enum Foo { A(Aligned2Never), B(Aligned2Never) } got a size-0 layout with an i16 tag field, which I assume would cause problems. ↩

Remove TmpLayout in layout_of_enum 09a3846 from <rust-lang#103693> made LayoutData be owned instead of interned in `Variants::Multiple::variants`[^1], so there's no need for `TmpLayout` in layout_of_enum anymore, and we can just store the variants' layouts directly in the prospective `LayoutData`s' `variants` fields. This should have no effect on semantics or layout. (written as part of rust-lang#145337 but not related to the layout optimizations in that PR) [^1]: see line 1154 of `compiler/rustc_target/src/abi/mod.rs` in the linked commit; `Variants::Multiple::variants` effectively changed from `IndexVec<.., Layout<'tcx>>` to `IndexVec<.., LayoutData>` where the `LayoutData`s are not interned as `Layout`s (`LayoutData` was at the time called `LayoutS`)

Rollup merge of #145387 - zachs18:remove-tmplayout, r=cjgillot Remove TmpLayout in layout_of_enum 09a3846 from <#103693> made LayoutData be owned instead of interned in `Variants::Multiple::variants`[^1], so there's no need for `TmpLayout` in layout_of_enum anymore, and we can just store the variants' layouts directly in the prospective `LayoutData`s' `variants` fields. This should have no effect on semantics or layout. (written as part of #145337 but not related to the layout optimizations in that PR) [^1]: see line 1154 of `compiler/rustc_target/src/abi/mod.rs` in the linked commit; `Variants::Multiple::variants` effectively changed from `IndexVec<.., Layout<'tcx>>` to `IndexVec<.., LayoutData>` where the `LayoutData`s are not interned as `Layout`s (`LayoutData` was at the time called `LayoutS`)

bors · 2025-09-29T12:52:48Z

☔ The latest upstream changes (presumably #147145) made this pull request unmergeable. Please resolve the merge conflicts.

…s enum representations

…rate after the commits which add the relevant layout optimizations.

…ommits which add the relevant layout optimizations.

…in a better layout)

…filling layout.

…even when the niche is in an uninhabited variant.

programmerjake · 2025-10-05T21:41:08Z

pointers can be 16-bit, though I think all those targets are tier 3

moulins · 2025-10-07T13:27:28Z

compiler/rustc_abi/src/layout.rs

+        let elt = element;
+        if count == 0 {
+            return Err(LayoutCalculatorError::ZeroLengthSimdType);
+        } else if count > crate::MAX_SIMD_LANES {
+            return Err(LayoutCalculatorError::OversizedSimdType {
+                max_lanes: crate::MAX_SIMD_LANES,
+            });
+        }
+
+        // Compute the size and alignment of the vector
+        let dl = self.cx.data_layout();
+        let size = elt
+            .size(&self.cx)
+            .checked_mul(count, dl)
+            .ok_or_else(|| LayoutCalculatorError::SizeOverflow)?;
+        let (repr, align) = if repr_packed && !count.is_power_of_two() {
+            // Non-power-of-two vectors have padding up to the next power-of-two.
+            // If we're a packed repr, remove the padding while keeping the alignment as close
+            // to a vector as possible.
+            (BackendRepr::Memory { sized: true }, Align::max_aligned_factor(size))
+        } else {
+            (BackendRepr::SimdVector { element, count }, dl.llvmlike_vector_align(size))
+        };
+        let size = size.align_to(align);


This is duplicated code with simd_type, and the only caller doesn't need a full layout, so you could replace simd_type_for_scalar by:

fn simd_type_bare_repr<F>( &self, element: Scalar, count: u64, repr_packed: bool, ) -> Result<(Size, Align, BackendRepr), LayoutCalculatorError<F>>;

and use it both in simd_type and in the new layouting logic.

This comment has been minimized.

Sign in to view

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from 7095fd5 to 76ded17 Compare August 13, 2025 06:41

This comment has been minimized.

Sign in to view

zachs18 mentioned this pull request Aug 13, 2025

Tracking Issue for enum access in offset_of #120141

Open

3 tasks

This comment has been minimized.

Sign in to view

zachs18 mentioned this pull request Aug 14, 2025

Remove TmpLayout in layout_of_enum #145387

Merged

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from e1f38f7 to 6fbb6b7 Compare August 20, 2025 21:44

rustbot added the T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. label Aug 20, 2025

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch 2 times, most recently from bc74d06 to ac3f50f Compare September 8, 2025 03:20

This comment has been minimized.

Sign in to view

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from ac3f50f to 89752c2 Compare September 15, 2025 16:43

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from 89752c2 to c0c1a81 Compare September 24, 2025 16:07

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from c0c1a81 to f9f573e Compare October 4, 2025 18:17

zachs18 mentioned this pull request Oct 5, 2025

Optimize repr(Rust) enums by omitting tags in more cases involving uninhabited variants. rust-lang/compiler-team#922

Open

3 tasks

zachs18 added 7 commits October 5, 2025 13:52

Prepare offset_of for non-absent uninhabited variants in using tagles…

4911131

…s enum representations

Add/extend tests for offset_of! for enums with uninhabited variants

ec7c320

Add tests for Result<u32, (u32, !)> layout. The comments will be accu…

1d130a5

…rate after the commits which add the relevant layout optimizations.

Add more enum layout tests. The comments will be accurate after the c…

02cafd5

…ommits which add the relevant layout optimizations.

Don't encode enum tag for uninhabited repr(Rust) enum variants

2955d8f

Do not store tag for single-inhabited-variant enums (if this results …

0b658af

…in a better layout)

Do not hold space for niche in uninhabited variants when using niche-…

9452645

…filling layout.

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from f9f573e to 018fe5a Compare October 5, 2025 19:10

This comment has been minimized.

Sign in to view

Fix enum layout to use niche-filling layout in guaranteed-NPO cases, …

fb682a3

…even when the niche is in an uninhabited variant.

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from 018fe5a to fb682a3 Compare October 5, 2025 19:29

This comment has been minimized.

Sign in to view

Revisions for pointer size

e8d40a6

zachs18 force-pushed the enum-uninhabited-or-single-variant-no-tag branch from 17fa840 to e8d40a6 Compare October 6, 2025 00:00

moulins reviewed Oct 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337

[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337

zachs18 commented Aug 13, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Sep 29, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

programmerjake commented Oct 5, 2025

Uh oh!

This comment has been minimized.

moulins Oct 7, 2025

Uh oh!

Uh oh!

[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337

Are you sure you want to change the base?

[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337

Conversation

zachs18 commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Sep 29, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

programmerjake commented Oct 5, 2025

Uh oh!

This comment has been minimized.

moulins Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zachs18 commented Aug 13, 2025 •

edited

Loading