How can we safely implement `bytemuck::Pod` generically?

[ripytide]

I was going to add bytemuck::Pod back to the types when I saw that bytemuck has a derive macro for it so I was going to use that but it won't let me:

rustc: Pod requires cannot be derived for non-packed types containing generic parameters because the padding requirements can't be verified for generic non-packed structs

It seems that since the generic type T could be have padding in it we can't implement Pod on it safely. Is there a DoesNotContainPadding marker trait of some sort so we can implement this safely?

In v0.8 this was implemented simply as:

unsafe impl<T> crate::Pod for RGB<T> where T: crate::Pod {}

But as mentioned this is not a safe implementation so I'd like to restrict it a bit more so it becomes safe.

[kornelski]

Unfortunately there's no trait for no padding.

nevermind, wrong example

~~Indeed the current blanket impl is unsound~~ I haven't considered that alignment can add padding, and it seems to even override `packed`!: ```rust #[repr(align(16))] struct Ooopsie(u8); #[repr(packed)] struct Rgb(T, T, T); // size_of::>() is 48 ```

size_of and align_of are const, so maybe it would be possible to hack some static assert that checks for padding? (like this)

[ripytide]

I was thinking we just make another unsafe marker trait and then use that as a bound when implementing Pod:

unsafe trait NotPadded { }

And then implement it on all the core types. Then users could implement it on their own types too but they would have to make the unsafe trait impl which shifts responsibility onto them.

Really this is the sort of trait I'd hope to find in the bytemuck crate but alas it seems they don't have it.

[kornelski]

Pod implies lack of padding already. So actually my previous example was wrong, because u8 with align >1 is not allowed to have Pod implemented.

In that case Pod for Rgb<T: Pod> is fine. It's only Rgba<T: Pod, A: Pod> that's unsound, because the two types could have different alignment, without having padding themselves. It requires align_of::<T> >= align_of::<A>()

[ripytide]

Ah okay, thanks for your advice, I very rarely get this low level in rust/programming in general so I don't really know what I'm doing :smile:

In this case should we still impl<T: Pod, A: Pod> Pod for Rgba<T, A> with an const assertion of align_of::<T> >= align_of::<A>(), is that enough to guarantee soundness?

The impls for Rgb and non-alpha types can then stay the same as they were in v0.8?

[ripytide]

Wait nevermind I don't think it's possible to add a const assert statement into a trait impl. I'll just implement Pod for Rgba<T> for now then, we can always expand the trait impl later if we figure out a way to make it work for heterogeneous pixels.

[kornelski]

It's possible to fail this at compile time (usize can't be negative):

const BREAK: usize = std::mem::align_of::<T>() - std::mem::align_of::<A>();

but unfortunately this is evaluated lazily. fn new() { Self::BREAK; } works, but constructing via literal can still cause unsoundness.

I couldn't find a way to force this check at compile time, so this will have to be limited to uniform types.