Clarify guarantees for Box allocation

[jethrogb]

This basically says Box does the obvious things for its allocations.

See also: https://users.rust-lang.org/t/alloc-crate-guarantees/24981

This may require a T-libs FCP? Not sure.

r? @sfackler

[sfackler]

@rfcbot fcp merge

We'll probably want to be a bit more specific about what the specified behavior is, but I do think that we want to guarantee this, in similar ways to how we already guarantee that we'll never e.g. do small string optimizations in String.

[rfcbot]

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Kimundi
• @SimonSapin
• @alexcrichton
• @dtolnay
• @sfackler
• @withoutboats

Concerns:

• global resolved by Clarify guarantees for Box allocation #58183 (comment)

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[SimonSapin]

@rfcbot concern global

The principle sounds good, but the diff is incorrect. Box uses the global allocator, which (now) happens to default to System but can be overridden by #[global_allocator].

[SimonSapin]

Maybe this should mention Layout::new<T>() specifically?

[withoutboats]

I think the language around conversion with pointers needs to be worded more precisely also.

[jethrogb]

In the changes I just pushed I have elaborated on the conversion.

@SimonSapin

the diff is incorrect. Box uses the global allocator

thanks for catching that.

@SimonSapin

Maybe this should mention Layout::new<T>() specifically?

I used Layout::for_value because it should hold for DSTs as well.

[jethrogb]

@SimonSapin is your concern resolved?

[cramertj]

Is this all true for zero-sized types?

[sfackler]

Ah yeah, we need to add an exception for ZSTs since those can't interact with global allocators.

[jethrogb]

Added qualifier.

[jethrogb]

Tangentially related, is there a reason std::alloc::Layout::from_size_align(0, 1) returns Ok?

[SimonSapin]

Creating a (possibly) zero-size layout is valid. It could be used as an intermediate value passed to Layout::extend, for example.

[SimonSapin]

Yes, thanks.

@rfcbot resolve global.

One more tweak: what matters is whether the value is zero-size or whether the allocation would be zero-size, rather than the type. This makes a difference for dynamically-sized types: [u8] is not zero-size (it’s not Sized) but its values can be and Vec::<u8>::new().into_boxed_slice() produces a Box<[u8]> that doesn’t allocate.

[SimonSapin]

@rfcbot resolve global

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[jethrogb]

Changed “non-zero-sized types” to “non-zero-sized values”.

[jethrogb]

Actually, should we add some guarantees around zero-sized values as well? Such as

1. it's ok to leak a pointer from Box::into_raw if it's a zero-sized value
2. it's ok to convert any non-null well-aligned pointer (or something stronger like NonNull<T>::dangling()) to a Box<T> if the value is zero-sized

[SimonSapin]

Maybe this should all go into the docs for Box::into_raw and Box::from_raw, rather than in docs for the Box type?

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process,I would like to thank @sfacklerfor their work and everyone else who contributed.

The RFC will be merged soon.

[SimonSapin]

Maybe this should all go into the docs for Box::into_raw and Box::from_raw, rather than in docs for the Box type?

We can still do this but the current diff looks good already.

@bors r+

[bors]

📌 Commit e41e694 has been approved by SimonSapin

[Centril]

@RalfJung Oliver suggested we add a miri test for what's written here to ensure it's fine for CTFE; I have no idea how to do that, but maybe you do?

[RalfJung]

@oli-obk do you mean like this?

[oli-obk]

yes