Associated Err type for the Alloc trait

[lachlansneff]

10 got a little too confusing, so I think it's good to separate this sub-issue into its own issue.

Currently, the Alloc trait looks like this:

pub unsafe trait Alloc {
    unsafe fn alloc(&mut self, layout: Layout) -> Result<NonNull<u8>, AllocErr>;
    unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout);

    // ...
}

I propose that we add an associated type called Err. All methods that currently return Result<_, AllocErr> would return Result<_, Self::Err>.

pub unsafe trait Alloc {
    type Err: Debug;

    unsafe fn alloc(&mut self, layout: Layout) -> Result<NonNull<u8>, Self::Err>;
    unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout);

    // ...
}

Then, the std::alloc module would contain an AbortAlloc (or something named similarly).

#[cold]
fn alloc_abort<E: Debug>(err: E, layout: Layout) -> ! {
    eprintln!("Allocator error: {:?} with layout: {:?}", err, layout);
    std::process::abort()
}

pub struct AbortAlloc<A: Alloc>(A);

unsafe impl<A: Alloc> Alloc for AbortAlloc<A> {
    type Err = !;

    unsafe fn alloc(&mut self, layout: Layout) -> Result<NonNull<u8>, !> {
        self.0.alloc(layout).map_err(|e| alloc_abort(e, layout))
    }
    unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout) {
        self.0.dealloc(ptr, layout)
    }

    // The rest of the methods would simply fall through in the same way.
    // ...
}

impl<A: Alloc> From<A> for AbortAlloc<A> {
    fn from(allocator: A) -> Self {
        AbortAlloc(allocator)
    }
}

Then, types like Vec could have their normal methods that interact with the allocator only be implemented when the Alloc::Err type is !. Otherwise, the try_* methods would be available which would propagate the error upwards.

Additionally, the new_in/etc methods would take an Into<A> parameter so that the user experience of creating an infallible collection wouldn't require the user to explicitly wrap their allocator in an AbortAlloc.

[SimonSapin]

Then, types like Vec could have their normal methods that interact with the allocator only be implemented when the Alloc::Err type is !.

Disagree. All methods of Vec should be available regardless of the error type, as discussed in https://github.com/rust-lang/wg-allocators/issues/10#issuecomment-489698504.

[Ericson2314]

I think the borrow-as-other-allocator type is much better, because it make it much easier for code vigilantly handling all allocation failures to make sure it doesn't accidentally panic by mistake.

If I'm writing a kernel module, I want to still use libraries and be confident after some quick grepping that none of them use AbortAlloc.

[Ericson2314]

I've rebased my old PR https://github.com/rust-lang/rust/pull/60703

[lachlansneff]

@SimonSapin I disagree, handle_alloc_error would become redundant, it doesn't make sense to have multiple places where allocation errors can be handled.

[TimDiekmann]

I don't think a new type has to be introduced for this:

pub trait InfallibleAllocErr: fmt::Display {}
impl InfallibleAllocErr for ! {}
impl InfallibleAllocErr for std::convert::Infallible {}

impl<T, A: Alloc> Box<T, A>
where
    A::Error: InfallibleAllocErr,
{
    pub fn new_in(x: T, mut a: A) -> Self {
        match Self::try_new_in(x, a) {
            Ok(b) => b,
            Err(e) => panic!("Infallible allocation failed: {}", e),
        }
    }
}

Edit: Once the never_type is stabilized, the Bound Alloc<Error=!> could be used instead of an extra trait.

[TimDiekmann]

To get back to the OP, we should decide, if we want to experiment with an associated error type at all. AllocRef would then looks like this:

trait AllocRef {
    type Error;

    ...
}

	add an associated error	postpone	stick with AllocErr
@Amanieu			:-1:
@Ericson2314	:+1:
@glandium
@gnzlbg
@Lokathor
@scottjmaddox		:confused:
@TimDiekmann	:+1:
@Wodann	:+1:

Please vote with

• :+1: add an associated error
• :confused: postpone
• :-1: stick with AllocErr

[scottjmaddox]

I feel like the pro's and con's of each approach have not been discussed, at least on this issue, and it's too soon to make a decision. Is there another issue with discussion comparing Associated Type vs separate Trait? If so, could someone link a summary?

I'm unclear on if any allocators are actually infallible. I know, most Linux OS's are configured to kill processes rather than fail malloc, but do the allocators themselves make any guarantee's about being infallible? If not, then there's no need to even handle that case.

[TimDiekmann]

I'd like to remind, that voting this into nightly is by no mean a final decision. It's just for publishing the change to the rest of the rust community to try this out :slightly_smiling_face:

[Ericson2314]

I guess I had pushed "postpone" by mistake!

[Amanieu]

I'm happy to push this into nightly as an experiment, but I would like this decision to be revisited before stabilizing the AllocRef trait. In particular, I expect that almost all implementations of this trait will simply just use AllocErr. The only use case for another error type that I have seen mentioned is !, but that brings its own set of issues and seems pretty controversial.

[Ericson2314]

@Amanieu I would agree with all that; butdo also note that after rounds of discussion with @TimDiekmann and @gnzlbg, they've improved the ! story a lot since I was first harping on it :).

[Amanieu]

There's a lot of discussion to go through, a summary with the latest ideas on using ! would be nice.

[TimDiekmann]

Based on the latest discussions, I think an allocator should never return !. True infallibility isn't possible, and if an allocation in a collection aborts or returns an error on failure must not be decided in the allocator but in the collection API.

[TimDiekmann]

I'll push the associated error upstream, but we have to listen carefully to the community, if this is useful or not.

[SimonSapin]

Based on the latest discussions, I think an allocator should never return !

I agree. But that seems to be the only use case for an associated error type that people are seriously discussing. In theory an inhabited non-zero-size error type could be used by an allocator to return more details about what when wrong exactly, but I haven’t heard of anyone actually wanting to do this.

So I’m a bit confused that you seem to both be in favor of having the associated type and think that its only use case is not a good idea.

[TimDiekmann]

an inhabited non-zero-size error type could be used by an allocator to return more details about what when wrong exactly, but I haven’t heard of anyone actually wanting to do this.

I wonder if no one has ever wanted something like this because they didn't know that it was theoretically possible, or if no one has ever seen a use-case. Especially because most people only use the collection API which never returns a Result on stable.

One use case could be a tracing allocator, which prints every action (success + failures) with all possible information. Another application could be debugging: With a small switch you can read additional debugging information:

#[cfg(debug_assertions)]
type Allocator = MyAllocWithCustomError;
#[cfg(not(debug_assertions))]
type Allocator = MyAlloc;

I don't have an example currently, but I can also imagine an allocator, which uses FFI and the binding has a dedicating error logging API similar to errno. The allocator could read those information when failing.

[SimonSapin]

It’s a fair point that no-one has done this with the current std::alloc::AllocRef because they can’t. But still:

One use case could be Another application could be I don't have an example currently, but I can also imagine

These are all theoretical, so until someone comes with something more concrete (such as prior art in another language) I feel this is not a good way to spend complexity budget.

[Ericson2314]

@SimonSapin

So I’m a bit confused that you seem to both be in favor of having the associated type and think that its only use case is not a good idea.

I am pretty sure @TimDiekmann means that he does want try_ methods on collections that return an associated error type, but that doesn't require an associated error type on alocators, which is true.

[SimonSapin]

I don’t understand what that means. Associated types are always associated to some trait. What trait would be relevant to allocation in containers if not AllocRef?

[Lokathor]

I think like, to use a bit of Rust pseudo-code:

try_push(&mut Vec<T, A: AllocRef>, t: T) -> Result<(), (A::Err, T)>

[SimonSapin]

… which implies the existence of type Err; inside trait AllocRef. So I don’t understand how it

doesn't require an associated error type on alocators

[TimDiekmann]

Without an associated error type, it would just return Result<(), (AllocErr, T)> or Result<(), AllocErr>

[Amanieu]

If the consensus is that ! is not suitable as an allocator error type then I agree with @SimonSapin: that was the only use case presented for this feature, so I don't think we should support Err as an associated type.

[TimDiekmann]

So what is the consensus here? Close this proposal?

[Amanieu]

I would say close it for now. If a good use case for custom errors comes up while AllocRef is available on nightly then we might revisit this decision.

[Ericson2314]

I opened https://github.com/rust-lang/wg-allocators/issues/39 for the try_ methods without this. If something is unclear about the "policy param" hopefully @TimDiekmann @gnzlbg or I can clarify.

I would really like this to do this as soon as the unstable type params are ready, so I'd appreciate if we can vote on this issue too.

[Ericson2314]

FWIW, Besides Err=! and custom diagnostics, I like the type parameter simply because it helps me make sure my code is correct. There are so-called "wadler free theorems" that one gets from parametericty that help ensure e.g. the alloc error a function return is not some unrelated one that happens to be in scope. But I doubt erring on the side of more type parameters like this is a popular thing in the Rust community, sigh.

[scottjmaddox]

@Ericson2314 I'm afraid I don't follow. If you substituted a different AllocError type that happened to be in scope (that for some strange reason was named AllocError), the compiler would catch it as incompatible with the official AllocError. I doubt this is what you're talking about, but I can't figure out how else to interpret your comment. Would you mind expanding?

[Ericson2314]

@scottjmaddox the idea is whenever one writes code which doesn't fix the A::Err, that type parameter is abstract and won't unify with any concrete type. It's as if it is its own struct MySecretErrorType defined just in the one function with the parameter. This can be used to help write correct code.

[AljoschaMeyer]

First off, apologies if dropping into a closed issue of a working group's issue tracker out of nowhere is not appropriate.

I'd like to make a case for an error type I didn't see in this issue yet: It allows to express that an implementation of an allocation-error-aware trait is infallible because it never allocates in the first place. Consider a trait like the following:

trait AllocatorAwareClone<A: Alloc> {
    fn clone(&self, alloc: A) -> Result<Self, A::Error>;
}

Now suppose there is a struct NoAlloc<A> which implements Alloc<Error = !> by wrapping an allocator A, delegating to deallocate and implementing allocate by panicking. Then any code cloning instances of such types that do not require heap allocations for cloning can explicitly mark that no heap allocations take place and thus no allocation error can happen by calling foo.clone(NoAlloc<A>).