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

Add array transpose From implementations: [MaybeUninit<T>; N] <-> MaybeUninit<[T; N]> #110

Closed
SUPERCILEX opened this issue Sep 19, 2022 · 3 comments · Fixed by rust-lang/rust#102023
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@SUPERCILEX
Copy link

Problem statement

The primary use case for MaybeUninit is to initialize arrays of stuff (typically u8s). This is currently poorly supported, requiring the user to enable unstable features (e.g. uninit_array) or immediately mark an array as assume_init which is very confusing to beginners.

Motivation, use-cases

let data = [MaybeUninit::<u8>::uninit(); 2048]; // This could instead be MaybeUninit::<[u8; 2048]>::uninit(), but now you need to use raw pointers to write the data
for ... {
  data.write(...);
}
let data: [u8; 2048] = UH_OH;

UH_OH can be replaced with a transmute (Requires unsafe. Also is that actually safe? It is, but no one wants to spend hours digging around to figure that out.) or with array_assume_init (Requires nightly).

Having a From implementation between [MaybeUninit<T>; N] and MaybeUninit<[T; N]> makes it easy to move between filling an uninitialized array and then marking it as initialized.

Solution sketches

See rust-lang/rust#102023

Links and related work

rust-lang/rust#88837

Other proposals have suggested implementing Index{,Mut}. While I think that's probably a slightly nicer API in its limited scope, users are very quickly going to be sad when they see that their favorite MaybeUninit method isn't available on [MaybeUninit<T>; N]. Eventually that will lead to API duplication as [MaybeUninit<T>; N] methods are added to mirror the MaybeUninit ones. [MaybeUninit<T>; N] <-> MaybeUninit<[T; N]> conversion avoids this entirely.

@SUPERCILEX SUPERCILEX added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Sep 19, 2022
@thomcc
Copy link
Member

thomcc commented Sep 19, 2022

I think this is a good idea. The other reason this is superior to MaybeUninit::uninit().assume_uninit() is that this doesn't require unsafe.

@thomcc
Copy link
Member

thomcc commented Sep 22, 2022

On further though, I wonder if just a transpose() function is a clearer way to spell this than having it be via From. It seems a decent bit clearer about the operation that's being performed.

@SUPERCILEX
Copy link
Author

On further though, I wonder if just a transpose() function is a clearer way to spell this than having it be via From. It seems a decent bit clearer about the operation that's being performed.

Been going back and forth on this and I think you're right. Also after seeing rust-lang/rust#48711, I'm now properly scared: if we later decide the From impls are bad we're screwed, whereas adding transpose methods doesn't prevent us from adding From impls in the future and will give the impls time to sit.

@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants