Support repr alignment on unions. #43274
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
|
||
| #![feature(attr_literals)] | ||
| #![feature(repr_align)] | ||
| #![feature(untagged_unions)] |
There was a problem hiding this comment.
The feature should be unnecessary here.
There was a problem hiding this comment.
Tried removing it, but it seems it's still needed.
There was a problem hiding this comment.
I looked more into why this is the case - it's needed because of the struct inside the union in one of the tests. The struct is not Copy by default and unions require their members to be copy by default. Non-copy types in untagged unions aren't stabilised yet. If I made the struct derive from Copy then I could probably remove the feature, I think I'll leave it for now though.
| @@ -176,6 +182,18 @@ pub fn main() { | |||
| } | |||
| assert!(is_aligned_to(&e, 16)); | |||
|
|
|||
| // check union alignment | |||
| assert_eq!(mem::align_of::<UnionContainsAlign>(), 16); | |||
| assert_eq!(mem::size_of::<UnionContainsAlign>(), 16); | |||
There was a problem hiding this comment.
Why should it be 32? It's a union, not a struct, the size should be the same as the largest element shouldn't it?
|
|
||
| fn main() { | ||
| assert_eq!(align_of::<U16>(), 16); | ||
| assert_eq!(size_of::<U16>(), 16); |
There was a problem hiding this comment.
Same comment as above, size of should be max(union stride, a string and b stride) shouldn't it?
|
Disregard everything I wrote, it's 3 A.M. and I'm mixing bits and bytes. :shame: |
Mark-Simulacrum
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit ebc2f7d has been approved by |
Support repr alignment on unions. Requested as part of RFC 1358 #33626 (comment).
|
☀️ Test successful - status-appveyor, status-travis |
Requested as part of RFC 1358 #33626 (comment).