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.

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

Attempt to document the current state of the union. Part 1: Layout #365

Closed
wants to merge 1 commit into from

Conversation

alercah
Copy link

@alercah alercah commented Oct 8, 2022

I have tried to document everything I believe we have consensus on. I've left some things open that I possibly could have closed, but because this PR is very big, I would like to focus on getting it in as quickly as possible and worrying about whatever's left afterwards.

I strongly encourage others to submit follow up PRs to close out the other open issues.

Closes #156.
Closes #298.
Closes #352.

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Thanks for finally restarting the writeup process! I left a bunch of comments, but also the entire thing is too long for me to review in one sitting, so if there are ways to split this up then we should use them.

@@ -207,6 +208,8 @@ requirement of 2.

*Padding* (of a type `T`) refers to the space that the compiler leaves between fields of a struct or enum variant to satisfy alignment requirements, and before/after variants of a union or enum to make all variants equally sized.

Padding for a type is either [interior padding], which is part of one or more fields, or [exterior padding], which is before, between, or after the fields.
Copy link
Member

Choose a reason for hiding this comment

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

What is this useful for?

Copy link
Author

Choose a reason for hiding this comment

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

Unclear if you mean the sentence or the definitions in general.

The definitions of interior and exterior padding I added early on, expecting to need to refer to them, but revisions may have made them less necessary. I will make sure to take a look on whether they are still needed.

This particular sentence was because I find "X is either Y or Z" type statements helpful to in the context of a definition of X, to make clear that all X is exactly one of Y or Z. But if you don't like it I don't mind removing it.

Copy link
Member

Choose a reason for hiding this comment

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

I was referring to the distinction of interior and exterior padding... as my later comments indicated, I think that is terminology we just don't need.

[ EXT [ field0_0_ty | INT | field0_1_ty | INT ] EXT ]
[ EXT [ field1_0_ty | INT | NON NON NON | INT ] EXT ]
[ EXT | NON NON NON | INT [ field2_0_ty ] INT | EXT ]
```
Copy link
Member

@RalfJung RalfJung Oct 8, 2022

Choose a reason for hiding this comment

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

IMO a simpler way to define this is to say: for each 'row' (variant) of the union, we define the set of padding bytes as

  • whatever padding bytes the field has
  • all the bytes before and after the field

Then the padding of the union is simply the intersection of those sets of padding bytes.

In particular for unions, the exterior/interior padding distinction is kind of mirky, since we can have a layout like

[ [field0_ty] <----> ]
[ <----> [field1_ty] ]

where the same byte is exterior padding in one variant and interior padding in another variant.

Copy link
Author

Choose a reason for hiding this comment

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

Then the padding of the union is simply the intersection of those sets of padding bytes.

I was assuming that for a Repr-raw union, we would want the set of padding bytes to be exactly the exterior padding bytes, i.e., the bytes that are padding before and after all fields, ignoring field-internal padding. Is this not true?

Copy link
Member

Choose a reason for hiding this comment

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

It is not true for current unions, there were examples somewhere of types like MaybeUninit<(u16, u32)> being passed in 2 registers and thus having the padding of the fields lost.

For a "preserve all bits union" I was assuming we have no padding at all.

@alercah alercah changed the title Attempt to document the current state of the union. Attempt to document the current state of the union. Part 1: Layout Oct 15, 2022
@alercah alercah mentioned this pull request Oct 15, 2022
This isn't intended to create controversy, but document where discussion
has settled. Please feel free to open more PRs to clear up additional
items.

Closes rust-lang#156.
#### Padding (tail)
[tail padding]: #tail-padding

Tail padding is [exterior padding] that comes after all fields of a type.
Copy link
Member

Choose a reason for hiding this comment

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

Why is it useful to distinguish tail padding from other padding? We have padding before, between (only for structs), and after fields. Seems strange to not teat them the same everywhere, and if we do treat them the same, we don't need these special terms.

@RalfJung
Copy link
Member

RalfJung commented Aug 1, 2023

Closing due to inactivity. Feel free to reopen if you want to get back to this, but note that the UCG book isn't really maintained these days. That said I am not sure where else such information would go so I am still fine with keeping it here.

@RalfJung RalfJung closed this Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants