nialna2
nialna2

Reputation: 2224

How can I pass my mutable references to a function inside iterator loops in Rust?

I have the following code:

    pub fn run_systems(mut self) {
        for mut agent in self.agents.iter_mut() {
            for mut system_id in agent.systems.iter_mut() {
                let system = self.systems.get(system_id).unwrap();
                system.simulate(&mut agent,  &mut self);
            }
        }
    }

I get the following error on the system.simulate line:

cannot borrow `agent` as mutable more than once at a time
second mutable borrow occurs here`. Apparently, iterating over the arrays is a borrowing operation.

I have also tried not using the iter function and iterating directly on owned values (not sure what that does):

    pub fn run_systems(mut self) {
        for mut agent in self.agents {
            for mut system_id in agent.systems {
                let system = self.systems.get(&system_id).unwrap();
                system.simulate(&mut agent,  &mut self);
            }
        }
    }

But as soon as I reference &system_id, it detects an implicit borrowing in the for mut system_id in agent.systems { line

borrow of partially moved value: `agent`
partial move occurs because `agent.systems` has type `Vec<String>`, which does not implement the `Copy` traitrustcE0382
lib.rs(72, 34): `agent.systems` partially moved due to this implicit call to `.into_iter()`
collect.rs(233, 18): this function takes ownership of the receiver `self`, which moves `agent.systems`

I have tried all sorts of ways to write this code but can't find a way that works. How can I iterate over those values while also being able to pass mutable references to their content to other functions?

Here's a playground of it:

use std::collections::HashMap;

struct World {
    agents: Vec<Agent>,
    systems: HashMap<String, Box<dyn System>>,
}
impl World {
    pub fn new() -> World {
        return World {
            agents: Vec::new(),
            systems: HashMap::new(),
        };
    }
    pub fn run_systems(mut self) {
        for mut agent in self.agents {
            for system_id in agent.systems {
                let system = self.systems.get(&system_id).unwrap();
                system.simulate(&mut agent, &mut self);
            }
        }
    }
    /// Adds an agent to the world
    pub fn add_agent(&mut self, agent: Agent) -> &Self {
        self.agents.push(agent);
        return self;
    }

    /// Adds a system to the available systems
    pub fn add_system<S: System + 'static>(&mut self, system: S, id: String) -> &Self {
        self.systems.insert(id, Box::new(system));
        return self;
    }
}

struct Agent {
    systems: Vec<String>,
}

trait System {
    fn simulate(&self, agent: &mut Agent, world: &mut World);
}

#[derive(Default)]
struct SomeSystem;
impl System for SomeSystem {
    fn simulate(&self, agent: &mut Agent, world: &mut World) {
        // Code here
    }
}

fn main() {
    let system_id = String::from("SOME_SYSTEM");
    let world = World::new();
    world.add_system(SomeSystem::default(), system_id);
    let agent = Agent {
        systems: vec![system_id],
    };
    world.add_agent(agent);
    world.run_systems();
}
error[E0382]: borrow of partially moved value: `agent`
   --> src/main.rs:18:33
    |
16  |             for system_id in agent.systems {
    |                              -------------
    |                              |
    |                              `agent.systems` partially moved due to this implicit call to `.into_iter()`
    |                              help: consider borrowing to avoid moving into the for loop: `&agent.systems`
17  |                 let system = self.systems.get(&system_id).unwrap();
18  |                 system.simulate(&mut agent, &mut self);
    |                                 ^^^^^^^^^^ value borrowed here after partial move
    |
note: this function takes ownership of the receiver `self`, which moves `agent.systems`
    = note: partial move occurs because `agent.systems` has type `Vec<String>`, which does not implement the `Copy` trait

error[E0502]: cannot borrow `self` as mutable because it is also borrowed as immutable
  --> src/main.rs:18:45
   |
17 |                 let system = self.systems.get(&system_id).unwrap();
   |                              ------------ immutable borrow occurs here
18 |                 system.simulate(&mut agent, &mut self);
   |                        --------             ^^^^^^^^^ mutable borrow occurs here
   |                        |
   |                        immutable borrow later used by call

error[E0382]: borrow of partially moved value: `self`
  --> src/main.rs:18:45
   |
15 |         for mut agent in self.agents {
   |                          -----------
   |                          |
   |                          `self.agents` partially moved due to this implicit call to `.into_iter()`
   |                          help: consider borrowing to avoid moving into the for loop: `&self.agents`
...
18 |                 system.simulate(&mut agent, &mut self);
   |                                             ^^^^^^^^^ value borrowed here after partial move
   |
   = note: partial move occurs because `self.agents` has type `Vec<Agent>`, which does not implement the `Copy` trait

error[E0596]: cannot borrow `world` as mutable, as it is not declared as mutable
  --> src/main.rs:54:5
   |
53 |     let world = World::new();
   |         ----- help: consider changing this to be mutable: `mut world`
54 |     world.add_system(SomeSystem::default(), system_id);
   |     ^^^^^ cannot borrow as mutable

error[E0382]: use of moved value: `system_id`
  --> src/main.rs:56:23
   |
52 |     let system_id = String::from("SOME_SYSTEM");
   |         --------- move occurs because `system_id` has type `String`, which does not implement the `Copy` trait
53 |     let world = World::new();
54 |     world.add_system(SomeSystem::default(), system_id);
   |                                             --------- value moved here
55 |     let agent = Agent {
56 |         systems: vec![system_id],
   |                       ^^^^^^^^^ value used here after move

error[E0596]: cannot borrow `world` as mutable, as it is not declared as mutable
  --> src/main.rs:58:5
   |
53 |     let world = World::new();
   |         ----- help: consider changing this to be mutable: `mut world`
...
58 |     world.add_agent(agent);
   |     ^^^^^ cannot borrow as mutable

Upvotes: 4

Views: 1614

Answers (3)

nialna2
nialna2

Reputation: 2224

I'll answer my own question with the solution I ended up choosing.

Fundamentally, the problem with the code in the question is that we are trying to mutate on something while iterating on it (the World struct and the agents list). Rust protects us against that because we could be completely changing the size or entries in the array that we're in the middle of iterating, which is a common bug you have to consider when writing code in other languages.

The solution is to simply not do that and instead make sure you don't iterate on the same thing as what you're iterating. I did need my systems to mutate agent states though, so what I did was splitting them in two different parts:


// Agents have an id
pub struct Agent {
    pub id: u128,
    pub systems: Vec<String>,
}

// New AgentState separate struct
pub trait AgentState {
}

// The worrld contains a hashmap of states
struct World {
    agents: Vec<Agent>,
    agent_states: HashMap<u128, Box<dyn AgentState>>,
    systems: HashMap<String, Box<dyn System>>,
}

impl World {

    // When adding a new agent, we also setup its state
    pub fn add_agent<A: AgentState + 'static>(&mut self, state: A) -> &Self{
        let agent = Agent::new(Vec::new());
        let id = agent.id;
        self.agents.push(agent);
        self.agent_states.insert(id, Box::new(state));
        self
    }

    // When running systems, we can pass a mutable reference to the states hashmap because we're not iterating on it
    pub fn run_systems(&'static mut self) -> &Self {
        for agent in self.agents.iter_mut() {
            let systems = agent.systems.clone();
            for system_id in systems {
                let system = self.systems.get(&system_id).unwrap();
                system.simulate(agent, &mut self.agent_states);
            }
        }
        self
    }
}

// The System trait signature now changes the simulate function to take the hashmap of states
/// The main trait that all system structs need to implement
pub trait System {
    fn id() -> String where Self: Sized;
    fn dyn_id(&self) -> String;
    /// This function is called for every actor that uses the system and gives user code the opportunity to change the state
    fn simulate<'a>(&self, agent: &'a mut Agent, states: &'a mut HashMap<u128, Box<dyn AgentState>>);
}

With this setup, when agents are created, their state is stored separately and can be mutated independently of iterating on agents.

There is one big caveat to this which is that you can't add or remove agent states inside systems, because things would get out of sync (the agent would still exist while its state wouldn't). For more advanced use cases like this, a different approach would be needed.

This solution works as long as dynamic addition/removal of agents isn't needed.

Upvotes: 0

Michael Anderson
Michael Anderson

Reputation: 73590

One way that I have used to approach this kind of issue is to have simulate not accept mutable references, but to return a list of the actions it wishes to perform. This might looks something like this:


// No longer uses mutable references, and returns a list of 
// actions to perform on the world.
trait System {
    fn simulate(&self, agent: &Agent, world: &World) -> Vec<Action>;
}

enum Action {
   ...
}

impl World {
    ...

    pub fn run_systems(&mut self) {
        let mut actions = vec![];
        for agent in &self.agents {
            for system_id in &agent.systems {
                let system = self.systems.get(system_id).unwrap();
                actions.extend(system.simulate(agent, self));
            }
        }

        for action in actions {
            self.apply_action(action)
        }
    }

    pub fn apply_action(&mut self, action: &Action) {
        match(action) {
           case SomeAction(agent_id) -> {
             ...
        }
    }
}

Upvotes: 1

user2722968
user2722968

Reputation: 16535

In either case, this can't work with the way simulate() is defined. It takes a &mut Self as a second parameter, which implies exclusive access to self - all of self. But this is not possible because A) there are references to self via the iterators in the first case or B) self has been partially destructured by taking ownership of self.agents in the second case.

Upvotes: 0

Related Questions