Is it possible to mutate a struct's field inside a loop?

Question

Given is a struct which holds a struct with some byte code and an instruction pointer. It implements the pattern of fetch, decode, and execute:

use std::convert::TryFrom;

/// Trait for a virtual machine.
pub struct VirtualMachine {
    code: CodeMemory,
    instruction_pointer: usize,
}

impl VirtualMachine {
    pub fn new(byte_code: Vec<u8>) -> VirtualMachine {
        VirtualMachine {
            code: CodeMemory::new(byte_code),
            instruction_pointer: 0,
        }
    }

    /// Run a given program.
    pub fn run(&mut self) -> Result<(), &str> {
        loop {
            let opcode = self.fetch();

            if opcode.is_err() {
                return Err(opcode.unwrap_err());
            }

            let instruction = self.decode(opcode.unwrap());

            if instruction.is_err() {
                return Err("Bad opcode!");
            }

            let instruction = instruction.unwrap();

            if instruction == Instruction::Halt {
                return Ok(());
            }

            self.execute(instruction);
        }
    }

    fn fetch(&mut self) -> Result<u8, &str> {
        self.code.fetch(self.instruction_pointer)
    }

    fn decode(&mut self, opcode: u8) -> Result<Instruction, Error> {
        Instruction::try_from(opcode)
    }

    fn execute(&mut self, instruction: Instruction) {
        self.inc_instruction_pointer();

        match instruction {
            Instruction::Nop => (),
            Instruction::Halt => panic!("The opcode 'halt' should exit the loop before execute!"),
        }
    }

    fn inc_instruction_pointer(&mut self) {
        self.instruction_pointer += 1;
    }
}

struct CodeMemory {
    byte_code: Vec<u8>,
}

impl CodeMemory {
    fn new(byte_code: Vec<u8>) -> CodeMemory {
        CodeMemory { byte_code }
    }

    fn fetch(&self, index: usize) -> Result<u8, &str> {
        if index < self.byte_code.len() {
            Ok(self.byte_code[index])
        } else {
            Err("Index out of bounds!")
        }
    }

}

#[derive(Debug, PartialEq)]
pub enum Error {
    UnknownInstruction(u8),
    UnknownMnemonic(String),
}

#[derive(Debug, Copy, Clone, PartialEq)]
pub enum Instruction {
    Nop,
    // ...
    Halt,
}

impl TryFrom<u8> for Instruction {
    type Error = Error;

    fn try_from(original: u8) -> Result<Self, Self::Error> {
        match original {
            0x01 => Ok(Instruction::Nop),
            0x0c => Ok(Instruction::Halt),
            n => Err(Error::UnknownInstruction(n)),
        }
    }
}

The compiler complains that:

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/lib.rs:20:26
   |
18 |     pub fn run(&mut self) -> Result<(), &str> {
   |                - let's call the lifetime of this reference `'1`
19 |         loop {
20 |             let opcode = self.fetch();
   |                          ^^^^ mutable borrow starts here in previous iteration of loop
...
23 |                 return Err(opcode.unwrap_err());
   |                        ------------------------ returning this value requires that `*self` is borrowed for `'1`

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/lib.rs:26:31
   |
18 |     pub fn run(&mut self) -> Result<(), &str> {
   |                - let's call the lifetime of this reference `'1`
19 |         loop {
20 |             let opcode = self.fetch();
   |                          ---- first mutable borrow occurs here
...
23 |                 return Err(opcode.unwrap_err());
   |                        ------------------------ returning this value requires that `*self` is borrowed for `'1`
...
26 |             let instruction = self.decode(opcode.unwrap());
   |                               ^^^^ second mutable borrow occurs here

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/lib.rs:38:13
   |
18 |     pub fn run(&mut self) -> Result<(), &str> {
   |                - let's call the lifetime of this reference `'1`
19 |         loop {
20 |             let opcode = self.fetch();
   |                          ---- first mutable borrow occurs here
...
23 |                 return Err(opcode.unwrap_err());
   |                        ------------------------ returning this value requires that `*self` is borrowed for `'1`
...
38 |             self.execute(instruction);
   |             ^^^^ second mutable borrow occurs here

I think I understand the problem described by the compiler, but I can't find a solution or pattern how to implement this in Rust in a safe way. Is it possible to mutate a struct field inside a loop?

I'm using Rust 1.34 to use the TryFrom trait.

Answer 1

There are two things preventing your code sample from compiling.

First, you have a number of methods declared as taking &mut self when they don't need to be.

• VirtualMachine::fetch only calls CodeMemory::fetch which does not require a mutable self.

• VirtualMachine::decode does not even access any of the fields of VirtualMachine

Secondly, as pointed out in @fintella's answer, CodeMemory::fetch returns a string slice as an error.

You don't specify a lifetime of this string slice so it is inferred to be the same as the lifetime of the CodeMemory instance, which in turn is tied back to the lifetime of the VirtualMachine instance.

The effect of this is that the lifetime of the immutable borrow taken when you call fetch lasts for the whole scope of the return value from fetch - in this case pretty much the whole of your loop.

In this case, the string slice you are returning as an error message is a string literal, which has static scope, so you can fix this by changing the definition of CodeMemory::fetch to:

fn fetch(&self, index: usize) -> Result<u8, &'static str> { /* ... */ }

and VirtualMachine::fetch to:

fn fetch(&self) -> Result<u8, &'static str> { /* ... */ }

After making those changes, it compiles for me.

Answer 2

You probably do not want to be returning Result<_, &str> from any of your functions. If you use Result<_, &'static str> or Result<_, String> you should have much less fighting with the borrow checker. Even better would be to use a dedicated error type, but that is beyond the scope of this answer.

The reason that returning a Result<_, &str> is problematic is that it ends up tying the lifetime of the return value to the lifetime of self, which limits how you can use self during the lifetime of the result.