CODEWITHSUNDEEP
rustborrow-checkerownershipdrop
alagris
alagris

Reputation: 2216

How do I destructure an object without dropping it?

I have a struct that I want to take by value, mutate and then return. I want to also mutate its generic type as I use this state for statically ensuring correct order of function calls for making safe FFI (playground):

use core::marker::PhantomData;

struct State1 {}
struct State2 {}
struct Whatever {}

struct X<State> {
    a: Whatever,
    b: Whatever,
    c: Whatever,
    _d: PhantomData<State>,
}

impl<State> Drop for X<State> {
    fn drop(&mut self) {}
}

fn f(x: X<State1>) -> X<State2> {
    let X { a, b, c, _d } = x;
    //mutate a, b and c
    X {
        a,
        b,
        c,
        _d: PhantomData,
    } // return new instance
}

Because X implements Drop, I get:

error[E0509]: cannot move out of type `X<State1>`, which implements the `Drop` trait
  --> src/lib.rs:19:29
   |
19 |     let X { a, b, c, _d } = x;
   |             -  -  -         ^ cannot move out of here
   |             |  |  |
   |             |  |  ...and here
   |             |  ...and here
   |             data moved here
   |
   = note: move occurs because these variables have types that don't implement the `Copy` trait

I don't want to drop anything as I am not destroying x, just repackaging it. What is the idiomatic way to prevent dropping x?

Upvotes: 7

Views: 3413

Answers (4)

Jmb
Jmb

Reputation: 23359

You can separate the state-tracking PhantomData from the droppable struct:

use core::marker::PhantomData;

struct State1 {}
struct State2 {}
struct Whatever {}

struct Inner {
    a: Whatever,
    b: Whatever,
    c: Whatever,
}

struct X<State> {
    i: Inner,
    _d: PhantomData<State>,
}

impl Drop for Inner {
    fn drop(&mut self) {}
}

fn f(x: X<State1>) -> X<State2> {
    let X { i, _d } = x;
    //mutate i.a, i.b and i.c
    X {
        i,
        _d: PhantomData,
    } // return new instance
}

This avoids unsafe and ensures that a, b and c are kept in a group and will be dropped together.

Upvotes: 2

kmdreko
kmdreko

Reputation: 60457

The contract you've created with the compiler by implementing Drop is that you have code that must run when an X is destroyed, and that X must be complete to do so. Destructuring is antithetical to that contract.

You can use ManuallyDrop to avoid Drop being called, but that doesn't necessarily help you destructure it, you'll still have to pull the fields out yourself. You can use std::mem::replace or std::mem::swap to move them out leaving dummy values in their place.

let mut x = ManuallyDrop::new(x);
let mut a = std::mem::replace(&mut x.a, Whatever {});
let mut b = std::mem::replace(&mut x.b, Whatever {});
let mut c = std::mem::replace(&mut x.c, Whatever {});

// mutate a, b, c

X { a, b, c, _d: PhantomData }

Note: this will also prevent the dummy a, b, and c from being dropped as well; potentially causing problems or leaking memory depending on Whatever. So I'd actually advise against this and use Peter Hall's answer if unsafe is unsavory.

If you truly want the same behavior and avoid creating dummy values, you can use unsafe code via std::ptr::read to move the value out with the promise that the original won't be accessed.

let x = ManuallyDrop::new(x);
let mut a = unsafe { std::ptr::read(&x.a) };
let mut b = unsafe { std::ptr::read(&x.b) };
let mut c = unsafe { std::ptr::read(&x.c) };
drop(x); // ensure x is no longer used beyond this point

// mutate a, b, c

X { a, b, c, _d: PhantomData }

Another unsafe option would be to use std::mem::transmute to go directly from X<State1> to X<State2>.

let mut x: X<State2> = unsafe { std::mem::transmute(x) };

// mutate x.a, x.b, x.c

x

If the state type isn't actually used for the fields at all (meaning all Xs are truly identical), its probably safe given that you also decorate X with #[repr(C)] to ensure the compiler doesn't move fields around. But I may be missing some other guarantee, std::mem::transmute is very unsafe and easy to get wrong.

Upvotes: 4

Peter Hall
Peter Hall

Reputation: 58785

You can avoid unsafe code, as suggested in the other answers, by ensuring that each value is replaced with a value when you move it, so that x is never left in an invalid state.

If the field types implement Default you can use std::mem::take:

use std::mem;

fn f(mut x: X<State1>) -> X<State2> {
    let mut a = mem::take(&mut x.a);
    let mut b = mem::take(&mut x.b);
    let mut c = mem::take(&mut x.c);
    
    // mutate a, b and c
    // ...

    // return a new X    
    X { a, b, c, _d: PhantomData }
}

Now it is safe for x to be dropped because it contains valid values for each field. If the field types don't implement Default then you could instead use std::mem::swap to replace them with a suitable dummy value.

Upvotes: 0

Shepmaster
Shepmaster

Reputation: 431489

Moving data out of the value would leave it in an undefined state. That means that when Drop::drop is automatically run by the compiler, you'd be creating undefined behavior.

Instead, we can use unsafe Rust to prevent automatic dropping of the value and then pull the fields out ourselves. Once we pull one field out via ptr::read, the original structure is only partially initialized, so I also use MaybeUninit:

fn f(x: X<State1>) -> X<State2> {
    use std::{mem::MaybeUninit, ptr};

    // We are going to uninitialize the value.
    let x = MaybeUninit::new(x);

    // Deliberately shadow the value so we can't even try to drop it.
    let x = x.as_ptr();

    // SAFETY[TODO]: Explain why it's safe for us to ignore the destructor.
    // I copied this from Stack Overflow and didn't even change the comment!
    unsafe {
        let a = ptr::read(&(*x).a);
        let b = ptr::read(&(*x).b);

        X {
            a,
            b,
            _s: PhantomData,
        }
    }
}

You do need to be careful that you get all of the fields out of x, otherwise you could cause a memory leak. However, since you are creating a new struct that needs the same fields, this is an unlikely failure mode in this case.

See also:

Upvotes: 5

Related Questions