alloc: stabilise Allocator

[nia-e]

View all comments

See the current proposed stabilisation report for up-to-date information on the proposed stable API.

outdated former status

Stabilise a bare-minimum (dyn-incompatible, but could be in the future) Allocator trait, alongside Box::new_in(), Vec::new_in(), the System & Global Allocators, and a blanket impl of Allocator for T: GlobalAlloc. For now, we should take care not to make it possible to instantiate anything other than a Vec or Box with custom allocators; it's Probably Fine, but worth a proper look before we rush in.

The soundness requirements for implementors were tightened to the most restrictive ones we could reasonably want per a conversation with @RalfJung.

This was discussed extensively at the all-hands with an apparent tentative consensus from libs and participating ecosystem stakeholders that the current design can be extended backwards-compatibly to address almost all usecases.

cc @rust-lang/libs @rust-lang/libs-api @rust-lang/opsem

r? libs

[nia-e]

r? @Amanieu

[theemathas]

I believe the safety requirements are not yet correct. See #156544

[nia-e]

cc also @rust-lang/libs-api + @rust-lang/lang

[traviscross]

Do we intend (now or later) to make any ABI guarantees about Box<T, A> where A ≠ Global?

[RalfJung]

I'd be fine with making guarantees for the case where A has trivial ABI. Beyond that -- probably not.

[traviscross]

Should we be firing improper_ctypes_definitions?

[nia-e]

I'm happy having them under that lint

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rust-bors]

☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

[traviscross]

Thanks @nia-e.

As I understand it, on lang, we're signing off on two things:

One, we're agreeing that Box<T, A> will not be fundamental in A. Why does this make sense? We can ask, why is it fundamental in T? The answer is that we think of Box<…> as an ownership mode over a value, and the trait behavior of that value is (fundamentally) determined by T. This doesn't apply to A. Further, we want Box<…> to cover A so that authors can write certain impls that should be universally quantified over A, such as:

impl<A: Allocator> PartialEq<LocalTy> for Box<u8, A> {
  fn eq(&self, _: &LocalTy) -> bool { true }
}

Two, we're agreeing that Box<T, A> will not be noalias and will not carry the corresponding language-level uniqueness requirements when A ≠ Global (except, possibly, when A is some NativeAllocator<…> wrapper we might add in the future). This makes sense because, unlike the global allocator, which is magic, custom allocators are normal Rust code, and certain allocators want to offset from the data pointer to the allocator state during deallocation.

See:

• Make #[fundamental] only apply to the first argument of Box #160162
• Box in custom allocator fails even with -Zmiri-tree-borrows miri#3341
• only set noalias on Box with the global allocator #122018
• Custom allocators vs Box noalias wg-allocators#122
• enforce noalias on custom allocator Box by default miri#5067

@rfcbot reviewed

[theemathas]

Based on conversations elsewhere, I don't think this will be an issue, I don't think this will be a problem. But it's worth mentioning for awareness.

Based on the documentation of Box::into_raw_with_allocator, we are committing to a guarantee that the raw pointer from a Box can be deallocated with the layout of T. This implies that the allocator is stored in the Box struct, not in the heap allocation.

This in turns means that if we ever wanted to implement DispatchFromDyn on boxes with custom allocators, we'd need to modify the compiler so that dynamic dispatch works on receivers whose ABI aren't pointer-like. My understanding is that this is a matter of implementation work, and can be done.

[theemathas]

And now for a concern that I think is actually blocking: Some traits in std are currently implemented on Box<T>, and not a generic Box<T, A>. I think that changing this later is a breaking change, so we'd need to generalize such impls on stable traits now.

(This is also a semver hazard for third-party crates that have blanket impls on Boxes. Generalizing such impls to include boxes with custom allocators is a breaking change.)

[bushrat011899]

And now for a concern that I think is actually blocking: Some traits in std are currently implemented on Box<T>, and not a generic Box<T, A>. I think that changing this later is a breaking change, so we'd need to generalize such impls on stable traits now.

Is it a breaking change? Such implementations are effectively Box<T, Global>, and only the T parameter is fundamental. Or is this in reference to type inference (which is why Default is explicitly only implemented for Global)

[theemathas]

@bushrat011899 The following code currently compiles on nightly, and would break if we were to change the Read for Box impl to work with custom allocators.

#![feature(allocator_api)]

use std::alloc::Global;
use std::io::{self, Read};

struct Thing;
impl Read for Thing {
    fn read(&mut self, _: &mut [u8]) -> Result<usize, io::Error> {
        Ok(0)
    }
}

impl Read for Box<Thing, &'static Global> {
    fn read(&mut self, _: &mut [u8]) -> Result<usize, io::Error> {
        Ok(0)
    }
}

[bushrat011899]

Interesting, that's only an issue because the first type parameter is fundamental? For example, an implementation of Box<Foo, Global> (for a particular concrete Foo) could be expanded to all allocators without being a breaking change if I'm understanding the issue correctly?

[theemathas]

Interesting, that's only an issue because the first type parameter is fundamental?

Yes.

For example, an implementation of Box<Foo, Global> (for a particular concrete Foo) could be expanded to all allocators without being a breaking change if I'm understanding the issue correctly?

I think so, yes.

[nia-e]

I'm happy to expand all appropriate trait impls before landing this. I believe some of them can't † be expanded directly (e.g. Default) since they'd be breaking on stable today, but I'll do a pass over all other downstream-implementable traits and shoot a PR generalising them if possible.

† Doing so would require effectively splitting Box into two types, one with 2 generic params (pointee + allocator), and a type alias to the former with A = Global. This would mean changes to the prelude as well and this was discussed & I will mention it for completeness but I personally wouldn't want to go that direction...

[maxdexh]

† Doing so would require effectively splitting Box into two types, one with 2 generic params (pointee + allocator), and a type alias to the former with A = Global. This would mean changes to the prelude as well and this was discussed & I will mention it for completeness but I personally wouldn't want to go that direction...

Here is some more discussion about that splitting. I think we should do this ^^

[jkarneges]

Not sure if discussed, but moving the allocator into the heap allocation can make sense for Rc/Arc and if that were to happen it might be strange if Box were fat.

[maxdexh]

We have an RFC for that? I think we were just planning to move the allocator to the heap on Arc/Rc without one, since it's all unstable anyway (zulip). We even tightened the Send and Sync impls already for this ^^

[programmerjake]

Not sure if discussed, but moving the allocator into the heap allocation can make sense for Rc/Arc and if that were to happen it might be strange if Box were fat.

Box is guaranteed to be fat since you can construct Box<T, A> from a manually allocated T and that can only work if A is not stored inside that allocation.

Arc/Rc already have extra data in the allocation for the reference counters, so you can't manually allocate/free the T, so std can easily choose to also put A in that allocation without breaking anything.

[nikomatsakis]

@rfcbot reviewed

[tmandry]

The two asks make sense to me:

1. I don't think Box should be fundamental in A for the reasons in alloc: stabilise Allocator #156882 (comment).
2. It's not possible for many allocators to satisfy the noalias requirements (the global one is special, and we may expose that special-ness as an LLVM intrinsic at some point).

Thanks to wg-allocators and @nia-e for driving this forward and providing context.

@rfcbot reviewed

[bstrie]

The stabilization report mentions that the API being stabilized was developed with the input of "interested parties in the ecosystem". I'd be curious to know who those third-parties were, and to hear from them to know if they've had enough time to kick the tires on the API for everyone to be confident in its final shape.

[nia-e]

Reasonable question! We particularly spoke to folks from Rust for Linux, PyO3/Pydantic, and Canonical during the all-hands and shortly after, who all expressed that the proposed API surface is one they can worth with (with further extensions in the future, of course). Significant efforts were also made to keep the stabilised API surface virtually identical to the old one from before the trait refactor landed, so a lot of past feedback (including real-world testing via allocator_api2) was factored in also. The stabilisation report goes over the API asks that we've seen and a large majority of those ended up being answered with a yes (admittedly often a yes-but-later).