rust-lang / wg-allocators

Home of the Allocators working group: Paving a path for a standard set of allocator traits to be used in collections!
http://bit.ly/hello-wg-allocators
205 stars 9 forks source link

Return `MaybeUninit<u8>` rather than `u8` #66

Closed TimDiekmann closed 3 years ago

TimDiekmann commented 3 years ago

We should considor to return MaybeUninit at allocating methods. While it's unsafe to dereference pointers and the caller has to ensure the validity of the pointer anyway, it's an hint, that the memory may be uninitialized. I don't expect much effect to the end user, as in most cases, the returned pointer will be casted anyway.

Lokathor commented 3 years ago

If alloc_zeroed is still a separate method (i forget) then it should not return MaybeUninit of course.

TimDiekmann commented 3 years ago

With rust-lang/rust#74850 it will be separate again.

Amanieu commented 3 years ago

I don't particularly like this idea. The current plan is to return a NonNull<[u8]> which is a raw pointer that is unsafe to dereference. Adding MaybeUninit doesn't add any safety and will makes things like copying bytes via ptr::copy require additional casts.

TimDiekmann commented 3 years ago

I don't think things like copying via ptr::copy is the main issue here as I expect copying byteswise u8 is a niche. I guess in most cases you either have to cast it anyway or MaybeUninit<u8> fits better as it has no representation (unlike u8, which is still an unsigned number). What I really don't like is the horrible long signature. Boiled down this is *mut [u8] (9 characters), but it's written as NonNull<[MaybeUninit<u8>]> (26 characters). It may be worth discussing a Byte type, which allows an arbitrary bit pattern with uninitialized content.

zakarumych commented 3 years ago
pub type Byte = MaybeUninit<u8>;
pub type Bytes = [Byte];
TimDiekmann commented 3 years ago

I don't particularly like this idea. The current plan is to return a NonNull<[u8]> which is a raw pointer that is unsafe to dereference. Adding MaybeUninit doesn't add any safety and will makes things like copying bytes via ptr::copy require additional casts.

After playing a bit around with AllocRef returning NonNull<[u8]>, it propose either to return NonNull<[MaybeUninit<u8>]> or add a method to NonNull<[T]> return &mut [MaybeUninit<T>]. As your argument makes sense and the former proposal adds nothing to AllocRef, I'll close this (and #68).

@RalfJung Is there a plan to add something like NonNull<[T]>::as_ref_uninit() and NonNull<[T]>::as_mut_uninit() returning &(mut) [MaybeUninit<T>]? I could imaging adding those behind #![feature(non_null_as_uninit_slice)].

RalfJung commented 3 years ago

Is there a plan to add something like NonNull<[T]>::as_ref_uninit() and NonNull<[T]>::as_mut_uninit() returning &(mut) [MaybeUninit]?

I don't know of any such plans, but these sound like useful methods. Basically they would be safe to call whenever the pointer is dereferencable and the aliasing guarantees are met, but there are no requirements of whether the pointee is initialized or not.