Lokathor/bytemuck

Blanket impls for PhantomData require parameter to be Zeroable/Pod

kiljacken opened this issue · 2 comments

Hi!

I was hoping someone could clarify something, as I don't understand why the following limit is neccesary.

The blanket impls of Zeroable and Pod for PhantomData<T> require T to be Zeroable/Pod:

unsafe impl<T: Zeroable> Zeroable for PhantomData<T> {}
unsafe impl<T: Pod> Pod for PhantomData<T> {}

With PhantomData being a ZST, could it not just be Zeroable and Pod no matter the type of T? Or am I missing something critical?

Context

I'm trying to represent integers like the zerocopy::byteorder module, but with bytemuck. Something like this:

#[derive(Zeroable, Copy, Clone, Eq, PartialEq, Hash)]
#[repr(transparent)]
pub struct U16<O>([u8; 2], PhantomData<O>);

Where O is for all uses going to be a byteorder::ByteOrder. This gives an error when using the Zeroable derive:

error[E0277]: the trait bound `O: Zeroable` is not satisfied
   --> crates/game-files/src/bytemuck_ext.rs:341:1
    |
341 | #[repr(transparent)]
    | ^ the trait `Zeroable` is not implemented for `O`
    |
    = note: required because of the requirements on the impl of `Zeroable` for `PhantomData<O>`
note: required by a bound in `bytemuck_ext::_::{closure#0}::check::assert_impl`
   --> crates/game-files/src/bytemuck_ext.rs:340:10
    |
340 | #[derive(Zeroable, Copy, Clone, Eq, PartialEq, Hash)]
    |          ^^^^^^^^ required by this bound in `bytemuck_ext::_::{closure#0}::check::assert_impl`
341 | #[repr(transparent)]
    | - required by a bound in this
    = note: this error originates in the derive macro `Zeroable` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider restricting type parameter `O`
    |
342 | pub struct U16<O: bytemuck::Zeroable>([u8; 2], PhantomData<O>);
    |                 ++++++++++++++++++++

For more information about this error, try `rustc --explain E0277`.
error: could not compile `game-files` due to previous error

Haven't gotten the Pod derive to work either seemingly due to #78, but fixing that looks like a lot more effort.
I can manually impl both for the types in question, as I'm pretty sure it's sound in my case, but maybe I missed something.

In general, PhantomData<T> is saying "pretend I'm holding a T". So, if you want the rest of the type system to pretend that you're holding a T, then bytemuck will play along with that fiction.

As you noticed, anyone can bypass the derive logic if they feel like they know what they're doing. Because of this, the provided impls and the derive logic are deliberately written to lean towards not giving an impl if something seems suspect or unknown.

With PhantomData, since the actual value is 0-sized then it's technically always fine. However, if you were holding PhantomData of a reference type instead of just an endian-marker type, that would be very suspicious to be transmuting around.

Right, that makes sense. Wouldn't want to be mucking lifetimes around willy nilly. Thanks for the prompt reply!

I better go slap + 'static on some impls