Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guarantee repr(C) enum discriminants are stable within a compilation #1454

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Guarantee repr(C) enum discriminants are stable within a compilation #1454

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/type-layout.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@ the default `enum` size and alignment for the target platform's C ABI.
> Note: The enum representation in C is implementation defined, so this is
> really a "best guess". In particular, this may be incorrect when the C code
> of interest is compiled with certain flags.
>
> However, it is guaranteed that, for any two `repr(C)` enum types, their
> discriminants will have the same size and alignment *within a given
> compilation of the program.* This is true for both field-less enums and
> for enums with fields.
Comment on lines +309 to +311
Copy link
Contributor

Choose a reason for hiding this comment

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

"within a given compilation of the program" doesn't seem clear to me what that means. Certainly all types have the same size and alignment during compilation. I'm not sure I can guess what this is trying to say, can you clarify?

Copy link
Contributor Author

@joshlf joshlf May 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe I should phrase this differently. The idea is that, whatever choice rustc makes about the size and alignment of the discriminant of a repr(C) enum, it makes the same choice for all repr(C) enums within a program.

The reason this is important is that the size and alignment aren't guaranteed in any way other than to say "rustc will choose some size and alignment." This puts unsafe code in a bind: how do you know what the size and alignment of the discriminant is if you're trying to work with the layout of an enum type? With this added text, you can construct a "dummy" field-less repr(C) enum, query for its size and alignment using size_of and align_of, and take these to be equal to the size and alignment of the discriminant of the enum you're working with.

The specific context is that zerocopy's TryFromBytes trait can currently be derived on field-less enums, and we want to add support for data-ful enums, but we can't without this guarantee (or some other way of querying for the size and alignment of repr(C) enum discriminants).

Copy link
Contributor

Choose a reason for hiding this comment

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

@joshlf: We discussed this in the rustdocs call today. This is going to need to be lang nominated, but before we do that, we'd like this language to be as clear as possible. Do you want to take another pass at stating this more clearly as discussed?

Copy link
Member

Choose a reason for hiding this comment

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

You probably mean the tag, not the discriminant. The discriminant is the value you get when you cast an enum to an integer or call std::mem::discriminant on it. The tag is the actual representation in memory. It may be smaller or larger than the discriminant. Without repr(C) or repr(iN) the tag may be a different integer than the discriminant or the discriminant may be represented using niches instead.


<div class="warning">

Expand Down