17

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?

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
Tomo
  • 3,431
  • 5
  • 25
  • 28

2 Answers2

28

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;
}
Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
Arjan
  • 19,957
  • 2
  • 55
  • 48
  • "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? – Tomo May 02 '16 at 21:15
  • @Tomo Updated the answer, not really an ideal solution. Hope someone else can suggest something better. – Arjan May 02 '16 at 21:44
  • 1
    Yes, this looks ugly. :) – Tomo May 02 '16 at 21:46
  • @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 `swap`s](https://play.rust-lang.org/?gist=987c95c143faf9115e40b8dca3eea034&version=stable&backtrace=0). – Veedrac May 03 '16 at 05:33
  • @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. – Tomo May 03 '16 at 06:44
  • 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>` 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? – mcferden Oct 13 '20 at 15:16
6

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:

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366