&mut is not the same as owning and returning

Hi rust forum!
I am having some trouble combining a function that takes and object and returns the same object, with other code that takes this object as a mutable reference. I created a small piece of code to demonstrate this (playground):

use std::mem;

struct MyNum(u32);

struct Point {
    x: MyNum,
}

fn calc(num: MyNum) -> MyNum {
    MyNum(num.0 + 1)
}

/*
/// This will not compile.
fn change_point1(point: &mut Point) {
    point.x = calc(point.x); // Cannot move out of borrowed content
}
*/

/// mem::replace approach
/// Drawback: There is the danger of forgetting to set point.x
/// in some flow of the function.
fn change_point2(point: &mut Point) {
    let x = mem::replace(&mut point.x, MyNum(0));
    // What if we forget to do this in some flow:
    point.x = calc(x);
}

/// Take ownership of the point, and return a new point.  
/// Drawback: This function has output. 
/// If we wanted to return a result, this function will have 
/// output of (Point, Result<...>) which is not fun to use.
fn change_point3(point: Point) -> Point {
    Point {
        x: calc(point.x),
    }
}

fn main() {
    let mut point = Point { x: MyNum(0) };
    change_point2(&mut point);
    let point = change_point3(point);
    assert_eq!(point.x.0, 2);
}

We have some internal type, MyNum, and a function calc that takes a MyNum and produces a new one.
Then there is some outer structure, Point, that contains a field x of type MyNum. We want to apply the function calc. Assume that we can not change the signature of calc: It must take ownership over the argument.

I consider three approaches to modifying the internal field x.

change_point1, which I hoped would work, is by doing this: point.x = calc(point.x). This does not compile, with the compiler error "Cannot move out of borrowed content".

I have the feeling that something is wrong with doing this, but I'm not sure what. My guess is that the compiler is afraid calc will "eat" the field x in case of a panic!, and then point will not have a valid state.

change_point2 uses the mem::replace. I have seen this trick in many places inside Tokio's futures code, mostly with state machines.
mem::replace will replace the field x with some dummy temporary field, giving ownership over the previous value. Using this method we can have point.x = calc(x) and it compiles.

The main drawbacks of this method are that we need to come up with some dummy instance, which is somewhat strange/ugly. In addition, it is possible to forget to set back point.x = calc(x), and then you are left with the dummy object at point.x. It is hard to see how one can forget to set point.x in the example above, but it could happen if your function gets bigger. This happened to me more than once, and it took me a while to spot it.

change_point3 is more functional / immutable approach. This reminds me of things I have seen with redux in javascript. Instead of taking &mut Point I am taking Point as an argument and return a Point.
Using this method I do not need to come up with a dummy object, and I also don't have the danger of forgetting to set point.x. However, I get a function with a more complicated signature.

If I also want to return a Result from the function (In case the function calc could fail), the resulting function signature will be something like:
fn change_point3(point: Point) -> (Point, Result<(),SomeError>)
which is not very comfortable to use.

My current approach is using change_point3 when I can, because I feel like it is the safest. However, there are cases when this approach is not possible. For example, when doing a Future implementation, one has to implement a poll() function that has this signature:

fn poll(&mut self) -> Poll<Self::Item, Self::Error>

If I want to implement Future for the Point object from above, I will have to use the approach of change_point2, as follows (Note: I did not try to compile this, but it should compile):

impl Future for Point {
    type Item = ();
    type Error = ();
    fn poll(&mut self) -> Poll<(), ()> {
        let x = mem::replace(&mut self.x, MyNum(0));
        self.x = calc(x);
        Ok(Async::Ready(())
    }
}

In this case I can not use change_point3 approach, because the signature of poll forces me to take a &mut Point.

My question is this: I would like to know if there is a sane way to make change_point1 work, or if you know of a better way to solve this issue.

I’m assuming you don’t want to/can’t make MyNum Copy (or Clone)?

1 Like

Hi Vitaly,
Yes, in my case I don't want to use Copy or Clone. For example: my object is a HashMap that contains many entries.

1 Like

I think the take_mut crate is what you are looking for.

take(&mut point.x, calc);

But note that your process will abort if calc panics, or else x would be in a memory unsafe state having been moved out of but not replaced.

1 Like

I would likely go with an approach similar to change_point2 except I'd wrap the HashMap into an Option, and then take() the map out of it. This will give you the dummy value (i.e. None) that you can unwrap/assert over, and at least detect at runtime if some code forgot to put the map back in. It's a bit unfortunate that you have to turn a compile-time checkable invariant into a runtime check, but if you can't modify calc(), nothing better springs to mind.

Since you mentioned futures, this technique of using Option to temporarily take ownership over values (and then put it back) is frequently used there.

You should ask whoever owns calc() to abstract over ownership of the argument. The function should probably be (or morally equivalent for the real type, HashMap):

fn calc<N: AsRef<MyNum>>(num: N) -> MyNum {
     MyNum(num.as_ref().0 + 1)
}
1 Like

Thank you for the great ideas.

@dtolnay: This is really interesting. I didn't know about take_mut. I have one question though: What do you mean when you say:

But note that your process will abort if calc panics

I don't know much about panics. I thought having a panic! crashes my whole process anyways. Where can I read more about this? The Rust documentation doesn't explain much about panic! internals.
Do you think that this is a change worth taking? What do I miss here in terms of safety for my program?

@vitalyd: I don't have control over the implementation of calc(). However, I didn't know about AsRef. This seems to be very useful! Can the calc function you introduced take both MyNum, &MyNum and &mut MyNum ?

1 Like

A panic may or may not crash the process - in short, depends on whether panics are allowed to unwind and whether anyone is actually handling them.

The take_mut crate is really intended for cases where you want to replace a value by applying a closure to the existing value. The problem is if the closure panics, then the field that was moved out of (cause its value was needed as an argument to the closure) is in an undefined state. take_mut deals with this possibility by straight up aborting the process, irrespective of whether panics are allowed to unwind or not.

It does (Playground of the below):

#[derive(Debug)]
struct MyNum(u32);

impl AsRef<MyNum> for MyNum {
    fn as_ref(&self) -> &MyNum {
        self
    }
}

fn calc<N: AsRef<MyNum>>(num: N) -> MyNum {
    MyNum(num.as_ref().0 + 1)
}

fn main() {
    let val = MyNum(5);
    println!("{:?}", calc(val));

    let by_ref = MyNum(10);
    println!("{:?}", calc(&by_ref));

    let mut by_mut_ref = MyNum(20);
    println!("{:?}", calc(&mut by_mut_ref));
}
1 Like

I don’t have control over the implementation of calc()

If that's the case, please don't use take_mut.

A panic runs all the neccessary destructors, prints a diagnostic message and, with the right environment variable, possibly even a backtrace.

An abort gives you nothing. (Well okay, maybe it gives you a nonzero exit code, I forget.)

Why did your program die? No idea!!
Where did it die? lolidunno, check gdb I guess!

Incidentally,

A panic runs all the neccessary destructors

this is the reason why take_mut turns panics into aborts. If it didn't, then somewhere further up the stack you'd end up with having a destructor run on uninitialized data.

1 Like

A more conventional signature here would be fn change_point4(point: Point) -> Result<Point, SomeError>. If you need to recover the Point on failure, you can always make it part of the error type.

@vitalyd: The AsRef trick is very cool. Thanks for that.

@ExpHP:

If that’s the case, please don’t use take_mut.

I will probably follow this advice. I like my safety guarantees and I don't want to mess them up. However, it is very interesting to know about the existence of take_mut. I can never know, it might be useful for something I do in the future.

@derspiny: Thanks for this idea. I could have Point as the Ok() part of my Result, but I need to put something else there. I paste here a real function from my code base to demonstrate this:

pub fn atomic_process_trans_list(balance_state: BalanceState,
                             transactions: &[NetworkerTCTransaction])
    -> (BalanceState, Result<ProcessTransListOutput, ProcessTransListError>) {

    let trans_balance_state = TransBalanceState::new(balance_state);
    match process_trans_list(transactions, trans_balance_state) {
        (tbs, Ok(out)) => (tbs.commit(), Ok(out)),
        (tbs, Err(e)) => (tbs.cancel(), Err(e)),
    }
}

This function takes a list of transactions and tries to apply them to some state called balance_state. A transaction might fail, and in that case I want to restore balance_state to what it was before I applied all the transactions. As can be seen in this example, the function signatures (Both of atomic_process_trans_list and process_trans_list) are not very fun to work with. However, it's my preferred solution so far though.