Clarify guarantees provided by repr(packed)

[joshlf]

Clarify that repr(packed) minimizes inter-field padding and, in the special case of repr(packed(1))/repr(packed), guarantees no inter-field padding.

[Gankra]

See also #1152 which is similarly trying to clarify repr details which are seemingly-obvious but worth stating explicitly.

[joshlf]

See also #1152 which is similarly trying to clarify repr details which are seemingly-obvious but worth stating explicitly.

Thanks, that's awesome to see!

[joshlf]

What are the next steps to get this approved/merged?

[ehuss]

I'm not entirely certain what this is trying to clarify. If the fields are aligned in a certain way, the size of the padding falls out as a consequence of the field layout. In what way would there be more padding than what is required? I also don't see packed(1) as being much of a special case, or at least not different in the layout method.

Can you say more about why it seems like these guarantees need to be explicitly called out?

[djkoloski]

repr(Rust) structs explicitly do not make any guarantees about data layout. Under the previous wording, a repr(Rust, packed) struct could have inter-field padding as packed only "may also alter the padding between fields". This updated wording guarantees that repr(Rust, packed) structs have no inter-field padding and other packed(N) reprs have the minimum amount of padding to satisfy the repr.

[ehuss]

Perhaps the "no guarantees" is worded too specifically, and #1152 is trying to soften that. Using packed with the default representation still follows the rules set out in the packed section.

The note about "may also alter padding between fields" is more of a heads-up warning that packed(N) may change the offsets of the fields and as a consequence changes to padding may occur. The real rules are specified in the paragraph below. It is not a statement that padding may be arbitrarily altered. Perhaps that sentence could be reworded to make that clearer? Like say `packed` may also alter the alignment of each field as specified below. and just not mention "padding" if it is misleading?

[djkoloski]

Even with that clarification, I think the problem still remains. Even though a struct/union and all of its fields are lowered to 1-alignment, that does not guarantee that it will not have padding. The layout could add arbitrary padding (for example, to reach some more desirable total size) unless it's specifically guaranteed that repr(packed(1)) structs/unions do not have any padding. At least, that's my understanding of how alignment interacts with type layout.

[joshlf]

Even with that clarification, I think the problem still remains. Even though a struct/union and all of its fields are lowered to 1-alignment, that does not guarantee that it will not have padding. The layout could add arbitrary padding (for example, to reach some more desirable total size) unless it's specifically guaranteed that repr(packed(1)) structs/unions do not have any padding. At least, that's my understanding of how alignment interacts with type layout.

This is my understanding as well - the reference, as currently written, does not guarantee that a packed struct will have the minimum possible padding. It allows that that could happen, but it doesn't guarantee that it will. This PR is intended to change that so that it can be relied upon.

I also don't see packed(1) as being much of a special case, or at least not different in the layout method.

It's a natural consequence of the guarantee I'd like to add, and doesn't technically need to be there, agreed. However, it's a very common use case, and so I figured it was useful to call it out specifically. I'd be happy to rephrase it as "an important consequence of these rules" or something like that.

[ehuss]

I'd be happy to rephrase it as "an important consequence of these rules" or something like that.

That might be better.
There also appears to be a CI error.

@rust-lang/lang Nominating this for discussion/approval. This seems reasonable to me, though I don't know if there is something I haven't considered.

[joshlf]

I'd be happy to rephrase it as "an important consequence of these rules" or something like that.

That might be better.

Done.

[nikomatsakis]

These changes look good to me -- I like the new wording.

[nikomatsakis]

(We'll cover it in the lang team meeting tomorrow, presumably.)

[nikomatsakis]

We discussed this in today's @rust-lang/lang meeting and the general consensus was that this change made sense, but that it also made sense to do an FCP since this was a change to the lang spec, if only a small one. Therefore...

@rfcbot fcp merge

[rfcbot]

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

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm

No concerns currently listed.

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!

See this document for info about what commands tagged team members can give me.

[rfcbot]

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

psst @nikomatsakis, I wasn't able to add the final-comment-period label, please do so.

[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.

This will be merged soon.

psst @nikomatsakis, I wasn't able to add the finished-final-comment-period label, please do so.

[ehuss]

Thanks!