Best way to solve a "it is already borrowed error"

help
1

Given the following code:

struct Action {
    diff_x: f32,
    diff_y: f32,
}

pub struct Actor {
    x: f32,
    y: f32,
    actions: Vec<Action>
}

impl Actor {
    fn translate(&mut self, x: f32, y: f32) {
        self.x += x;
        self.y += y;
    }
    
    fn act(&mut self) {
        self.actions.iter_mut().for_each(|a| a.act(self));
    }
}

impl Action {
    fn act(&mut self, actor: &mut Actor) {
        actor.translate(self.diff_x, self.diff_y);
    }
}

fn main() {
    let mut actor = Actor { x: 0.0, y: 0.0, actions: vec![Action { diff_x: 1.0, diff_y: 2.0 }] };
    actor.act();
}

I get the following error:

error[E0500]: closure requires unique access to `*self` but it is already borrowed
19 |         self.actions.iter_mut().for_each(|a| a.act(self));
   |         ------------            -------- ^^^       ---- second borrow occurs due to use of `*self` in closure
   |         |                       |        |
   |         |                       |        closure construction occurs here
   |         |                       first borrow later used by call
   |         borrow occurs here

I understand the borrowing problem, but I don't know what's the best way to fix it. For now, I'm doing:

    fn act(&mut self) {
        let actions = std::mem::replace(&mut self.actions, Vec::new());
        for mut action in actions {
            action.act(self);
            self.actions.push(action);
        }
    }

but it doesn't feel right. Is there a better way?

2

Assuming that you don't need an Action to modify the actions list, you can modify your definitions to split the borrows:

struct Action {
    diff_x: f32,
    diff_y: f32,
}

pub struct ActorData {
    x: f32,
    y: f32,
}

pub struct Actor {
    data: ActorData,
    actions: Vec<Action>,
}

impl ActorData {
    fn translate(&mut self, x: f32, y: f32) {
        self.x += x;
        self.y += y;
    }
}

impl Actor {
    fn act(&mut self) {
        self.actions.iter_mut().for_each(|a| a.act(&mut self.data));
    }
}

impl Action {
    fn act(&mut self, actor: &mut ActorData) {
        actor.translate(self.diff_x, self.diff_y);
    }
}
3 Likes
3

@2e71828's solution is right if you don't need the Actions to have access to an Actor with actions, but your approach above can work if they do (maybe an action can add further actions). In that case you can neaten it a little using std::mem::take and it can be written in one line like the original version if you want:

std::mem::take(&mut self.actions).iter_mut().for_each(|a| a.act(self));
2 Likes
4

Thanks for the suggestions.

@jameseb7 std::mem::take leaves the list empty, so to keep it intact, I'd need to do something like this, right?

    fn act(&mut self) {
        let mut actions = std::mem::take(&mut self.actions);
        actions.iter_mut().for_each(|a| { 
            a.act(self);
        });
        let _ = std::mem::replace(&mut self.actions, actions);
    }

@2e71828 Splitting the borrows works. That way I don't need to deal with recreating the list.

5

The last line can just be self.actions = actions, but yes.

1 Like
6

Or self.actions.extend(actions) so that you don't silently discard any new actions that were added during the loop (by another action).

3 Likes