`MaybeUninit` leads to UB

[jhpratt]

`MaybeUninit` leads to UB

[neachdainn]

...changing the public API from what it is in std is not an option.

Why not? If you do the newtype method I mentioned in the previous comments, it will change how the type is called but it does so in a way that is compatible across Rust versions. I would assume that someone using standback::MaybeUninit would (like to) be aware that they are playing with UB and that their code could be unsound depending on which compiler version the user is using. In the case of <1.36, the unsafe assertion is actually useful - you are asserting that no UB happens. In the case of >=1.36, it is unnecessary but won't prevent the crate from compiling.

In my opinion, hiding UB behind a safe interface is bad form regardless of the purpose of the crate. Particularly when I can see someone just dropping this and the prelude into their crate "just in case" and then not knowing that they have UB in their safe Rust.

---

EDIT:

Yeah, this issue was brought up on Reddit as well.

For most cases, behavior is accurate, thankfully.

You don't create a safe interface for something that is "usually" sound. You should create an unsafe interface the moment it could ever be unsound. In the code below, it is impossible to tell if undefined behavior happens or not. This goes against the entire goal of (safe) Rust. If that is not enough to convince you to not put unsound things behind a safe interface, I will stop pestering you.

use standback::mem::MaybeUninit;

fn main() {
   // Maybe undefined behavior, maybe not. Maybe it will run as expected,
   // maybe it will reformat the user's hard drive. The person using it has no idea. 
    let foo: MaybeUninit<bool> = MaybeUninit::uninit();
}

[neachdainn]

Using the following as a minimum example:

use core::mem::{self, ManuallyDrop};

#[repr(transparent)]
#[derive(Copy)]
pub struct MaybeUninit<T> {
    value: ManuallyDrop<T>,
}

impl<T: Copy> Clone for MaybeUninit<T> {
    #[inline(always)]
    fn clone(&self) -> Self {
        *self
    }
}

impl<T> MaybeUninit<T> {
#[inline(always)]
    pub fn uninit() -> MaybeUninit<T> {
        MaybeUninit {
            value: unsafe { mem::uninitialized() },
        }
    }
}

#[test]
fn main() {
    let t: MaybeUninit<bool> = MaybeUninit::uninit();
}

Miri has the following to say:

running 1 test
error: Miri evaluation error: type validation failed: encountered uninitialized bytes, but expected something less or equal to 1
   --> /Users/nate/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libcore/mem/mod.rs:532:5
    |
532 |     intrinsics::uninit()
    |     ^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected something less or equal to 1
    |
note: inside call to `std::mem::uninitialized::<std::mem::ManuallyDrop<bool>>` at src/main.rs:20:29
   --> src/main.rs:20:29
    |
20  |             value: unsafe { mem::uninitialized() },
    |                             ^^^^^^^^^^^^^^^^^^^^
note: inside call to `MaybeUninit::<bool>::uninit` at src/main.rs:27:32
   --> src/main.rs:27:32
    |
27  |     let t: MaybeUninit<bool> = MaybeUninit::uninit();
    |                                ^^^^^^^^^^^^^^^^^^^^^

I tried to create an implementation that doesn't cause UB but, unfortunately, untagged unions with non-Copy types is not allowed in 1.31 and I have not been able to come up with something that would work.

In my opinion, I would not hide something like this inside of a safe interface. The best approach might be to marke MaybeUninit::uninit and MaybeUnint::zeroed as unsafe functions and, for versions newer than 1.36, create a newtype that wraps the standard library's implementation of MaybeUninit to provide an identical interface.

[jhpratt]

Yeah, this issue was brought up on Reddit as well. As you noted, it's not possible to create an untagged union with non-Copy types in older compilers, do I wasn't able to copy the code directly from std.

I will document this behavior when I get a chance, but changing the public API from what it is in std is not an option. This is meant to be a drop-in replacement with minimal performance, semantic, or other differences.

[jhpratt]

As I said, changing the public API (beyond const fn) is not an option. Doing so would defeat the purpose of this crate.

I am going to look into whether leaving the Copy requirement in place would make sense. If so, I'll likely go with that.