How do I make iterating over a vector on a struct via self mutable and use that mutable reference later?

Question

I'm trying to iterate over a vector of statements and match (or double match) the results from evaluating them. I think the root cause of my error is the immutable borrow from iterating: for stmt in &self.statements

I'm totally fine changing my whole design if I've worked myself into a corner here.

I thought mutable borrows during an immutable borrow is fine,as long as there is only one user/consumer of the mutable borrow at a time.

The error:

Checking rust-playground v0.1.0 (C:\Users\joraki\projects\rust-playground)
error[E0502]: cannot borrow `*self` as mutable because it is also borrowed as immutable
  --> src\main.rs:25:29
   |
18 |         for stmt in &self.statements {
   |                     ----------------
   |                     |
   |                     immutable borrow occurs here
   |                     immutable borrow later used here
...
25 |                 Err(err) => self.runtime_error(&err)
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here

For more information about this error, try `rustc --explain E0502`.
error: could not compile `rust-playground` due to previous error

Here refers to for stmt in &self.statements

fn main() {
    println!("Hello, world!");
}

#[derive(PartialEq, Debug)]
pub enum Stmt {
    Expression(String),
    Print(String)
}

pub struct Interpreter {
    statements: Vec<Stmt>,
    runtime_error: bool
}

impl Interpreter {
    pub fn interpret(&mut self) {
        for stmt in &self.statements {
            let result = self.evaluate(stmt);
            match result {
                Ok(x) => match x {
                    Some(y) => println!("{}", y),
                    None => println!("No value returned. No errors.")
                },
                Err(err) => self.runtime_error(&err)
            }
        }
    }
    
    fn runtime_error(&mut self, err: &str) {
        self.runtime_error = true;
        println!("{}", err);
    }

    fn evaluate(&self, stmt: &Stmt) -> Result<Option<String>, String> {
        match stmt {
            Stmt::Expression(expr) => {
                Ok(Some(expr.to_string()))
            },
            Stmt::Print(expr) => Err(format!("Something bad happened - {}", expr))
        }
    }
}

Please provide a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) that actually reproduces your error. When I try to build your code, I primarily get `error: \`self\` parameter is only allowed in associated functions`. Further, don't copy-paste the error from your IDE, but instead run `cargo check` and paste its output here. — Finomnis, Sep 23 '22 at 18:14
I updated my code to be a working sample that reproduces the same error and updated the error to be the exact output of `cargo check`. — Josh R, Sep 23 '22 at 18:28
You could always make an `Rc<[Stmt]>` to access the statements too. Lots of options depending on what your goals are. — loganfsmyth, Sep 23 '22 at 18:51

Finomnis · Accepted Answer · 2022-09-23T18:58:28.217

I thought mutable borrows during an immutable borrow is fine, as long as there is only one user/consumer of the mutable borrow at a time.

Yes and no. Borrows cannot overlap. If you mutably borrow through one reference, then you have to mutate it over this reference, you can't have a second one.

For example in your case: The for stmt in &self.statements borrows a part of self for the entire duration of the loop. Now if you were permitted mutable access during that period, it would be possible for the runtime_error to modify self.statements. In most languages where this is not a compiler error (like C++), modifying a vector while iterating over it results in undefined behaviour. As Rust by definition does not have undefined behaviour, this results in a compiler error.

I think there are a couple of ways to refactor your code to work around this issue. But this is not a false positive by the compiler, this is an actual problem.

One thing you could do is replace the bool with an AtomicBool, which has no runtime overhead, but can be modified while non-mutable through interior mutability. That way, your runtime_error function could be non-mutable:

use std::sync::atomic::{AtomicBool, Ordering};

#[derive(PartialEq, Debug)]
pub enum Stmt {
    Expression(String),
    Print(String),
}

pub struct Interpreter {
    statements: Vec<Stmt>,
    runtime_error: AtomicBool,
}

impl Interpreter {
    pub fn interpret(&mut self) {
        for stmt in &self.statements {
            let result = self.evaluate(stmt);
            match result {
                Ok(x) => match x {
                    Some(y) => println!("{}", y),
                    None => println!("No value returned. No errors."),
                },
                Err(err) => self.runtime_error(&err),
            }
        }
    }

    fn runtime_error(&self, err: &str) {
        self.runtime_error.store(true, Ordering::Relaxed);
        println!("{}", err);
    }

    fn evaluate(&self, stmt: &Stmt) -> Result<Option<String>, String> {
        match stmt {
            Stmt::Expression(expr) => Ok(Some(expr.to_string())),
            Stmt::Print(expr) => Err(format!("Something bad happened - {}", expr)),
        }
    }
}

That said, if I look at the intention of your code (implementing an interpreter) I think it might be an even better solution to split the statements and the state of the interpreter into two different structs.

The real problem in your original code arises because Rust has to assume that you might modify statements in your runtime_error method. If you split those into two structs, the problem would go away:

#[derive(PartialEq, Debug, Clone)]
pub enum Stmt {
    Expression(String),
    Print(String),
}

struct InterpreterState {
    runtime_error: bool,
}

impl InterpreterState {
    fn runtime_error(&mut self, err: &str) {
        self.runtime_error = true;
        println!("{}", err);
    }

    fn evaluate(&self, stmt: &Stmt) -> Result<Option<String>, String> {
        match stmt {
            Stmt::Expression(expr) => Ok(Some(expr.to_string())),
            Stmt::Print(expr) => Err(format!("Something bad happened - {}", expr)),
        }
    }
}

pub struct Interpreter {
    statements: Vec<Stmt>,
    state: InterpreterState,
}

impl Interpreter {
    pub fn interpret(&mut self) {
        for stmt in &self.statements {
            let result = self.state.evaluate(stmt);
            match result {
                Ok(x) => match x {
                    Some(y) => println!("{}", y),
                    None => println!("No value returned. No errors."),
                },
                Err(err) => self.state.runtime_error(&err),
            }
        }
    }
}

This is because the compiler is now able to deduce that you are using &self.statements simultaneously with &mut self.state, which do not overlap/collide.

This is so helpful and excellent, thank you! I appreciate how you looked at the intention of my code and helped me understand that splitting the state apart - so the Rust compiler can tell I'm not modifying the `self.statements` iterable is great. I'll be using this technique again :) — Josh R, Sep 23 '22 at 20:34

score 0 · Answer 2 · answered Sep 23 '22 at 18:23

To resolve the error, you need to derive clone for Interpreter struct. Before iterating over self.statements, clone self into some variable and iterate over that variable and you can use self inside for loop.

The below modified code solves the error mentioned in the question.

#[derive(Clone)]
pub struct Interpreter {
    statements: Vec<Stmt>,
    runtime_error: bool
}

impl Interpreter {
    pub fn interpret(&mut self) {
        let mut interpretor = self.clone();
        for stmt in &interpretor.statements {
            let result = self.evaluate(stmt);
            match result {
                Ok(x) => match x {
                    Some(y) => println!("{}", y),
                    None => println!("No value returned. No errors.")
                },
                Err(err) => self.runtime_error(&err)
            }
        }
    }
    
    fn runtime_error(&mut self, err: &LoxError) {
        self.runtime_error = true;
        println!("{}", err);
    }
}

While that would probably work, I'm not sure if Interpreter is meant to be cloned. So I'd say it is one possible solution, but not a definite one. — Finomnis, Sep 23 '22 at 18:31
If I'd go with that solution, I would only clone `self.statements`, not the entire struct. That only requires `Stmt` to be `Clone`, not `Interpreter`. But I think cloning here is overhead that could be prevented with a small structural change. — Finomnis, Sep 23 '22 at 18:59

How do I make iterating over a vector on a struct via self mutable and use that mutable reference later?

2 Answers2