CODEWITHSUNDEEP
rust
quentinadam
quentinadam

Reputation: 3150

closure requires unique access to `self` but it is already borrowed

I basically have the following code that needs to modify a vector field (by removing some items through the retains function) and in the predicate of that retains function, I need to call a mutable function on self :

struct Task {}

struct Processor {
    counter: u32,
    tasks: Vec<Task>
}

impl Processor {
    
    fn execute(&mut self, task: &Task) -> bool {
        self.counter += 1;
        // Do some stuff with task and return wether it was sucessfully executed
        true
    }

    fn run(&mut self) {
        self.tasks.retain(|task| !self.execute(task))
    }
}

The compiler complains with two errors:

error[E0501]: cannot borrow `self.tasks` as mutable because previous closure requires unique access
  --> src/main.rs:90:9
   |
90 |         self.tasks.retain(|task| !self.execute(task))
   |         ^^^^^^^^^^^------^------^^----^^^^^^^^^^^^^^^
   |         |          |      |       |
   |         |          |      |       first borrow occurs due to use of `self` in closure
   |         |          |      closure construction occurs here
   |         |          first borrow later used by call
   |         second borrow occurs here

error[E0500]: closure requires unique access to `self` but it is already borrowed
  --> src/main.rs:90:27
   |
90 |         self.tasks.retain(|task| !self.execute(task))
   |         ---------- ------ ^^^^^^  ---- second borrow occurs due to use of `self` in closure
   |         |          |      |
   |         |          |      closure construction occurs here
   |         |          first borrow later used by call
   |         borrow occurs here

I do understand the issue, but how do I make this work ?

Upvotes: 4

Views: 1841

Answers (2)

Kevin Reid
Kevin Reid

Reputation: 43753

Option 1: Split your struct into the right parts so they can be borrowed separately. The key part here is that execute doesn't take &mut Processor, only &mut Engine.

struct Task {}

struct Engine {
    counter: u32,
}

struct Processor {
    engine: Engine,
    tasks: Vec<Task>
}

impl Engine {
    fn execute(&mut self, task: &Task) -> bool {
        self.counter += 1;
        true
    }
}

impl Processor {
    fn run(&mut self) {
        let engine = &mut self.engine;
        self.tasks.retain(|task| !engine.execute(task))
    }
}

Option 2: Move the tasks vector out with std::mem::take, so they're temporarily owned by the function instead of the Processor, then put them back:

impl Processor {
    fn run(&mut self) {
        let mut tasks = std::mem::take(&mut self.tasks);
        tasks.retain(|task| !self.execute(task));
        self.tasks = tasks;
    }
}

(This does not copy the memory for all the tasks, because the Vec's heap allocated storage is undisturbed by moving the Vec value itself.)

Option 3: If in a more complex situation you can't find a split between parts or a temporary move that works for all your operations, then you could use RefCell to do run-time instead of compile-time borrow checking — you'd need one cell for tasks and another cell or cells for the parts execute needs.

Upvotes: 3

PitaJ
PitaJ

Reputation: 15012

Rust doesn't yet have a way of only borrowing certain members of self. Because of this, it can't infer that you aren't mutating both self.tasks and self.counter at the same time. All it knows is that you're trying to mutate self twice at the same time.

To work around this, you'll need to move tasks out of self, then perform the operation you want, then move it back into self.

Rust has a handy operation that does exactly this in the standard library. Since Vec implements Default (yielding an empty vector, no allocation necessary), we can use std::mem::take:

    fn run(&mut self) {
        // swaps `self.tasks` with an empty `Vec`, and yields `tasks` to us
        // this won't allocate, because `Vec::new` and therefore `Vec::default` do not
        let mut tasks = std::mem::take(&mut self.tasks);
        // do what we want with it
        tasks.retain(|task| !self.execute(task));
        // move it back into `self`
        self.tasks = tasks;
    }

Playground

Upvotes: 6

Related Questions