Clarify safety properties of casts between [MaybeUninit<T>] <-> [T].

[briansmith]

Reduce the scope of the unsafe blocks to the unsafe operations.

[newpavlov]

I think let ptr = slice as *mut [MaybeUninit<T>] as *mut [T] would be a bit easier to understand than let ptr: *mut [MaybeUninit<T>] = slice.

[josephlr]

I agree doing it on one line is more reasonable, we also probably don't need the first SAFETY comment, as it confuses things. It is always legal to cast from a *mut [A] to a *mut [B] (where A and B are sized). It's dereferencing where we need to be sure that:

• The pointer is aligned
• The range [ptr,ptr+len) is exclusive and valid for reads/writes
• The bytes are initialized

So something like:

    let ptr = slice as *mut [MaybeUninit<T>] as *mut [T];
    // SAFETY: As `MaybeUninit<T>` is layout compatible with `T`, `ptr` is
    // appropriately aligned and points to the same memory as `slice`. As `ptr`
    // was created from a valid `&mut` reference and the caller ensures every
    // element is initialized, it is safe to dereference `ptr`.
    unsafe { &mut *ptr }

[josephlr]

I agree doing it on one line is more reasonable, we also probably don't need the first SAFETY comment, as it confuses things. It is always legal to cast from a *mut [A] to a *mut [B] (where A and B are sized). It's dereferencing where we need to be sure that:

• The pointer is aligned
• The range [ptr,ptr+len) is exclusive and valid for reads/writes
• The bytes are initialized

So something like:

    let ptr = slice as *mut [MaybeUninit<T>] as *mut [T];
    // SAFETY: As `MaybeUninit<T>` is layout compatible with `T`, `ptr` is
    // appropriately aligned and points to the same memory as `slice`. As `ptr`
    // was created from a valid `&mut` reference and the caller ensures every
    // element is initialized, it is safe to dereference `ptr`.
    unsafe { &mut *ptr }

[josephlr]

Perhaps:

    let ptr = slice as *const [T] as *const [MaybeUninit<T>];
    // SAFETY: As `MaybeUninit<T>` is layout compatible with `T`, `ptr` is
    // appropriately aligned and points to the same memory as `slice`. As `ptr`
    // was created from a valid reference, any `T` is a valid `MaybeUninit<T>`,
    // and the return type is immutable, it is safe to dereference `ptr`.
    unsafe { &*ptr }

[josephlr]

Perhaps we should update this to be:

The caller ensures that the return slice will never be uninitialized.

[josephlr]

Then this implementation can be:

    let ptr = slice as *mut [T] as *mut [MaybeUninit<T>];
    // SAFETY: As `MaybeUninit<T>` is layout compatible with `T`, `ptr` is
    // appropriately aligned and points to the same memory as `slice`. As `ptr`
    // was created from a valid `&mut` reference, any `T` is a valid
    // `MaybeUninit<T>`, and the caller ensures the returned reference will not
    // be uninitialized, it is safe to dereference `ptr`.
    unsafe { &*ptr }

[briansmith]

I updated this with the following changes:

• Use polyfills for core::ptr::{from_ref, from_mut} instead of the unnecessary casts we were using before. My overall goal with this series of PRs is to remove all as casts in favor of safer alternatives, as much as practical. I agree that the implicit coersion, though safer than a cast, is unfortunate because it requires a separate statement. Using from_ref/from_mut alleviates this tension.

• Undo all the changes to the safety comments. I appreciate the suggestions on how to improve the safety comments I tried to write. Ultimately I think less is more here. What really matters regarding the safety of these functions is the validity of the type coersions and casts. "Proper" safety comments just completely obfuscate this and distract the reader from focusing on the correctness of the type conversions.

[josephlr]

Thanks for the fixes @briansmith. Agreed w.r.t. to the shorter SAFETY comments