1

Consider the following code:

enum Deferred<F, T> {
  Fn(F),
  Ready(T),
}

impl<F, T> Deferred<F, T>
where
  F: FnOnce() -> T,
  T: Clone,
{
  fn get_clone(&mut self) -> T {
    let clone;
    *self = match *self {
      Self::Fn(f) => { // `f` moved here
        let data = f(); // this needs to consume `f`
        clone = data.clone();
        Self::Ready(data)
      },
      Self::Ready(data) => { // `data` moved here
        clone = data.clone();
        Self::Ready(data)
      },
    };
    
    clone
  }
}

The compiler complains about the *self = match *self {...}; statement because its right hand side takes ownership of contents of self. Is there a way to accomplish this behaviour with just a mutable reference to self?

I found a workaround using F: FnMut() -> T so that f doesn't have to be moved but this approach clearly has its limitations.

I also tried what the answer to Is there a safe way to temporarily retrieve an owned value from a mutable reference in Rust? suggested but it led to an issue with initialization of clone (the compiler could no longer reason that the match statement would initialize clone because the code was moved into a closure) so I had to use MaybeUninit with unsafe.

At that point it was better to read/write self through a raw pointer:

unsafe {
  std::ptr::write(
    self as *mut Self,
    match std::ptr::read(self as *const Self) {
      Self::Fn(f) => {
        let data = f();
        clone = data.clone();
        Self::Ready(data)
      },
      Self::Ready(data) {
        clone = data.clone();
        Self::Ready(data)
      }, 
    }
  );
}
Tesik
  • 68
  • 5
  • Additional note: Why the `clone()` at all? Both arms have owned versions of `T` to begin with, there is no need for the `clone()`. You can just `Self::Ready(f())` and `Self::Ready(data)`. – user2722968 Jul 04 '21 at 19:44

3 Answers3

1

It is not possible to do this in a straightforward safe fashion, because while f is being executed there is no possible valid value of Deferred as defined: you don't yet have a T to go in Deferred::Ready, and you're consuming the F so you can't have Deferred::Fn.

If you use the take_mut crate or similar, that accomplishes this by replacing panicking with aborting, so the invalid state can never be observed. But, in your case, I would recommend introducing a third state to the enum instead — this changes the semantics but in a way that, shall we say, respects the fact that f can fail.

enum Deferred<F, T> {
    Fn(F),
    Ready(T),
    Failed,
}

impl<F, T> Deferred<F, T>
where
    F: FnOnce() -> T,
    T: Clone,
{
    fn get_clone(&mut self) -> T {
        match std::mem::replace(self, Self::Failed) {
            Self::Ready(data) => {
                let clone = data.clone();
                *self = Self::Ready(data);
                clone
            },
            Self::Fn(f) => {
                let data = f();
                *self = Self::Ready(data.clone());
                data
            }
            Self::Failed => {
                panic!("A previous call failed");
            }
        }
    }
}

With this solution, the very first thing we do is swap out *self for Self::Failed, so we own the Deferred value and can freely move out the non-clonable F value. Notice that the expression being matched is not a borrow of self, so we aren't blocked from further modifying *self.

This particular solution does have a disadvantage: it's unnecessarily writing to *self on every call (which is unobservable, but could reduce performance). We can fix that, by separating the decision of what to do from doing it, but this requires writing a second pattern match to extract the value:

impl<F, T> Deferred<F, T>
where
    F: FnOnce() -> T,
    T: Clone,
{
    fn get_clone(&mut self) -> T {
        match self {
            Self::Ready(data) => {
                return data.clone();
            },
            Self::Failed => {
                panic!("A previous call failed");
            }
            Self::Fn(_) => {
                // Fall through below, relinquishing the borrow of self.
            }
        }

        match std::mem::replace(self, Self::Failed) {
            Self::Fn(f) => {
                let data = f();
                *self = Self::Ready(data.clone());
                data
            }
            _ => unreachable!()
        }
    }
}
Kevin Reid
  • 37,492
  • 13
  • 80
  • 108
1

The fundamental problem here is trying to take ownership of the value in the enum while simultaneously having a live reference (via &mut self) to it. Safe Rust code will not let you do that, which is the reason for the compiler error. This is also a problem with your unsafe-solution: What if any code inside one of the match-arms panics? Then the owned value created via ptr::read gets dropped, the stack unwinds, and the caller might catch that panic and is then perfectly capable of observing the Deferred it owns (of which it gave a &mut to get_clone()) after it has been dropped, that is, in an invalid state and therefor causing Undefined Behaviour (see the docs for ptr::read for an example).

What must be achieved therefore is to hold &mut self in a valid/defined state while taking ownership. You can do that by having a cheap or possibly even free variant on Deferred, which is put into &mut self while the new value is being constructed.

For instance:

#[derive(Debug)]
enum FooBar {
    Foo(String),
    Bar(&'static str),
}

impl FooBar {
    fn switch(&mut self) {
        // notice here
        *self = match std::mem::replace(self, FooBar::Bar("temporary")) {
            FooBar::Foo(_) => FooBar::Bar("default"),
            FooBar::Bar(s) => FooBar::Foo(s.to_owned()),
        }
    }
}

fn main() {
    let mut s = FooBar::Bar("hello");
    s.switch();
    println!("{:?}", s);
}

Here, we use std::mem::replace to switch the value behind &mut self with a cheaply constructed temporary value; replace() returns ownership of the original value, which we match on to construct a new value; the value returned by the match-expression is then put into place via the *self-assignment. If everything goes well, the FooBar::Bar("temporary") is not ever observed; but it could be observed if the match panicked; but even then, the code is at least safe. In case your match can't unwind at all, the compiler might even be able to eliminate that useless store entirely.

Coming back to your original code, I don't see how to construct a Deferred without T being Default, as you can neither construct the Fn nor the Ready case in a safe way. Either that can be added via an additional bound, or you can add a third variant Empty to your enum.

user2722968
  • 13,636
  • 2
  • 46
  • 67
  • You could even make this nicer by implementing `Default` for `FooBar` and then use `std::mem::take`. – isaactfa Jul 05 '21 at 06:41
0

As I see it, the difference between your problem and the one that you referenced is that you also want to return the inner T while replacing self. Which is something that the there proposed crate take_mut does not provide. However, I noticed that it seems a bit unmaintained. So, I had a short look around, and found the replace_with crate instead. It basically does the same as take_mut, but it also has a replace_with_or_abort_and_return function -- exactly what you need:

use replace_with::replace_with_or_abort_and_return;

impl<F, T> Deferred<F, T>
where
    F: FnOnce() -> T,
    T: Clone,
{
    fn get_clone(&mut self) -> T {
        replace_with_or_abort_and_return(self, |s| match s {
            Self::Fn(f) => {
                // `f` moved here
                let data = f(); // this needs to consume `f`
                let clone = data.clone();
                (clone, Self::Ready(data))
            }
            Self::Ready(data) => {
                // `data` moved here
                let clone = data.clone();
                (clone, Self::Ready(data))
            }
        })
    }
}

Tho, notice, that taking a value out of a mutable borrow, has the potential of UB that is specifically if your f() panics, because self would be left in an uninitialized state. Therefore, we get this slightly cumbersome function name, which indicates that if f() panic, the program will be aborted (which is the safe thing to do). However, if you expect f() to panic and don't want to abort the program, you can instead provide a default for self via the replace_with_and_return or replace_with_or_default_and_return function.

Cryptjar
  • 1,079
  • 8
  • 13