Deprecate uninit_array

[SUPERCILEX]

Now that #98304 has gone through, uninit_array should be removed since creating a const array is the One True Way of doing things.

Tracking issue: #96097

Closes #66845

Blocked on: rust-lang/libs-team#122 (comment)

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[SUPERCILEX]

r? @RalfJung

[RalfJung]

No objections from my side, but this should go through t-libs.
r? libs

[thomcc]

I don't feel that strongly here, but I'm not sure I'm a fan of this removal. I've used MaybeUninit::uninit_array() many times (quite often when optimizing code in std to avoid zeroing buffers), and think it is a nice function to have.

Even if it doesn't have a performance benefit over [const { MaybeUninit::uninit() }; N], it has considerably less syntactic noise.

[thomcc]

Actually, now that I look carefully the const { ... } block doesn't seem to be required to avoid the issues? I think I had misread the PR, and don't feel that strongly if that's not needed.

[RalfJung]

const { ... } is required when T is not Copy.

[clarfonthey]

My gut feeling is also against this, although I feel like the MaybeUninit methods for arrays could use an upgrade.

What if instead of using a method like this, we just directly implemented Index and IndexMut for MaybeUninit<[T; N]>? That way, the existing methods for other types could be used directly, and we'd still have an ergonomic way to use uninit arrays.

[SUPERCILEX]

What if instead of using a method like this, we just directly implemented Index and IndexMut for MaybeUninit<[T; N]>? That way, the existing methods for other types could be used directly, and we'd still have an ergonomic way to use uninit arrays.

How would this work when you're trying to read initialized memory? Furthermore, I don't see your suggestion as incompatible with this PR: we still want to nuke this method since the language is now capable of expressing uninit_array in safe code.

[clarfonthey]

Index and IndexMut just offset pointers and don't actually do any reads themselves; if you returned references to MaybeUninit<T> and MaybeUninit<[T]>, it wouldn't cause any issues.

In terms of deleting this method, keep in mind that it's not yet doable entirely with safe code, since T needs to be Copy or you need to enable the const expressions feature, which isn't stable yet. Even though you'll be on nightly for this method anyway, I don't think we should remove it until a valid alternative is stable.

[SUPERCILEX]

In terms of deleting this method, keep in mind that it's not yet doable entirely with safe code, since T needs to be Copy or you need to enable the const expressions feature, which isn't stable yet.

That's actually not true, see how to do it in this docs update: 54014b7#diff-5e72af2fe825c2180f437d54adbdbb19bb40a9cc82bc6c53c2d993d72b08ce3fR149

Index and IndexMut just offset pointers and don't actually do any reads themselves

Forgot that those returned references. This was already attempted (#88837), but I can commit to giving it another spin and then we can nuke the last method in this tracking issue.

[clarfonthey]

I would be in favour of doing that, especially if we can make arrays and slices work as expected without any dedicated methods.

In terms of deletion, even though this is unstable, I do think that we should deprecate it for a few weeks before outright removing it, since I and a lot of people have been using this method. This has been done before and feels pretty reasonable in this case.

[SUPERCILEX]

I do think that we should deprecate it for a few weeks before outright removing it, since I and a lot of people have been using this method. This has been done before and feels pretty reasonable in this case.

Seems pretty reasonable, done.

[na-sa-do]

That's actually not true, see how to do it in this docs update: 54014b7#diff-5e72af2fe825c2180f437d54adbdbb19bb40a9cc82bc6c53c2d993d72b08ce3fR149

This technique isn't possible with a generic.

fn foo<T>() -> [MaybeUninit<T>; 4] {
    const UNINIT_T: MaybeUninit<T> = MaybeUninit::uninit(); // E0401
    [UNINIT_T; 4]
}

[SUPERCILEX]

Dang, that's a bummer. Haven't had a chance to dig further, but assuming there's no way around this I still think deprecation + removal should move forward since you need to be on nightly to use this feature anyway.

To address an earlier comment, I'm not seeing the "but you need to switch to another unstable feature" as a valid counterpoint without further evidence. You could say that other feature might not be stabilized and so we'll need to bring this back, but:

• Since it's inline const, I find that highly unlikely.
• Worst case scenario people can use unsafe manually while waiting for uninit_array to be brought back.
• The index impls should make this point moot since people can use an uninitialized array directly.

[RalfJung]

https://github.com/RalfJung/inline-const works even when there are generics.

But anyway, the main point above was:

Even if it doesn't have a performance benefit over [const { MaybeUninit::uninit() }; N], it has considerably less syntactic noise.

[clarfonthey]

This is why I proposed adding Index and IndexMut to arrays of MaybeUninit, since it would still allow initializing the individual parts of the array before using the non-array methods to initialize it.

This still doesn't provide a dedicated operation to "transpose" the uninit and array parts of the type, but it provides the same functionality without having to sacrifice readability.

[SUPERCILEX]

I think a transpose is more valuable than index methods since you can convert back and forth as needed. So is the conclusion that we should do that first before this PR?

[clarfonthey]

I don't think that transposing is a good idea because it makes it unclear which version of the transpose is preferred, and simply giving the option to make the top-level version usable seems preferable. Otherwise, we end up having several different ways of doing things without a clear reason to use one or another, which feels clunky.

[RalfJung]

Is it worth adding a type annotation?

Suggested change

	let mut array = MaybeUninit::uninit().transpose();
	let mut array = MaybeUninit::<[???; N]>::uninit().transpose();

There's a lot of inference going on here making the code quite hard to follow.

[SUPERCILEX]

Hmmm, yeah I was wondering about this too. My reasoning for this being ok was that if the compiler manages to infer the types then I don't need to think about them. I'm guessing this is flawed because if you have no constraints at all, then the compiler will happily give you an i32. An interesting question arises: are integers and floats the only two places where the compiler will ever pull a type out of its 🍑 if there are no constraints? If so, my gut feeling is that we should get rid of that behavior (separate discussion, I know) since not having 🍑 types would let you do what I'm doing now where I say, "well, the compiler did some inferences so whatever the types are they must be correct". Or maybe I'm thinking too hard about this and type annotations would make human consumption easier (though I'm still curious about the above questions).

[RalfJung]

In safe code I tend to follow the same approach -- if the compiler found a type that makes it all work, then that seems right (but I will still sometimes add type annotations to make the code more readable).

In unsafe code though, I don't like trusting the compiler like that. Types are much less meaningful in unsafe code (that's why the code is unsafe to begin with), so IMO type inference should only be relied upon for absolutely obvious cases -- which this one is not.

[SUPERCILEX]

Good point, will do.

[m-ou-se]

Even though you'll be on nightly for this method anyway, I don't think we should remove it until a valid alternative is stable.

I don't think we should try to maintain any sort of stability for nightly features. The point of nightly features is that they're experimental and that we can change or remove them at any time.

I feel like the MaybeUninit methods for arrays could use an upgrade.

Agreed

What if instead of using a method like this, we just directly implemented Index and IndexMut for MaybeUninit<[T; N]>? That way, the existing methods for other types could be used directly, and we'd still have an ergonomic way to use uninit arrays.

This RFC seems relevant: rust-lang/rfcs#3318

[m-ou-se]

We briefly discussed this in the libs-api meeting. We prefer removing the function over deprecating it, since it's unstable.

[m-ou-se]

I settled on using transpose when the types could be inferred since that minimizes human thinking. In the other cases I used const arrays.

The sentiment in the libs-api meeting was that the array version is actually easier to understand than the uninit().transpose() version, despite being a bit more verbose.

[clarfonthey]

I still would like to express my discomfort with transposes as… exposing something more generic for the sake of it, instead of promoting a good API.

Right now we have the major hurdle that there have to be different methods for working with arrays, slices, and values. This doesn't actually solve this problem; in order to work with an array, you're still expected to transpose after the initial uninit, and (presumably) before the final assume_init.

IMHO, we can remove this entirely by simply letting people slice directly into a MaybeUninit to retrieve its components. If we accepted MaybeUninit<[T]> as a valid type and provided Index and IndexMut methods to it, callers can perform writes via all the expected means without having to transpose at all. This is perfectly sound as long as we return MaybeUninit<[T]> and MaybeUninit<T> references, since we never let an init reference "escape" with safe code.

This is the main reason why I promoted deprecating the old method as a compromise: if we're going to offer transposes, it seems worthwhile to see if users will actually prefer it. By removing the alternative, everyone's forced to move back to it instead, which just results in more frustration, IMHO.

The one downside of this, which I will admit is rather large, is the fact that the metadata of the unsized value "escapes" from the MaybeUninit and is actually init at all times. This seems reasonable to me for slices, but it won't make sense for other unsized types. As long as we only allow slices this way, I think it would be fine, but I admit this is the biggest flaw with this approach.

[SUPERCILEX]

Right now we have the major hurdle that there have to be different methods for working with arrays, slices, and values.

Absolutely agree that this sucks, that's what I'm trying to improve.

If we accepted MaybeUninit<[T]> as a valid type

Not sure how that's possible since [T] is unsized, but otherwise yes this would be great. BTW, not sure if you've seen this discussion where I mentioned slices: rust-lang/libs-team#122 (comment)

Also, I still think you'd need transpose APIs because you'll be passed in an &mut [MaybeUninit<u8>] which means you need to transpose to be able to assume_init the whole array, even if MaybeUninit<[T]> is a thing.

[apiraino]

hello, visiting and checking progress. By skimming the comment history I'm unclear on the status. Perhaps some design work from the author (cc @SUPERCILEX) ? Can anyone provide an update? :) thanks!

[SUPERCILEX]

I still think this will go through, but we're now potentially waiting on inline const stabilization: rust-lang/libs-team#122 (comment)

[SUPERCILEX]

Closing as this made progress elsewhere: #96097 (comment)