Inhibit all-absent-variant optimization for all enum reprs that inhibit layout optimization, not just repr(C).

[zachs18]

View all comments

Fixes #146984

[rustbot]

r? @SparrowLii

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

[theemathas]

Does this need a T-lang FCP?

[rustbot]

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.

[SparrowLii]

This looks fine to me. I'd like at least someone from t-lang to have a aware

r? lang

[zachs18]

@nikomatsakis Friendly ping. Does this need a T-lang FCP?

[zachs18]

@rustbot label I-lang-nominated

[Nadrieril]

For context, repr.inhibit_enum_layout_opt() means repr.c() || repr.int.is_some() today.

[traviscross]

@bors2 try

[rust-bors]

Unknown command "2". Run @bors help to see available commands.

[joshtriplett]

We talked about this in today's @rust-lang/lang meeting.

repr(int) (e.g. repr(u32)) does have some layout guarantees, and we were not meeting those guarantees. This could affect the layout of real structures. (Even though the enum in question would have to be uninhabited, it could appear somewhere that affects the size of an overall inhabited type, such as a union variant or enum variant.)

We'd like to evaluate making this change. It'll need relnotes (in the compatibility section), and we should also ping some of the folks who do creative serialization/deserialization stunts (e.g. zerocopy, rkyv) to find out if they might be affected by this.

@rfcbot merge lang
@rfcbot concern crater

[rust-rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

Concerns:

• crater resolved by Inhibit all-absent-variant optimization for all enum reprs that inhibit layout optimization, not just repr(C). #146989 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[zachs18]

Seems like all the retried regression results were the same as before. None seem related.

regressions

Crates.io:

Probably flaky

• netupi-core - probably flaky test (multiple tests use PersistenceManager::new(), which uses the same files every time, and some tests call PersistenceManager::save_config which writes to one of the files)
• nostalgia - probably flaky test (multiple doctests use Storage::new("/tmp/db"), including the doctest for delete)
• pswatch - probably flaky test (assumes that a sleep 5 process has not completed after std::thread::sleep(1 second))

Unknown, but probaly flaky

• swarc - idk, but doesn't use repr

Github regressions (all flaky or probably flaky):

• isgasho/openforum and lann/code-snippets - probably unrelated build issue - duplicate symbol canonical_abi_free, does not use repr
  • maybe due to old version of wit-bindgen-rust(?) (0.1.0 vs 0.52.0 latest)
• luansilvadb/backend_pbx - probably flaky, tests set env vars, doesn't use repr
• Lucian-4a25/rust-sort-algorithm - flaky, reproducible on nightly (incorrect custom sorting algorithm implementation)
• OldSinner/rs-json-handler - flaky, reproducible on stable (hashmap iteration order)
• rwcrosby/ncexe - probably flaky "Err value: Colors not supported" I assume crater's terminal doesn't support colors
• Sigrhofundr/exam_20250703 - probably flaky, timeout with multithreading and no synchronization, doesn't use repr
• teschmitt/bzippr - flaky, reproducible on stable

@rustbot label I-lang-nominated

[joshtriplett]

@rfcbot resolve crater

Thank you for the analysis!

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[Kivooeo]

This was approved and the FCP was completed successfully. I suppose it's time to merge it

@bors r+

[rust-bors]

📌 Commit 85872cf has been approved by Kivooeo

It is now in the queue for this repository.

[Kivooeo]

Let me do this more accurate

@bors r-

@bors r=SparrowLii

[rust-bors]

📌 Commit 85872cf has been approved by SparrowLii

It is now in the queue for this repository.

[traviscross]

Cc @rust-lang/lang-docs @rust-lang/fls