Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. #141947

zachs18 · 2025-06-03T06:34:11Z

Add that the enum must be #[repr(Rust)] and not #[repr(packed)] or #[repr(align)] in order to be ABI-compatible with its null-pointer-optimized field.

The specific rules here were decided on here: #130628 (comment) but repr was not mentioned. In practice, only #[repr(Rust)] (or no repr attribute, which is equivalent) works for this, so add that to the docs.

Restrict to #[repr(Rust)] only, since:

#[repr(C)] and the primitive representations (#[repr(u8)] etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload.
#[repr(transparent)] enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway.

As for #[repr(align)]: my current wording that it is completely disallowed may be too strong: it seems like #[repr(align(<= alignment of T))] enum Foo { X, Y(T) } currently does still have the same ABI as T in practice, though this may not be something we want to promise. (#[repr(align(> alignment of T))] definitely disqualifies the enum from being ABI-compatible with T currently).

I added the note about packed to match align, but #[repr(packed)] currently can't be applied to enums at all anyway, so might be unnecessary.

I think this needs T-lang approval?

cc @workingjubilee

Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field.

rustbot · 2025-06-03T06:34:15Z

r? @thomcc

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

workingjubilee · 2025-06-03T16:01:34Z

Wording about repr(align(N)) should probably be shaped in the form of advising people about the i64, u64, and f64 gotcha: on some platforms it is 8-aligned, on some platforms one or all are 4-aligned, and if you use repr(align(8)) because the type is 8 bytes, then you just lost the guarantee on those platforms. Likewise for all the primitive types, though even-lower alignments often aren't a concern here.

workingjubilee · 2025-06-03T16:13:26Z

I agree though that this needs review from

r? lang

workingjubilee · 2025-06-03T18:02:03Z

library/core/src/primitive_docs.rs

+///   - The enum `E` uses the [`Rust` representation], and is not modified by the `align` or
+///     `packed` representation modifiers.


Changing alignment of something can change its ABI, and both of these repr attributes, as modifiers, can do that. So I am okay with having the guarantee be conditional on any usage of the repr attributes. If T-lang wants us to guarantee that the attributes only invalidate the nullptr opt if they actually would result in a change in the alignment of the type, that may be doable, but it would require auditing the code involved and landing a number of tests, so I don't know that it should happen in this PR. Also, see my next comment.

workingjubilee · 2025-06-03T18:03:04Z

library/core/src/primitive_docs.rs

@@ -1833,6 +1833,8 @@ mod prim_ref {}
 /// - If `T` is guaranteed to be subject to the [null pointer
 ///   optimization](option/index.html#representation), and `E` is an enum satisfying the following
 ///   requirements, then `T` and `E` are ABI-compatible. Such an enum `E` is called "option-like".
+///   - The enum `E` uses the [`Rust` representation], and is not modified by the `align` or
+///     `packed` representation modifiers.
 ///   - The enum `E` has exactly two variants.
 ///   - One variant has exactly one field, of type `T`.
 ///   - All fields of the other variant are zero-sized with 1-byte alignment.


Reviewing more closely, I feel guaranteeing e.g. repr(align(2)) won't change the layout if T is 2-aligned would be inconsistent with this clause.

We could potentially raise this alignment to whatever the alignment of T is, for example. I don't want to alter the guarantee around the 1ZST case, however, as it places a number of annoying requirements on the implementation. Being able to special-case specifically on 1ZSTs simplifies a lot of things. I would be okay with guaranteeing that repr(align(1)) doesn't change anything, though, because it doesn't. There's also some inclarity about certain special cases around ZSTs already when they combine with other repr attributes like repr(C).

workingjubilee

To summarize my thoughts, this is fine with me as-is, speaking re: my (relative) familiarity with the layout code's bits and bobs.

traviscross

@rustbot labels +I-lang-nominated +P-lang-drag-1 +S-waiting-on-team

The diff looks right to me, and I agree with @workingjubilee's comments as well, but it would indeed be better for the team to have a look here first, so let's not merge this yet. This is the sort of thing where we all need to have a look at it, if nothing else, just to ensure we all keep context on what exactly we've said the rule is.

workingjubilee · 2025-06-11T19:09:26Z

@traviscross Based on the triage meeting notes, lang had a fair vibe about this. Was there an intention to r+ this as a simple amendment/clarification to the prior decision, FCP it, or is there a further discussion that I maybe should show up for?

I think if lang is happy with this except with the repr(align(N)) where N > 1 question, and it doesn't need an FCP, then we should ship this as-is and look at strengthening the guarantee in a followup. If it does need an FCP then we might as well do the haggling now.

traviscross · 2025-06-12T03:53:31Z

Thanks, yes, I mentioned in the call I was going to...

@bors r=workingjubilee,traviscross rollup

bors · 2025-06-12T03:53:34Z

📌 Commit c0851d7 has been approved by workingjubilee,traviscross

It is now in the queue for this repository.

…traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: rust-lang#130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc `@workingjubilee`

…traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: rust-lang#130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc ``@workingjubilee``

…traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: rust-lang#130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc ```@workingjubilee```

Rollup of 9 pull requests Successful merges: - #138016 (Added `Clone` implementation for `ChunkBy`) - #140770 (add `extern "custom"` functions) - #141162 (refactor `AttributeGate` and `rustc_attr!` to emit notes during feature checking) - #141474 (Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints) - #141947 (Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field.) - #142135 (docs: autogenerate compiler flag stubs based on -Zhelp) - #142252 (Improve clarity of `core::sync::atomic` docs about "Considerations" in regards to CAS operations) - #142337 (miri: add flag to suppress float non-determinism) - #142353 (compiler: Ease off the accelerator on `unsupported_calling_conventions`) r? `@ghost` `@rustbot` modify labels: rollup

…traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: rust-lang#130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc ````@workingjubilee````

Rollup of 8 pull requests Successful merges: - #138016 (Added `Clone` implementation for `ChunkBy`) - #140770 (add `extern "custom"` functions) - #141162 (refactor `AttributeGate` and `rustc_attr!` to emit notes during feature checking) - #141474 (Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints) - #141947 (Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field.) - #142252 (Improve clarity of `core::sync::atomic` docs about "Considerations" in regards to CAS operations) - #142337 (miri: add flag to suppress float non-determinism) - #142353 (compiler: Ease off the accelerator on `unsupported_calling_conventions`) r? `@ghost` `@rustbot` modify labels: rollup

…traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: rust-lang#130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc `````@workingjubilee`````

Rollup of 7 pull requests Successful merges: - #138016 (Added `Clone` implementation for `ChunkBy`) - #141162 (refactor `AttributeGate` and `rustc_attr!` to emit notes during feature checking) - #141474 (Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints) - #141947 (Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field.) - #142252 (Improve clarity of `core::sync::atomic` docs about "Considerations" in regards to CAS operations) - #142337 (miri: add flag to suppress float non-determinism) - #142353 (compiler: Ease off the accelerator on `unsupported_calling_conventions`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 6 pull requests Successful merges: - #138016 (Added `Clone` implementation for `ChunkBy`) - #141162 (refactor `AttributeGate` and `rustc_attr!` to emit notes during feature checking) - #141474 (Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints) - #141947 (Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field.) - #142252 (Improve clarity of `core::sync::atomic` docs about "Considerations" in regards to CAS operations) - #142337 (miri: add flag to suppress float non-determinism) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #141947 - zachs18:patch-4, r=workingjubilee,traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: #130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc ``````@workingjubilee``````

Update ABI compatibility docs about null-pointer-optimized enums.

c0851d7

Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field.

rustbot assigned thomcc Jun 3, 2025

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

rustbot added the T-lang Relevant to the language team label Jun 3, 2025

rustbot assigned traviscross and unassigned thomcc Jun 3, 2025

workingjubilee reviewed Jun 3, 2025

View reviewed changes

workingjubilee approved these changes Jun 3, 2025

View reviewed changes

traviscross approved these changes Jun 10, 2025

View reviewed changes

workingjubilee changed the title ~~Specify that "option-like" enums must be #[repr(Rust)] to be ABI-compatibile with their non-1ZST field.~~ Specify that "option-like" enums must be #[repr(Rust)] to be ABI-compatible with their non-1ZST field. Jun 10, 2025

Amanieu removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 11, 2025

workingjubilee added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 12, 2025

workingjubilee mentioned this pull request Jun 12, 2025

Rollup of 15 pull requests #142396

Closed

workingjubilee mentioned this pull request Jun 12, 2025

Rollup of 15 pull requests #142397

Closed

matthiaskrgr mentioned this pull request Jun 12, 2025

Rollup of 9 pull requests #142399

Closed

matthiaskrgr mentioned this pull request Jun 12, 2025

Rollup of 8 pull requests #142409

Closed

matthiaskrgr mentioned this pull request Jun 12, 2025

Rollup of 7 pull requests #142421

Closed

matthiaskrgr mentioned this pull request Jun 12, 2025

Rollup of 6 pull requests #142432

Merged

bors merged commit 0c81fd0 into rust-lang:master Jun 13, 2025
10 checks passed

rustbot added this to the 1.89.0 milestone Jun 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. #141947

Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. #141947

zachs18 commented Jun 3, 2025

Uh oh!

rustbot commented Jun 3, 2025

Uh oh!

workingjubilee commented Jun 3, 2025 •

edited

Loading

Uh oh!

workingjubilee commented Jun 3, 2025

Uh oh!

workingjubilee Jun 3, 2025

Uh oh!

workingjubilee Jun 3, 2025

Uh oh!

workingjubilee left a comment

Uh oh!

traviscross left a comment

Uh oh!

workingjubilee commented Jun 11, 2025

Uh oh!

traviscross commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

Uh oh!

Uh oh!

		/// - The enum `E` uses the [`Rust` representation], and is not modified by the `align` or
		/// `packed` representation modifiers.

Specify that "option-like" enums must be #[repr(Rust)] to be ABI-compatible with their non-1ZST field. #141947

Specify that "option-like" enums must be #[repr(Rust)] to be ABI-compatible with their non-1ZST field. #141947

Conversation

zachs18 commented Jun 3, 2025

Uh oh!

rustbot commented Jun 3, 2025

Uh oh!

workingjubilee commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Jun 3, 2025

Uh oh!

workingjubilee Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

workingjubilee Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

workingjubilee left a comment

Choose a reason for hiding this comment

Uh oh!

traviscross left a comment

Choose a reason for hiding this comment

Uh oh!

workingjubilee commented Jun 11, 2025

Uh oh!

traviscross commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

Uh oh!

Uh oh!

Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. #141947

Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. #141947

workingjubilee commented Jun 3, 2025 •

edited

Loading