Uh oh!
There was an error while loading. Please reload this page.
Collapse #[repr(transparent)] types - #619
Conversation
There was a problem hiding this comment.
This looks directionally good to me.
Insole_structural_newtype_field it talks about “one non-ZST field”, but repr(transparent) is more specific...ignored fields must be both size 0 and align 1. Looks like we are only checking is_zst()?
Also a few more projection tests might be good, something like:
#[repr(transparent)]structW(Inner);and then use W through &W, &mut W, raw pointers, arrays/slices, function args/returns, and nested transparent wrappers. Mostly want to make sure that once the wrapper type gets collapsed, accessing .0 still becomes a no-op and we don't accidentally emit an invalid access chain.
Might worth adding a test for an over-aligned ZST too:
#[repr(align(8))]structBlah;#[repr(transparent)]structW{inner:u32,marker:Blah,}marker is size 0 but it has alignment 8 so it can affect W’s layout.
Requesting changes, but feel free to put up again if you are merely extracting and don't want to change anything.
a639684 to
71ed06fCompareFirestar99
commented
Jul 2, 2026
Rewrote all the tests, seems like all the GEPs are emitted just fine |
71ed06f to
dcbf8f4CompareLegNeato
commented
Aug 5, 2026
@eddyb please take a look. |
eddyb
left a comment
There was a problem hiding this comment.
Oh, this is so easy now, thanks to SpirvType not having to match the rustc-side layout (other than having fields at the right offsets).
I've wanted to do this in the past, but the existence of e.g. struct_gep was blocking it (seems like that has been gone since rust-lang/rust@beed25b huh).
My only worry now is whether we have code acting on Ty/layout, and SpirvType, simultaneously, and expecting any connection between the two.
dcbf8f4 to
756cca0Compare756cca0 to
dcce081CompareFirestar99
commented
Aug 10, 2026
Forge-merging since @LegNeato's review is blocking the merge |
Uh oh!
There was an error while loading. Please reload this page.
nazar-pc
commented
Aug 10, 2026
Interesting, does this suffer from the same issue?: #[repr(C)]pubstructX(u32);If so, then it should probably be treated the same way as |
Firestar99
commented
Aug 11, 2026
I wasn't going to do it with |
nazar-pc
commented
Aug 11, 2026
I'm sure a general case is more involved, but I'd expect for trivial integers the result should be identical. This PR got me thinking if I'm leaving performance on the table by complicating the code with |
Supersedes #566