Lokathor/bytemuck

Zeroable/Pod for Option<T> and orphan rules

TheEdward162 opened this issue · 4 comments

The orphan rules forbid me from implementing Zeroable and Pod for Option<T> (my T is #[repr(transparent)] over NonZeroU32).

#[repr(transparent)]
#[derive(Clone, Copy)]
#[derive(TransparentWrapper)]
struct Handle(NonZeroU32);

// not allowed due to orphan rules
unsafe impl Zeroable for Option<Handle> {}
unsafe impl Pod for Option<Handle> {}

Now if I only needed this, I could work around it using TransparentWrapper. Unfortunately, there are also arrays involved 🔥. There is no reasonable way to peel arrays (#66, yes, but there are also MSRV constraints).

// a workaround attempt
#[repr(transparent)]
#[derive(Clone, Copy)]
struct HandleOption(Option<Handle>);
unsafe impl Zeroable for HandleOption {}
unsafe impl Pod for HandleOption {}
unsafe impl TransparentWrapper<Option<Handle>> for HandleOption {}

// this is fine
let a: u32 = 1;
let b: Option<NonZeroU32> = bytemuck::cast(a);
let c: HandleOption = bytemuck::cast(a);
let d: Option<Handle> = HandleOption::peel(c);

// unfortunately, this would need the possibility of peeling arrays
let a: [u32; 1] = [1];
let b: [Option<NonZeroU32>; 1] = bytemuck::cast(a);
let c: [HandleOption; 1] = bytemuck::cast(a);
let d: [Option<Handle>; 1] = HandleOption::peel_array(a);

So that convinces me that this needs to be solved in this crate. My idea here is to expose two traits:

// zeroable.rs
pub unsafe trait IsZeroableInOption: Sized {}
unsafe impl<Z: IsZeroableInOption> Zeroable for Option<Z> {}

unsafe impl IsZeroableInOption for NonZeroI8 {} 
unsafe impl IsZeroableInOption for NonZeroI16 {}
unsafe impl IsZeroableInOption for NonZeroI32 {}
unsafe impl IsZeroableInOption for NonZeroI64 {}
unsafe impl IsZeroableInOption for NonZeroI128 {}
unsafe impl IsZeroableInOption for NonZeroIsize {}
unsafe impl IsZeroableInOption for NonZeroU8 {}
unsafe impl IsZeroableInOption for NonZeroU16 {}
unsafe impl IsZeroableInOption for NonZeroU32 {}
unsafe impl IsZeroableInOption for NonZeroU64 {}
unsafe impl IsZeroableInOption for NonZeroU128 {}
unsafe impl IsZeroableInOption for NonZeroUsize {}

unsafe impl<T> IsZeroableInOption for NonNull<T> {}

// pod.rs
pub unsafe trait IsPodInOption: IsZeroableInOption + Copy + 'static {}
unsafe impl<P: IsPodInOption> Pod for Option<P> {}

unsafe impl IsPodInOption for NonZeroI8 {}
unsafe impl IsPodInOption for NonZeroI16 {}
unsafe impl IsPodInOption for NonZeroI32 {}
unsafe impl IsPodInOption for NonZeroI64 {}
unsafe impl IsPodInOption for NonZeroI128 {}
unsafe impl IsPodInOption for NonZeroIsize {}
unsafe impl IsPodInOption for NonZeroU8 {}
unsafe impl IsPodInOption for NonZeroU16 {}
unsafe impl IsPodInOption for NonZeroU32 {}
unsafe impl IsPodInOption for NonZeroU64 {}
unsafe impl IsPodInOption for NonZeroU128 {}
unsafe impl IsPodInOption for NonZeroUsize {}

#[cfg(feature = "unsound_ptr_pod_impl")]
unsafe impl<T: 'static> IsPodInOption for NonNull<T> {}

These would be basically mirrors of Zeroable and Pod but for Option<T>. It would then be possible to implement this for custom types:

unsafe impl bytemuck::IsZeroableInOption for Handle {}
unsafe impl bytemuck::IsPodInOption for Handle {}

// let a: [u32; 1] = [1];
let d: [Option<Handle>; 1] = bytemuck::cast(a);

Would you accept such a PR? Do you know of any other way how to do this without using const generics?

I understand the problem, and my sympathy, and I do want to solve the problem, and that approach looks interesting.

but before we go any farther I need to start with the clear statement that transmuting pointers is UB in current rust with current LLVM.

It might not be in some future rust/llvm system that's more consistent (it is unknown how things will turn out), but currently it's a source of UB.

But for the actual integer types this looks really cool.

Yes, I don't really have any interest in transmuting pointers, I simply included all the types for which when wrapped in an Option Zeroable and Pod are implemented. I'm mainly interested in the case of #[repr(transparent)] struct Handle(NonZeroU32) and transmuting between u32 and Option<Handle>

I would support this. I'm sure there are types aside from the NonZero* family that would benefit from this.

released in 1.10