Remove `Alloc::{alloc,dealloc}_array` and `Alloc::{alloc,dealloc}_one`APIs

[gnzlbg]

We should evaluate removing the Alloc::alloc_array and Alloc::dealloc_array APIs.

The main issue I have with these APIs is that they too easily allow creating zero-sized allocations if either T is a ZST, or the array length is zero. They also feel redundant and confusing for the Alloc trait, which is already quite complicated, e.g., Do I have to allocate / deallocate arrays with these? Can I mix match, e.g., Alloc::alloc_array with Alloc::dealloc and vice-versa? etc..

Ideally, Alloc and GlobalAlloc would "converge" some day, e.g., such that you can just implement Alloc for a #[global_allocator] and that's it, instead of having to implement multiple traits. Having generic methods in Alloc does make that a bit harder due to how #[global_allocator] is implemented internally and the constraints on that.

Layout already has an unstable Layout::array<T>(n: usize) method that can be used instead, and that feels like a much better place to put this functionality anyways. So just removing them, at least for the time being, sounds appealing to me. We could always add these later in a backwards compatible way anyways, but we don't have to have them now.

---

EDIT: as @TimDiekmann mentions all of this applies to alloc_one/dealloc_one as well.

[scottjmaddox]

I like the idea of leaning on Layout::array<T>(n: usize) array for this, and simplifying the Alloc trait. It makes sense to me that as much size calculation as possible should be in Layout, in order to maximize reusability.

[TimDiekmann]

The same applies to alloc_one and dealloc_one I guess?

[gnzlbg]

The same applies to alloc_one and dealloc_one I guess?

Yes, nice catch! EDIT: I've updated the issue to reflect this.

---

We should also consider if we want to, at least for an MVP, and as a "design guideline", ban all generic methods in Alloc, but that should be a different issue (it buys us some flexibility w.r.t. deprecating GlobalAlloc with Alloc in the future, where the current internals of GlobalAlloc are not straightforward to extend to generic methods).

[SimonSapin]

(This is the first I hear about deprecating GlobalAlloc. If there’s desire for that it’s worth discussing separately. But remember that we have to keep stable stuff around and working.)

[TimDiekmann]

We should also consider if we want to, at least for an MVP

What does MVP mean in this context? I guess you don't mean Most Valuable Player.

[TimDiekmann]

I don't see why we should deprecate GlobalAlloc in favor of Alloc. Both has their very own use case and acts differently. GlobalAlloc requires inner mutability while Alloc does not require this. For example it doesn't make sense to implement GlobalAlloc on a stack allocator.

[gnzlbg]

What does MVP mean in this context?

Minimum Viable Project. The smallest incremental library and language change that we could make to deliver the most value and that we could then continue to iterate on. As opposed to an RFC that attempts to cover all imaginable use cases, but due to its size becomes un-reviewable, un-mergeable, and is never shipped.

[SimonSapin]

Looking at the implementation of the methods that would be removed:

https://github.com/rust-lang/rust/blob/1.34.1/src/libcore/alloc.rs#L1035-L1231

They call corresponding Layout constructors and cast pointer types (easy/obvious enough so far) but also handle the cases where layout.size() == 0 (by returning an error). It looks like forgetting to check for zero-size could be a common source of bug. CC #16

[gnzlbg]

It looks like forgetting to check for zero-size could be a common source of bug. CC #16

Note that the docs say: "For zero-sized T, may return either of Ok or Err", so at least currently, whether a zero-sized layout is an error or not would depend on the allocator that Alloc is implemented for. The default implementation of these methods just assumes that an allocator that does not override these will not support zero-sized layouts.

So I don't think that, at least with the current API, we can clearly say that this would be a bug. If we were to ban zero-sized allocations (#16) then I'd suppose we would enforce that at the API boundary by using a NonZeroLayout type or similar to prevent this bug.

Note also that with the current API and implementation, GlobalAlloc cannot be extended to have these methods, so they cannot be overriden for alloc::Global, which means that even if the allocator would support ZSTs, alloc_array for alloc::Global could error, while calling alloc_array on the allocator directly could succeed.

Maybe we could fix this by adding an associated const to GlobalAlloc that specifies whether the allocator supports zero-sized allocations but... this is something that we'd only need to consider if we don't decide to ban zero-sized allocations like #16 proposes.