Remove the Box from the ShapeHandle

[sebcrozet]

Right now, the ShapeHandle is defined as a Arc<Box<Shape<N>>. It would be more efficient to avoid the double allocation by defining it as Arc<Shape<N>> instead. This is not possible currently because we need to use Arc::make_mut at some point, which relies in the fact that Box<Shape<N>> implements Clone.

@Ralith suggested that we use Arc::get_mut instead, after making sure we have a unique reference. This has the specificity of working on an Arc<Shape<N>>.

[sebcrozet]

I made some implementation attempt. I first modified the ShapeClone trait to:

pub trait ShapeClone<N: RealField> {
    fn clone_arc(&self) -> Arc<Shape<N>> {
        unimplemented!()
    }
}

impl<N: RealField, T: 'static + Shape<N> + Clone> ShapeClone<N> for T {
    fn clone_arc(&self) -> Arc<Shape<N>> {
        Arc::new(self.clone())
    }
}

and the ShapeHandle::make_mut method to:

    pub(crate) fn make_mut(&mut self) -> &mut Shape<N> {
        if let Some(shape) = Arc::get_mut(&mut self.0) {
            return shape
        }

        let unique_self = self.0.clone_arc();
        self.0 = unique_self;
        Arc::get_mut(&mut self.0).unwrap()
    }

Unfortunately, this does not compiles because of borrowing issues:

error[E0502]: cannot borrow `self.0` as immutable because it is also borrowed as mutable
   --> build/ncollide3d/../../src/shape/shape.rs:161:27
    |
154 |     pub(crate) fn make_mut(&mut self) -> &mut Shape<N> {
    |                            - let's call the lifetime of this reference `'1`
155 |         {
156 |             if let Some(shape) = Arc::get_mut(&mut self.0) {
    |                                               ----------- mutable borrow occurs here
157 |                 return shape
    |                        ----- returning this value requires that `self.0` is borrowed for `'1`
...
161 |         let unique_self = self.0.clone_arc();
    |                           ^^^^^^ immutable borrow occurs here

error[E0506]: cannot assign to `self.0` because it is borrowed
   --> build/ncollide3d/../../src/shape/shape.rs:162:9
    |
154 |     pub(crate) fn make_mut(&mut self) -> &mut Shape<N> {
    |                            - let's call the lifetime of this reference `'1`
155 |         {
156 |             if let Some(shape) = Arc::get_mut(&mut self.0) {
    |                                               ----------- borrow of `self.0` occurs here
157 |                 return shape
    |                        ----- returning this value requires that `self.0` is borrowed for `'1`
...
162 |         self.0 = unique_self;
    |         ^^^^^^ assignment to borrowed `self.0` occurs here

error[E0499]: cannot borrow `self.0` as mutable more than once at a time
   --> build/ncollide3d/../../src/shape/shape.rs:163:22
    |
154 |     pub(crate) fn make_mut(&mut self) -> &mut Shape<N> {
    |                            - let's call the lifetime of this reference `'1`
155 |         {
156 |             if let Some(shape) = Arc::get_mut(&mut self.0) {
    |                                               ----------- first mutable borrow occurs here
157 |                 return shape
    |                        ----- returning this value requires that `self.0` is borrowed for `'1`
...
163 |         Arc::get_mut(&mut self.0).unwrap()
    |                      ^^^^^^^^^^^ second mutable borrow occurs here

error: aborting due to 3 previous errors

I'm not sure why this error appears.

[Ralith]

Sounds like an NLL limitation. This works as a hack-around:

    pub(crate) fn make_mut(&mut self) -> &mut Shape<N> {
        if Arc::get_mut(&mut self.0).is_none() {
            let unique_self = self.0.clone_arc();
            self.0 = unique_self;
        }
        Arc::get_mut(&mut self.0).unwrap()
    }