Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl From<Box<T>> for Gc<T> where T: ?Sized #106

Open
remexre opened this issue Jul 12, 2024 · 1 comment
Open

impl From<Box<T>> for Gc<T> where T: ?Sized #106

remexre opened this issue Jul 12, 2024 · 1 comment

Comments

@remexre
Copy link

remexre commented Jul 12, 2024

It was mentioned in the README that this doesn't work yet, but I didn't see an issue -- it'd be nice to be able to go Vec<T> -> Gc<[T]>, but this seems like a good first stepping-stone.


I gave the code a look to see how easy or hard this feels; I'm very new to this codebase, but my gut feeling is that this should be possible on nightly:

  • Add an unsafe fn Context::allocate_layout(Layout) -> NonNull<GcBoxInner<()>>, which still constructs the header but leaves the contents uninitialized; safety conditions include something like "this must be initialized before returning from the enclosing Arena::mutate (or similar)"
  • Expose this at pub(crate) in the Mutation
  • Add a From<Box<T>> where T: ?Sized-like method that uses the ptr_metadata feature like so (untested):
impl<'gc, T: Collect + ?Sized + 'gc> Gc<'gc, T> {
    #[inline]
    pub fn from_box(mc: &Mutation<'gc>, boxed: Box<T>) -> Gc<'gc, T> {
        use core::{
            mem::ManuallyDrop,
            ptr::{addr_of_mut, copy_nonoverlapping, metadata, Pointee},
        };

        // Get the layout and metadata of the T.
        let layout = Layout::for_value::<T>(&boxed);
        let meta: <T as Pointee>::Metadata = metadata(&*boxed);

        // Create a Gc<'gc, ManuallyDrop<T>>. This has all sorts of safety obligations attached.
        // In particular, if a GC occurs while the boxed value is still uninitialized, UB will
        // occur during tracing.
        //
        // This really ought to be MaybeUninit instead, but that doesn't work with unsized types.
        let dst: NonNull<GcBoxInner<ManuallyDrop<T>>> = unsafe {
            let ptr: NonNull<GcBoxInner<()>> = mc.allocate_layout(layout);
            NonNull::from_raw_parts(ptr.cast(), meta)
        };

        // Make the Box be ManuallyDrop.
        let src: Box<ManuallyDrop<T>> = unsafe {
            let ptr = Box::into_raw(boxed);
            Box::from_raw(ptr as *mut ManuallyDrop<T>)
        };

        // Copy the value from src to dst, then free src. This won't drop the T, since we casted
        // the Box.
        unsafe {
            copy_nonoverlapping(
                &*src as *const ManuallyDrop<T> as *const u8,
                addr_of_mut!((*dst.as_ptr()).value) as *mut ManuallyDrop<T> as *mut u8,
                layout.size(),
            );
        }
        drop(src);

        // Cast away the ManuallyDrop.
        let ptr: NonNull<GcBoxInner<T>> = NonNull::from_raw_parts(dst.cast(), meta);

        // Return the nicely-wrapped-up pointer.
        Gc {
            ptr,
            _invariant: PhantomData,
        }
    }
}

I haven't looked deeply enough at allocate_layout to know whether unsizing the vtables will be a nightmare or not... The painful but probably sufficient approach, I suppose, would be to store the metadata in the GcBoxInner, along the lines of std::boxed::ThinBox's WithOpaqueHeader type?

@kyren
Copy link
Owner

kyren commented Jul 17, 2024

I haven't looked deeply enough at allocate_layout to know whether unsizing the vtables will be a nightmare or not

This is basically where the complexity is, and why we haven't provided this feature yet. Right now, we require that every allocated type have a statically known size and we use a (manual) VTable to store this size. We create the VTables in a const context and use 'static pointers to them, which you can see here: https://github.com/kyren/gc-arena/blob/master/src/types.rs#L91-L106

We're trying to avoid at all costs growing the header for every Gc allocated object, so our current idea is to have two different header formats, one for statically sized Gc objects and one for dynamically sized ones that includes the allocated size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants