Skip to content

impl Clone for Box<[T]> #25097

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

Closed
bluss opened this issue May 4, 2015 · 5 comments
Closed

impl Clone for Box<[T]> #25097

bluss opened this issue May 4, 2015 · 5 comments

Comments

@bluss
Copy link
Member

bluss commented May 4, 2015

We need to find a way to implement Clone for Box<[T]> where T: Clone.

In user code, "cloning" such a box manually is simple: .to_vec().into_boxed_slice() will do it with a short roundtrip through Vec<T> which should be unproblematic.

In the rust distribution, we can't impl Clone easily since the tools to do so are in libcollections but the Box type lives in liballoc.

@Thiez
Copy link
Contributor

Thiez commented May 4, 2015

You do not need vec for the implementation. Something like the following would work:

struct BoxBuild<T> {
    progress: Option<std::raw::Slice<T>>,
    total: usize,
}

impl<T> Drop for BoxBuild<T> {
    fn drop(&mut self) {
        use std::mem::{size_of, min_align_of};
        unsafe {
            if let Some(slice) = self.progress.take() {
                for n in 0..slice.len {
                    std::ptr::read(slice.data.offset(n as isize));
                }
                std::rt::heap::deallocate(
                    slice.data as *mut u8,
                    size_of::<T>() * self.total,
                    min_align_of::<T>());
            }
        }
    }
}

fn box_clone<T: Clone>(old: &Box<[T]>) -> Box<[T]> {
    use std::mem::{size_of, min_align_of, transmute};
    unsafe {
        // Ignore OOM. #yolo
        let mut alloc = (std::rt::heap::allocate(
                    size_of::<T>() * old.len(),
                    min_align_of::<T>())) as *mut T;
        let mut new = BoxBuild{
            progress: Some(std::raw::Slice {
                data: alloc as *const T,
                len: 0,
            }),
            total: old.len()
        };
        for o in old.iter() {
            std::ptr::write(alloc, o.clone());
            alloc = alloc.offset(1);
            new.progress.as_mut().map(|s|s.len += 1);
        }
        let result = new.progress.take().unwrap();
        transmute(result)
    }
}

Note that because clone() can panic it is required to keep track of the partial result so the correct stuff is dropped.

@bluss
Copy link
Member Author

bluss commented May 5, 2015

Yes, that's nice that it can be carefully solved by essentially extracting the needed parts from what Vec does. But the Box code is otherwise so clean, it doesn't feel like the right way to do it.

@Thiez
Copy link
Contributor

Thiez commented May 5, 2015

You're welcome to suggest an alternative but I doubt it can be done in a manner that is both correct and significantly cleaner :)

@bluss
Copy link
Member Author

bluss commented May 5, 2015

Your fundamental analysis is great: Box<[T]> just isn't an exception safe representation while filling it with elements. It needs to keep track of the fill level, so a representation like Vec or BoxBuild is needed...

@bluss
Copy link
Member Author

bluss commented Jul 24, 2015

Clone is also wanted for Box<str>, but that can be much simpler: There is no panic issue, and it can use a simple memcpy of the data.

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

3 participants