fitzgen / bumpalo

A fast bump allocation arena for Rust
https://docs.rs/bumpalo
Apache License 2.0
1.44k stars 112 forks source link

Can't unsize bumpalo's Box #106

Open vorner opened 3 years ago

vorner commented 3 years ago

Hello

It seems the bumpalo's Box doesn't implement CoerceUnsize, like the usual Box does. I'm not sure how one should be able to create upcastable smart pointers, as that's a nightly-only API :-(.

This code does not compile (and I can't figure out any equivalent that does):

use std::fmt::Debug;

use bumpalo::Bump;
use bumpalo::boxed::Box as BumpBox;

#[derive(Debug)]
struct X;

fn get_unsized(arena: &Bump) -> BumpBox<dyn Debug> {
    BumpBox::new_in(X, arena)
}

This fails with mismatched types. Trying into() and similar doesn't help.

I've looked at this example around downcast and I'm pretty sure the example uses the ordinary std::boxed::Box, not the bumpalo's one.

Is there a way to do the upcast I'm not seeing? Should the example be modified to use that?

fitzgen commented 3 years ago

You can perhaps do something like

let x = arena.alloc(x);
let x = x as &mut dyn Debug;
let x = unsafe { BumpBox::from_raw(x) };

Otherwise, you can use the cargo feature to enable Bump implementing the nightly Allocator trait and use the nightly feature to expose custom allocators with std types, and then the original example should work.

vorner commented 3 years ago

I'd say neither is usable in a "production" project that disallows both nightly and unsafe :-(. If I really needed that (instead of preferring not to allocate on the heap), I'd go with creating a separate crate to isolate that unsafe, but that's probably not worth it.

Would it be OK to put such code into a method inside bumpalo? Can I send a PR?

fitzgen commented 3 years ago

Unfortunately it is impossible to make a generic version of that snippet in today's Rust:

  1. A function can't be generic over a trait, rather than over a type, so there is no way to write the signature of a function that returns dyn T where T is a trait.
  2. There is no trait for as conversions, so the second line is impossible to write in a generic context.
vorner commented 3 years ago

Well, the dyn Trait is a type in its own right, so at least the first part is not really true. There also is a conversion for unsizing, but it's unstable :-(. This code demonstrates this:

#![feature(unsize)]

use std::fmt::Debug;
use std::marker::Unsize;

use bumpalo::Bump;
use bumpalo::boxed::Box as BumpBox;

fn box_it<'a, T, U>(arena: &'a Bump, x: T) -> BumpBox<'a, U>
where
    T: Unsize<U>,
    U: ?Sized + 'static,
{
    let x = arena.alloc(x);
    let x = x as &mut _;
    let x = unsafe { BumpBox::from_raw(x) };
    x
}

fn main() {
    let bump = Bump::new();
    let x = box_it::<usize, dyn Debug>(&bump, 42);
}

I wonder if there might be some other conversion trait that would also be usable. But it seems &'a mut T: Into<&'a mut U> is not available here.

I guess there would be a possibility of a macro to hide the unsafety, but that's ugly :-(.

jkelleyrtp commented 3 years ago

Is there any good work around for Box<dyn FnOnce>? This got stabilized for stable rust's Box, but I don't think those changes ever trickled into Bumpalo.

sirgl commented 3 years ago

I think it is all about variance of type. AFAIU, if Box stores not a mutable reference, but raw pointer, such a Box would be covariant and such type coertion would be possible. https://doc.rust-lang.org/nomicon/subtyping.html

poconn commented 1 year ago

A related issue is that even if you manage to construct a Box<dyn FnOnce()>, AFAICT it's not possible to call it because it can't be moved from:

fn f(b: bumpalo::boxed::Box<'_, dyn FnOnce()>) {
    b();
}
cannot move out of dereference of `bumpalo::boxed::Box<'_, dyn FnOnce()>`

and Box::into_inner(b)() doesn't work as it's unsized.

The former works with the stdlib Box, which I think has some special case deref magic?

Otherwise, you can use the cargo feature to enable Bump implementing the nightly Allocator trait and use the nightly feature to expose custom allocators with std types, and then the original example should work.

This doesn't work for my use case as the stdlib box is Send + Sync only if the allocator is.

EDIT: I ended up writing an allocator wrapper with a no-op deallocate():

``` struct BumpRef<'b> { bump: &'b Bump, } unsafe impl Allocator for BumpRef<'_> { fn allocate( &self, layout: std::alloc::Layout, ) -> Result, std::alloc::AllocError> { self.bump.allocate(layout) } unsafe fn deallocate( &self, _ptr: std::ptr::NonNull, _layout: std::alloc::Layout, ) { } } ```

Then used that with the stdlib Box and manually implemented Send for the type containing it. A little ugly but it should be sound as Box doesn't realloc so it avoids the data race on the Bump.