Make pinned-init usable with 1.82.0+

[bonzini]

This pull request, on top of #23, makes it possible to use pinned_init with stable Rust. In particular:

• new_uninit has been stabilized (and anyway in the end it was just a glorified Box::<MaybeUninit>::new())
• allocator_api is needed to have fallible allocation, but not if you just replace core::alloc::AllocError with core::convert::Infallible
• get_mut_unchecked is not needed, because after the introduction of InPlaceWrite you actually need to check that you're not writing to a shared Arc—thus Arc::get_mut is the right associated function to use.

The main work is to isolate a little bit more the pieces of code that refer to feature(allocator_api).

Successful CI run at bonzini#2.

Diff on top of #23 here.

[y86-dev]

do you mind rebasing this one on top of main? thanks!

[y86-dev]

If I only enable the arc feature (with default-features = false), then it doesn't compile, this should fix it.

[bonzini]

If I only enable the arc feature (with default-features = false), then it doesn't compile, this should fix it.

Thinking more about it, after the introduction of InPlaceWrite you actually need to check that you're not writing to a shared Arc. Thus Arc::get_mut is the right associated function to use; get_mut_unchecked goes away and the need for an arc feature with it.

[y86-dev]

You're correct. But having the function panic is not a good idea, since panics in the kernel are not an option.

In that case, it probably makes more sense to just not implement InPlaceWrite for Arc or come up with a better error type. We'd still need to use get_mut_unchecked in InPlaceInit though.

[bonzini]

it probably makes more sense to just not implement InPlaceWrite for Arc

As you prefer! The choice is:

• keep the code more similar to the kernel, at the cost of a possible panic due to lack of UniqueArc (userspace panics are not as bad, and InPlaceInit will never panic)
• avoid panics, and remove the impl<T> InPlaceWrite for Arc<T>, though this makes future backports a bit harder

We'd still need to use get_mut_unchecked in InPlaceInit though.

It's not strictly necessary. There are cases in which get_mut_unchecked is useful even with refcount > 1, but pinned_init is only concerned with the case of refcount == 1 because it provides a safe API. So using get_mut_unchecked is only a small optimization. Again there are multiple choices:

• use Arc::get_mut(...).unwrap(), the performance cost is probably minimal and we know it can't fail
• use Arc::get_mut_unchecked(...) and disable Arc on stable Rust, preserving the optimization.
• check for it in build.rs, but it seems a bit overengineered

In both cases, just tell me which of the possibilities you prefer and I'll implement it. :)

[y86-dev]

it probably makes more sense to just not implement InPlaceWrite for Arc

As you prefer! The choice is:

• keep the code more similar to the kernel, at the cost of a possible panic due to lack of UniqueArc (userspace panics are not as bad, and InPlaceInit will never panic)

• avoid panics, and remove the impl<T> InPlaceWrite for Arc<T>, though this makes future backports a bit harder

I think we should remove the InPlaceWrite impl for Arc. What do you mean by "this makes future backports a bit harder"?

We'd still need to use get_mut_unchecked in InPlaceInit though.

It's not strictly necessary. There are cases in which get_mut_unchecked is useful even with refcount > 1, but pinned_init is only concerned with the case of refcount == 1 because it provides a safe API. So using get_mut_unchecked is only a small optimization. Again there are multiple choices:

• use Arc::get_mut(...).unwrap(), the performance cost is probably minimal and we know it can't fail

• use Arc::get_mut_unchecked(...) and disable Arc on stable Rust, preserving the optimization.

• check for it in build.rs, but it seems a bit overengineered

Oh I have an idea, we can just use

match Arc::get_mut(...) {
    Ok(...) => ...
    Err(...) => unsafe { hint::unreachable_unchecked() },
}

That way we have the performance and don't rely on an unstable feature.

[bonzini]

What do you mean by "this makes future backports a bit harder"?

I mean that future backports may have to account for the Arc implementation being in <UniqueArc<T> as InPlaceWrite<T, E>> for the kernel and <Arc<T> as InPlaceInit<T, E>> for the userspace crate. No objections though, I will do as you requested over the next few days.

[y86-dev]

some minor things, but otherwise looks pretty good.

[y86-dev]

I sadly can't comment on commit messages, in the first commit 876f228 ("lib: revert InPlaceWrite implementation for Arc<MaybeUninit<T>>") I would prefer if you put Suggested-by: Benno Lossin <[email protected]>, for kernel tags, I use that. Thanks!

Same of course for the next one.

---

1919cf5 ("lib: make "std" independent of allocator_api") also needs its commit message updated, it still references the get_mut_unchecked feature.

[y86-dev]

thanks again for all the help!