Tracking issue for alloc_layout_extra

[Amanieu]

View all comments

Feature gate: #![feature(alloc_layout_extra)]

This issue tracks additional methods on Layout which allow layouts to be composed to build complex layouts.

Public API

pub const fn padding_needed_for(&self, align: usize) -> usize;
pub const fn repeat(&self, n: usize) -> Result<(Layout, usize), LayoutErr>;
pub const fn repeat_packed(&self, n: usize) -> Result<Layout, LayoutErr>;
pub const fn extend_packed(&self, next: Layout) -> Result<Layout, LayoutErr>;
pub const fn dangling(&self) -> NonNull<u8>;

The main use case is to construct complex allocation layouts for use with the stable global allocator API. For example:

• std::collections::HashMap
• hashbrown
• Not exactly an example of use, but it would be very useful in crossbeam-skiplist

Unresolved Questions

• One concern is that not many of these methods have been extensively used in practice. In the examples given above, only extend, array and align_to are used, and I expect that these will be the most used in practice. padding_needed_for is used in the implementation of Rc::from_raw and Arc::from_raw, but in theory could be superseded by the offset returned by extend.
• Related to the above, repeat_packed and extend_packed have no examples or doctests.
• Should repeat include trailing padding on the last element?
• Should padding_needed_for take a T instead of a usize?

[alexcrichton]

After reading over these again, we may want to either include (Layout, usize) in the return value of array or remove the usize return value from repeat, they seem like similar use cases to warrant having similar return values.

[Amanieu]

In the case of array<T> the offset is already available as size_of::<T>(). So it doesn't make sense to add a usize to the return value. I would rather remove the usize from repeat.

[alexcrichton]

Why isn't the same argument true for repeat? Isn't the size available via layout.size()?

[Amanieu]

Because in a Layout the size does not necessarily have to be a multiple of the alignment. But in a Rust type this is always the case.

[RalfJung]

See #55747 for an issue caused by misuse of the current API. Would be great if we could do something to avoid such mistakes.

[Amanieu]

The fundamental question here is: do we want to allow layouts where the size is not a multiple of the alignment? And if we do then we will want to add a convenience method to round up a Layout's size to its alignment.

[RalfJung]

Also if we do we should at least add a fat warning in the extend docs that likely one wants to finish this off by doing pad_to_align().

And maybe the default allocator should assert! that the size it is allocating is divisible by the alignment?

[RalfJung]

FWIW, we actually have explicit tests that allocate with size 8, alignment 16 -- i.e., with a size that is not a multiple of the alignment: heap::alloc_system_overaligned_request and heap::std_heap_overaligned_request.

[glandium]

I stumbled upon padding_needed_for and pad_to_align, and it seems they are being too cautious.

That is, per https://doc.rust-lang.org/nightly/src/core/alloc.rs.html#95, layout.size() is always <= usize::MAX - (layout.align() - 1).

Which means:

• The maximum value layout.size() can have is already aligned for layout.align() (layout.align() being a power of two, usize::MAX - (layout.align() - 1) is a multiple of layout.align())
• Incidentally, any value smaller than that maximum value will align at most to that maximum value.

IOW, pad_to_align can not return Err(LayoutErr), except for the layout not respecting its invariants, but we shouldn't care about that.

So it seems to me we can drop the Result and make the method return a Layout directly.