Iterating over a vector of mutable references to trait objects

Question

I have a struct that holds mutable references to trait objects:

trait Task {
    fn do_it(&mut self);
}

struct Worker<'a> {
    tasks: Vec<&'a mut Task>,
}

In a method of Worker, I want to iterate over the tasks and call their do_it:

impl<'a> Worker<'a> {
    pub fn work(&mut self) {
        for task in self.tasks.iter() {
            self.work_one(*task);
        }
    }

    fn work_one(&self, task: &mut Task) {
        task.do_it();
    }
}

Sadly, the borrow checker does not let me do it:

error[E0389]: cannot borrow data mutably in a `&` reference
  --> src/main.rs:12:27
   |
12 |             self.work_one(*task);
   |                           ^^^^^ assignment into an immutable reference

I cannot make Worker generic because I want it to hold tasks of many types. I also need tasks to be mutable. How do I do it in Rust?

Solution 1

You are calling tasks.iter() which produces immutable references to the elements of Vec. You actually get back &&mut Task, an immutable reference to a mutable one (that is why the Rust compiler is complaining).

To solve this, call tasks.iter_mut() to get an iterator of mutable references.

The second problem is calling defining work_one as a method. You already borrow a mutable reference from self when iterating, so you cannot get another borrow.

Working example (playground):

trait Task {
    fn do_it(&mut self);
}

struct Worker<'a> {
    tasks: Vec<&'a mut Task>,
}

impl<'a> Worker<'a> {
    pub fn work(&mut self) {
        for task in self.tasks.iter_mut() {
            Worker::work_one(*task);
        }
    }

    fn work_one(task: &mut Task) {
        task.do_it();
    }
}

To still have access to self in work_one this workaround can be used. This basically just swaps the two vectors so you do not actually borrow self when iterating and then swapping it back. This is ugly, there may be a better pattern here, maybe someone else will suggest something better.

pub fn work(&mut self) {
    let mut tasks = vec![];
    mem::swap(&mut tasks, &mut self.tasks);
    for task in tasks.iter_mut() {
        self.work_one(*task);
    }
    mem::swap(&mut tasks, &mut self.tasks);
}

A nicer alternative suggested by @Veedrac:

fn work(&mut self) {
    let mut tasks = mem::replace(&mut self.tasks, Vec::new());
    for task in &mut tasks {
        self.work_one(*task);
    }
    self.tasks = tasks;
}

Solution 2

You need to have a mutable reference to each item. iter returns immutable references. And a immutable reference to a mutable variable is not itself mutable. Use iter_mut or for task in &mut self.tasks instead.

Then, the easiest thing to do is to inline work_one into work:

pub fn work(&mut self) {
    for task in self.tasks.iter_mut() {
        task.do_it()
    }
}

Unfortunately, splitting this into two functions is quite painful. You have to guarantee that calling self.work_one will not modify self.tasks. Rust doesn't track these things across function boundaries, so you need to split out all the other member variables and pass them separately to a function.

See also:

• Why does refactoring by extracting a method trigger a borrow checker error?
• cannot borrow `self.x` as immutable because `*self` is also borrowed as mutable
• Why are borrows of struct members allowed in &mut self, but not of self to immutable methods?

Comments

[RoFF]

"immutable reference to a mutable one" Aaah, this is a piece I was missing. And regarding references to self: this means that if I would need to do any change of state of self, I need to do it withing the body of the for loop. Do I get it right?

[Arjan]

@Tomo Updated the answer, not really an ideal solution. Hope someone else can suggest something better.

[RoFF]

Yes, this looks ugly. :)

[Veedrac]

@Tomo Unless you're more specific about the problem, you probably can't do much better. If you lend out self mutably, you're giving permission for unrestricted modifications. That said, it can be made prettier than two swaps.

[RoFF]

@Veedrac I don't know if I need to modify Worker's state. I asked out of curiosity. Thank you for the mem::replace trick, it's good to have it in my toolbox.

[mcferden]

It's been 4 years, but never the less... I was struggling with the same problem recently and my solution ended up being the RefCell. So, for this example it would be like this: make Worker.tasks a RefCell<Vec<Task>> and change loop to for task in self.tasks.borrow_mut().iter_mut() { ... }. I wonder is it a good and ideomatic solution or just an ugly hack?