Skip to content

Offsets of union fields #353

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

Open
alercah opened this issue Jul 17, 2022 · 1 comment
Open

Offsets of union fields #353

alercah opened this issue Jul 17, 2022 · 1 comment

Comments

@alercah
Copy link

alercah commented Jul 17, 2022

Safe access to union fields is a feature with interest. See #352 for more background especially on the issue of what safety invariant applies to unions.

Assuming that a suitable safety invariant is selected, there is also the question of how unions are laid out in #[repr(Rust)] unions. It is, as with most #[repr(Rust)] layout parameters, currently unspecified.

This means that #[repr(Rust)] unions are useful pretty much only as untagged sum types, because they can never be used to perform any sort of type punning whatsoever. This has sometimes been defended based on the possibility of niche optimizations by aligning fields, however, since #73 is largely converged on all bit-patterns being valid on unions, niche optimizations are impossible for them anyway.

Consequently, there is little reason to not guarantee that #[repr(Rust)] unions always lay fields out at offset 0. Do we do this? If the #[safe_transmute_union] attribute suggested by @CAD97 were used, would we only guarantee this property if that were used (in which case I might suggest #[repr(safe)] as the attribute?).

@CAD97
Copy link

CAD97 commented Jul 17, 2022

Seconding that (assuming unions have trivial validity) we should just go ahead and guarantee all fields in a #[repr(Rust)] union are placed at offset 0. I would stay away from using #[repr] for #[safe_transmute_union], as a repr attribute adding new safe behaviors is unprecedented and honestly kinda scary. (#[repr(packed)] removes the ability to take the reference of fields, and this has been a huge footgun, especially because we forgot to make it unsafe 🙃)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants