Various clean-ups around `LayoutCalculator` - #163384

ada4a · 2026-09-26T17:58:55Z

Details in individual commits. The commits are roughly in the order of increasing controversiality. The changes are small overall imo, and I don't see a clear splitting point, but if you do want me to split up this PR, let me know.

cc @RalfJung (sorry I keep pinging you despite you not being on a review rotation.. others seem not too keen to look at this code)

This whole section of the code deals with constructing an alternative layout, but large nesting makes the control flow seem more complex than it actually is.

Constructing these error variants is basically free

Having the generic list span multiple lines is quite jarring, and imo easy to confuse with the parameter list at a glance.

rustbot · 2026-09-26T17:59:00Z

r? @nnethercote

rustbot has assigned @nnethercote.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: codegen, compiler
codegen, compiler expanded to 77 candidates
Random selection from 19 candidates

RalfJung · 2026-09-26T19:53:36Z

I'm afraid I don't have capacity for anything that's touching the code, not just the comments.

ada4a · 2026-09-26T21:34:03Z

No worries, thank you for taking a look:)

Improves consistency, and hopefully clarifies the purpose of previously-mysteriosly named methods like `univariant`.

nnethercote

Mostly looks good, just a couple of nits.

View changes since this review

nnethercote · 2026-09-27T11:49:02Z


+    /// Compute the layout for a univariant.
+    ///
+    /// For structs and univariant enums, prefer [`Self::layout_of_struct`].


What is a univariant? Is it different to a univeriant enum? I don't understand how the first and second sentences in this comment relate.

AFAICT it's basically everything in Variants::Single; this function is supposed to be used directly for non-ADTs (tuples, closures, ..), and is used internally by layout_of_struct, which takes care of structs and single-variant (or univariant, which indeed seems to be the same thing) enums.

That's the thing with this crate: there's a lot of jargon which is mentioned in a lot of places, but explained nowhere (afaict) – and I'm not sure what the place to introduce it would be. Based on my explanations above (which may or may not be correct), "univariant" could probably be defined in the docs for Variants::Single.

That helps, but... aren't "structs and univariant enums" a subset of "univariants"? In which case the second sentence is surprising to see immediately after the first. E.g. this function can be used for structs but you should prefer layout_of_struct? Then what is this function good for? Just unions and non-closure non-ADTs?

That helps, but... aren't "structs and univariant enums" a subset of "univariants"?

I think they are, yes.

In which case the second sentence is surprising to see immediately after the first. E.g. this function can be used for structs but you should prefer layout_of_struct?

Well, kind of... this arguably shouldn't be used for structs, because it doesn't account for NicheOptimizations, for example. But it does seem to be used for struct variants of enums -- see

rust/compiler/rustc_abi/src/layout.rs

Line 616 in b373574

let st = self.univariant(v, repr, StructKind::AlwaysSized).ok()?;

Then what is this function good for? Just unions and non-closure non-ADTs?

Pretty much, yes.

I'm struggling to come up with a way to describe this function which is helpful yet concise (i.e. doesn't duplicate a lot of the docs for Variants::Single)...

OTOH, grepping for "univariant" in the codebase, this method seems to be the main source of use of this term, so maybe adding verbose and redundant docs to it would be worth it after all.

nnethercote · 2026-09-27T11:49:30Z

    }

-    /// single-variant enums are just structs, if you think about it
+    /// Calculate the layout for a struct, or a single-variant enum.


Is a single-variant enum different to a univariant enum?

Answered as part of #163384 (comment)

One could argue that it would make sense to stick with one of the terms, for consistency. But "univariant" is imo less intuitive (unless you know a bit of Latin), and so I'd prefer to use "single-variant" for surface-level docs at least... WDYT?

Added a mention of "univariant enum" to Variants::Single

nnethercote · 2026-09-27T11:52:31Z


+/// Whether niche optimizations are should be performed during layout calculation.
+///
+/// UnsafeCell and UnsafePinned both disable niche optimizations


Add . at the end of the sentence.

Done. Added some intra-doc links as well while at it.

The name makes some sense at the callsite, but inside the function, the parameter refers to the `VariantIdx` that should be use for the newly-created layout, so name accordingly. Also move closer to the related `variants` param (Note: could consider passing `variants[present_first]` instead of `variants`, as that's the only way `variants` is used -- at the same type, passing `variants` seems more consistent with the rest of the file.)

This is a more type-safe alternative for the `niche_optimizations: bool` param

nnethercote · 2026-09-27T21:20:17Z

Thanks for the clarifications. I think:

"Single-variant enum" and "univariant enum" are both fine. I find their meaning obvious.
"Univariant" meaning "single-variant enum or struct or union or non-ADT type, excluding coroutines(?)" is extremely confusing and should be changed.
The Variant type name is confusing and should be renamed. Maybe LayoutVariants would be better, and LayoutVariants::Single could be used instead of "univariant". (Or maybe just Layouts or LayoutKinds? Avoiding Variant altogether is probably best.)

But this is beyond the scope of this PR and can be an optional follow-up. Your added documentation has improved the status quo, thank you.

@bors r+ rollup

rust-bors · 2026-09-27T21:20:19Z

📌 Commit 6668485 has been approved by nnethercote

It is now in the queue for this repository.

@RalfJung

…cote Various clean-ups around `LayoutCalculator` Details in individual commits. The commits are roughly in the order of increasing controversiality. The changes are small overall imo, and I don't see a clear splitting point, but if you do want me to split up this PR, let me know. cc @RalfJung (sorry I keep pinging you despite you not being on a review rotation.. others seem not too keen to look at this code)

ada4a · 2026-09-27T21:50:36Z

"Univariant" meaning "single-variant enum or struct or union or non-ADT type, excluding coroutines(?)" is extremely confusing and should be changed.

I mean... "univariant" means "has a single variant", and so this term ends up covering most of TyKinds, since having a single variant is basically the default, outside enums (and coroutines, as you note¹). Ralf did mention that Variants::Single could be instead defined as "all inhabited types that are not Multiple", which would mean we no longer need to enumerate all the TyKinds that end up with Single, which might make things less confusing?

Imo the confusion here is caused by the fact the enums can end up with all of the Variants, and that then overlaps with other TyKinds, which can only end up with one (! – with Empty, and "univariants" – with Single²) – one could probably have made things easier to understand by using different.. variants of Variants for enums vs everything else, but I get why that wasn't done.

The Variant type name is confusing and should be renamed. Maybe LayoutVariants would be better, and LayoutVariants::Single could be used instead of "univariant". (Or maybe just Layouts or LayoutKinds? Avoiding Variant altogether is probably best.)

I agree Variants is not ideal, but it does describe what it does – it represents how many variants a type has, and stores different information depending on how many variants there are. It's not really the variants of the layout (which would justify LayoutVariants), but rather of the type itself – so maybe TypeVariants would be a better name? But by the same logic, LayoutData should be called TypeLayoutData etc.

You probably realize this, but the reason corouines have multiple variants is because they're state machines, similar to what async lowers to. ↩
Though I guess a struct Never(!); would end up with Variants::Never as well, wouldn't it... ↩

nnethercote · 2026-09-27T22:11:04Z

"Variant" has a standard, well-known meaning that is only relevant for enums. This code introduces a non-standard, subtly different meaning of "variant" that does make a certain sense. I think introducing the second meaning is the original sin here. New words might be better: Uninhabited, SinglyInhabited, MultiInhabited?

Variants::Single could be instead defined as "all inhabited types that are not Multiple

If "zero" is part of "multiple", yes... though defining something in terms of what it's not is rarely satisfying, especially when "single" is the most common case.

ada4a · 2026-09-27T22:23:48Z

"Variant" has a standard, well-known meaning that is only relevant for enums. This code introduces a non-standard, subtly different meaning of "variant" that does make a certain sense. I think introducing the second meaning is the original sin here. New words might be better: Uninhabited, SinglyInhabited, MultiInhabited?

Ah, sorry, I misunderstood you earlier. This does make a lot of sense – when expanding the docs on Variants in #163073, I found myself writing "inhabited variant" instead of just "variant" in a lot of places. Because of that, maybe the overall enum ought to be called InhabitedVariants – or just Inhabitedness, which would better fit the variant names you propose.

Variants::Single could be instead defined as "all inhabited types that are not Multiple

If "zero" is part of "multiple", yes...

That's what the "inhabited" in the definition would be for 😉 Uninhabited types end up with Empty

Rollup of 11 pull requests Successful merges: - #163085 (Suggest similarly named modules in import paths) - #163098 ([debugger visualizers] Add workaround to read `Rc` strong/weak counts) - #163120 (ci: make musl.sh look for patches next to the script) - #163301 (Fix unused_must_use for scenario which may need to keep value) - #163307 (Less `SpanData` in diagnostics) - #152972 (implement PartialEq<VecDeque<U>> for Vec<T>, &[T], &mut [T], [T; N], &[T; N] and &mut [T; N]) - #162536 (Implement Default for NumBuffer) - #163141 (Document safety requirements for intrinsic fallbacks) - #163384 (Various clean-ups around `LayoutCalculator`) - #163405 (Remove some #[linkage] options) - #163413 (mailmap: add Matilde Morrone)

Rollup of 11 pull requests Successful merges: - #163085 (Suggest similarly named modules in import paths) - #163098 ([debugger visualizers] Add workaround to read `Rc` strong/weak counts) - #163120 (ci: make musl.sh look for patches next to the script) - #163301 (Fix unused_must_use for scenario which may need to keep value) - #163307 (Less `SpanData` in diagnostics) - #163389 (`rustc_builtin_macros` cleanup, part 7) - #152972 (implement PartialEq<VecDeque<U>> for Vec<T>, &[T], &mut [T], [T; N], &[T; N] and &mut [T; N]) - #162536 (Implement Default for NumBuffer) - #163141 (Document safety requirements for intrinsic fallbacks) - #163384 (Various clean-ups around `LayoutCalculator`) - #163413 (mailmap: add Matilde Morrone)

@RalfJung

Rollup merge of #163384 - ada4a:push-xlkootrrlxkt, r=nnethercote Various clean-ups around `LayoutCalculator` Details in individual commits. The commits are roughly in the order of increasing controversiality. The changes are small overall imo, and I don't see a clear splitting point, but if you do want me to split up this PR, let me know. cc @RalfJung (sorry I keep pinging you despite you not being on a review rotation.. others seem not too keen to look at this code)

Rollup of 11 pull requests Successful merges: - rust-lang/rust#163085 (Suggest similarly named modules in import paths) - rust-lang/rust#163098 ([debugger visualizers] Add workaround to read `Rc` strong/weak counts) - rust-lang/rust#163120 (ci: make musl.sh look for patches next to the script) - rust-lang/rust#163301 (Fix unused_must_use for scenario which may need to keep value) - rust-lang/rust#163307 (Less `SpanData` in diagnostics) - rust-lang/rust#163389 (`rustc_builtin_macros` cleanup, part 7) - rust-lang/rust#152972 (implement PartialEq<VecDeque<U>> for Vec<T>, &[T], &mut [T], [T; N], &[T; N] and &mut [T; N]) - rust-lang/rust#162536 (Implement Default for NumBuffer) - rust-lang/rust#163141 (Document safety requirements for intrinsic fallbacks) - rust-lang/rust#163384 (Various clean-ups around `LayoutCalculator`) - rust-lang/rust#163413 (mailmap: add Matilde Morrone)

ada4a added 3 commits September 26, 2026 18:10

Use a let-chain

284af39

This whole section of the code deals with constructing an alternative layout, but large nesting makes the control flow seem more complex than it actually is.

Remove needless laziness

d2b3f9b

Constructing these error variants is basically free

Move complex clauses to the where clause

6c333a9

Having the generic list span multiple lines is quite jarring, and imo easy to confuse with the parameter list at a glance.

rustbot assigned nnethercote Sep 26, 2026

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 Sep 26, 2026

This comment has been minimized.

Sign in to view

ada4a force-pushed the push-xlkootrrlxkt branch from 951ffa7 to 593633d Compare September 26, 2026 21:33

This comment has been minimized.

Sign in to view

ada4a force-pushed the push-xlkootrrlxkt branch from 593633d to dc2f701 Compare September 26, 2026 21:45

This comment has been minimized.

Sign in to view

ada4a force-pushed the push-xlkootrrlxkt branch from dc2f701 to 4bb015b Compare September 26, 2026 21:54

Add layout_of_ prefix to all the layout-calculating methods

e1dfc38

Improves consistency, and hopefully clarifies the purpose of previously-mysteriosly named methods like `univariant`.

ada4a force-pushed the push-xlkootrrlxkt branch from 4bb015b to f598021 Compare September 26, 2026 22:02

nnethercote reviewed Sep 27, 2026

View reviewed changes

ada4a added 3 commits September 27, 2026 20:26

Add docs

37c0ed5

introduce NicheOptimizations

6668485

This is a more type-safe alternative for the `niche_optimizations: bool` param

ada4a force-pushed the push-xlkootrrlxkt branch from f598021 to 6668485 Compare September 27, 2026 19:16

rust-bors Bot 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 27, 2026

jhpratt mentioned this pull request Sep 27, 2026

Rollup of 11 pull requests #163415

Closed

jhpratt mentioned this pull request Sep 28, 2026

Rollup of 11 pull requests #163419

Merged

rust-bors Bot merged commit 66ea879 into rust-lang:main Sep 28, 2026
13 checks passed

rustbot added this to the 1.101.0 milestone Sep 28, 2026

ada4a deleted the push-xlkootrrlxkt branch September 28, 2026 08:20

assay-reviewer-app Bot mentioned this pull request Sep 30, 2026

fix(statusgen): autoflip credits the delivering PR by its Brief: trailer medici-finance/assay#1868

Merged

github-actions Bot mentioned this pull request Oct 3, 2026

Toolchain upgrade to nightly-2026-10-06 flux-rs/flux#1811

Draft

Conversation

ada4a commented Sep 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 26, 2026

Uh oh!

This comment has been minimized.

RalfJung commented Sep 26, 2026

Uh oh!

ada4a commented Sep 26, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

nnethercote left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ada4a Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ada4a Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nnethercote commented Sep 27, 2026

Uh oh!

rust-bors Bot commented Sep 27, 2026

Uh oh!

ada4a commented Sep 27, 2026

Footnotes

Uh oh!

nnethercote commented Sep 27, 2026

Uh oh!

ada4a commented Sep 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ada4a commented Sep 26, 2026 •

edited by rustbot

Loading

nnethercote left a comment •

edited by rustbot

Loading

ada4a Sep 27, 2026 •

edited

Loading

ada4a Sep 27, 2026 •

edited

Loading