Skip to content

Document rustc_abi::VariantLayout - #163256

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
ada4a:push-zntxyqulsvvs
Sep 26, 2026
Merged

Document rustc_abi::VariantLayout#163256
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
ada4a:push-zntxyqulsvvs

Conversation

@ada4a

@ada4a ada4a commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #151742.

cc @saethlin @moulins @RalfJung as you were involved with the original PR

@rustbot 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 24, 2026
@rustbot

rustbot commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

r? @mu001999

rustbot has assigned @mu001999.
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 21 candidates

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@ada4a
ada4a force-pushed the push-zntxyqulsvvs branch 2 times, most recently from 783d49c to 908b36e Compare September 24, 2026 13:00
ada4a
ada4a commented Sep 24, 2026
View reviewed changes
Comment thread compiler/rustc_abi/src/lib.rs Outdated
#[cfg_attr(feature = "nightly", derive(StableHash))]
pub struct VariantLayout<FieldIdx: Idx> {
pub size: Size,
// FIXME: enum variants should not need their own `BackendRepr`, so this can be removed

@ada4a ada4a Sep 24, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd taken this comment from #151742 (comment), but have since found this piece of code:

rust/compiler/rustc_abi/src/layout.rs

Lines 677 to 680 in 2350b22

// It can't be a Scalar or ScalarPair because the offset isn't 0.
if !layout.is_uninhabited() {
layout.backend_repr = BackendRepr::Memory { sized: true };
}

which seems to contradict it

View changes since the review

@ada4a ada4a Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this?

rust/compiler/rustc_abi/src/layout.rs

Lines 1021 to 1035 in 2350b22

// If we pick a "clever" (by-value) ABI, we might have to adjust the ABI of the
// variants to ensure they are consistent. This is because a downcast is
// semantically a NOP, and thus should not affect layout.
if matches!(abi, BackendRepr::Scalar(..) | BackendRepr::ScalarPair { .. }) {
for variant in &mut layout_variants {
// We only do this for variants with fields; the others are not accessed anyway.
// Also do not overwrite any already existing "clever" ABIs.
if matches!(variant.backend_repr, BackendRepr::Memory { .. } if variant.has_fields())
{
variant.backend_repr = abi;
// Also need to bump up the size, so that the entire value fits in here.
variant.size = cmp::max(variant.size, size);
}
}
}

@RalfJung RalfJung Sep 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that 2nd snippet shows the hacks we need to ensure that backend_repr is mostly consistent. If variant did not have a backend_repr, we could simplify things.

That example also shows that size is another value that variants probably shouldn't have.

@mu001999

mu001999 commented Sep 24, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned chenyukang and unassigned mu001999 Sep 24, 2026
RalfJung
RalfJung reviewed Sep 25, 2026
View reviewed changes
Comment thread compiler/rustc_abi/src/lib.rs
@chenyukang

chenyukang commented Sep 25, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned petrochenkov and unassigned chenyukang Sep 25, 2026
@petrochenkov

petrochenkov commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

r? @RalfJung

@rustbot rustbot assigned RalfJung and unassigned petrochenkov Sep 25, 2026
@rustbot

rustbot commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

RalfJung
RalfJung reviewed Sep 26, 2026
View reviewed changes
Comment thread compiler/rustc_abi/src/lib.rs
@ada4a

ada4a commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@RalfJung

RalfJung commented Sep 26, 2026

Copy link
Copy Markdown
Member

Thanks!

@bors r+ rollup

@rust-bors

rust-bors Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

📌 Commit f5c85c8 has been tentatively approved by RalfJung

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors 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 26, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Sep 26, 2026
Merged
rust-bors Bot pushed a commit that referenced this pull request Sep 26, 2026
@bors
Auto merge of #163386 - JonathanBrouwer:rollup-VqKqcjW, r=JonathanBro…
895d5ee 
…uwer

Rollup of 6 pull requests

Successful merges:

 - #150824 (Document platform-specific behavior of `current_exe`, including that Linux can add `" (deleted)"`)
 - #159564 (Allow elided ('static) lifetimes in `thread_local!`)
 - #163026 (hir_typeck: simplify `upvar::determine_capture_info` impl)
 - #163256 (Document `rustc_abi::VariantLayout`)
 - #163366 (Stabilize SyncView)
 - #163376 (Option, Result: not all arguments passed to map_or are eagerly evaluated)
rust-bors Bot pushed a commit that referenced this pull request Sep 26, 2026
@bors
Auto merge of #163386 - JonathanBrouwer:rollup-VqKqcjW, r=JonathanBro…
75a75c3 
…uwer

Rollup of 6 pull requests

Successful merges:

 - #150824 (Document platform-specific behavior of `current_exe`, including that Linux can add `" (deleted)"`)
 - #159564 (Allow elided ('static) lifetimes in `thread_local!`)
 - #163026 (hir_typeck: simplify `upvar::determine_capture_info` impl)
 - #163256 (Document `rustc_abi::VariantLayout`)
 - #163366 (Stabilize SyncView)
 - #163376 (Option, Result: not all arguments passed to map_or are eagerly evaluated)
@rust-bors
rust-bors Bot merged commit 5ccaeb5 into rust-lang:main Sep 26, 2026
13 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Sep 26, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 26, 2026
@bors
Unrolled build for #163256 in rollup 163386
8b73c5f 
Rollup merge of #163256 - ada4a:push-zntxyqulsvvs, r=RalfJung

Document `rustc_abi::VariantLayout`

Follow-up to #151742.

cc @saethlin @moulins @RalfJung as you were involved with the original PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees

@RalfJung RalfJung

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.101.0

Development

Successfully merging this pull request may close these issues.

7 participants

@ada4a @rustbot @rust-log-analyzer @mu001999 @chenyukang @petrochenkov @RalfJung