Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return MaybeUninit<u8> rather than u8 #66

Closed
TimDiekmann opened this issue Aug 4, 2020 · 7 comments
Closed

Return MaybeUninit<u8> rather than u8 #66

TimDiekmann opened this issue Aug 4, 2020 · 7 comments
Labels
A-Alloc Trait Issues regarding the Alloc trait

Comments

@TimDiekmann
Copy link
Member

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.

@TimDiekmann TimDiekmann added the A-Alloc Trait Issues regarding the Alloc trait label Aug 4, 2020
@Lokathor
Copy link

Lokathor commented Aug 4, 2020

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

@TimDiekmann
Copy link
Member Author

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

@Amanieu
Copy link
Member

Amanieu commented Aug 4, 2020

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
Copy link
Member Author

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
Copy link

pub type Byte = MaybeUninit<u8>;
pub type Bytes = [Byte];

@TimDiekmann
Copy link
Member Author

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
Copy link
Member

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.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 18, 2020
…r=RalfJung

Add `as_uninit`-like methods to pointer types and unify documentation of `as_ref` methods

This adds a convenient method to retrieve a `&(mut) [MaybeUninit<T>]` from slice pointers (`*const [T]`, `*mut [T]`, `NonNull<[T]>`). See also rust-lang/wg-allocators#66 (comment).

~I'll add a tracking issue as soon as it's reviewed and CI passed.~
Tracking Issue: rust-lang#75402

r? @RalfJung
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Alloc Trait Issues regarding the Alloc trait
Projects
None yet
Development

No branches or pull requests

5 participants