repr(transparent): don't consider most length-0 arrays trivial#155984
repr(transparent): don't consider most length-0 arrays trivial#155984Jules-Bertholet wants to merge 3 commits intorust-lang:mainfrom
repr(transparent): don't consider most length-0 arrays trivial#155984Conversation
|
rustbot has assigned @dingxiangfei2009. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
| if elem_trivial { | ||
| check_unsuited(tcx, typing_env, *elem_ty) |
There was a problem hiding this comment.
Why do you allow any arrays here? Seems easier to just reject them all?
I'd be surprised if there is much/any use of arrays as "trivial" types in repr(transparent).
There was a problem hiding this comment.
I think it's conceptually nice to treat "array with element type T" the same as "struct with field of type T". And it turns out that we need some non-trivial complexity anyway, to support the ghost crate.
For that we need a version of this that emits a hard error. |
72a9d17 to
d4c13b1
Compare
This comment has been minimized.
This comment has been minimized.
With this PR, an array type is considered trivial for the purpose of `repr(transparent)` only if its element type is—we emit the `repr_transparent_non_zst_fields` FCW otherwise. This has two benefits: ## Forbid non-portable definitions Some types have alignment 1 only on certain platforms. Prior to this PR, the following snippet would compile on AVR, and *only* on AVR: ```rust #[repr(transparent)] struct Foo(i32, [u16; 0]); ``` After this PR, the above now fails to compile on any target. ## FFI and CFI compatibility We want to add support for Control Flow Integrity to Rust at some point. There are some good reasons to want CFI to consider `*const [u8; 0]` and `*const [u8; 1]` compatible with one another. But that means we must consider `*const [u8; 0]` and `*const ()` to be CFI-incompatible. Declaring `[u8; 0]` non-trivial for `repr(transparent)` makes that easier to achieve. See discussion on Zulip: <https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/ABI-compatibility.20rules.20of.20ZST.20types/near/591412488>
Needed to support the `ghost` crate.
c5c22f8 to
2af89b0
Compare
|
The Miri subtree was changed cc @rust-lang/miri |
|
The PR should now be ready for crater @rustbot ready |
With this PR, an array type is considered trivial for the purpose of
repr(transparent)only if its element type is—we emit therepr_transparent_non_zst_fieldsFCW (#78586) otherwise. To support a pattern used by theghostcrate, we also permit all array types with length 0 when they are contained within arepr(Rust, packed(1))ADT.This has two benefits:
Forbid non-portable definitions
Some types have alignment 1 only on certain platforms. Prior to this PR, the following snippet would compile on AVR, and only on AVR:
After this PR, the above now fails to compile on any target.
FFI and CFI compatibility
We want to add support for Control Flow Integrity to Rust at some point. There are some good reasons to want CFI to consider
*const [u8; 0]and*const [u8; 1]compatible with one another. But that means we must consider*const [u8; 0]and*const ()to be CFI-incompatible. Declaring[u8; 0]non-trivial forrepr(transparent)makes that easier to achieve. See discussion on Zulip:https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/ABI-compatibility.20rules.20of.20ZST.20types/near/591412488
@rustbot label T-lang needs-fcp A-repr
Also needs a crater run.