How would you have solved this borrow checker "problem"?


#1

Sorry, this kind of question may have been asked a million time but I still don’t get the borrow checker.

Here is my code

struct DebugHook {
    counter: u64
}

impl DebugHook {
    pub fn inspect(&mut self, cpu: &Cpu) {
        // here I don't use the cpu but I could log the cpu register in a mutable file for instance
        self.counter.wrapping_add(1);
    }
}

struct Cpu {
    register: u8,
    cpu_hooks: Vec<DebugHook>
    // lot of other fields here
}

impl Cpu {
    fn run_1_instruction(&mut self) {
        for hook in self.cpu_hooks.iter_mut() {
            hook.inspect(self);
        }
        // running an instruction modifies the Cpu
        self.register = 1
    }
}

fn main() {
    let mut cpu = Cpu {
        cpu_hooks: vec!(DebugHook{counter:0}),
        register: 3
    };
    
    cpu.run_1_instruction();
    assert_eq!(cpu.register, 1);
    assert_eq!(cpu.cpu_hooks[0].counter, 1)
}

And the error message is

rustc 1.14.0 (e8a012324 2016-12-16)
error[E0502]: cannot borrow `*self` as immutable because `self.cpu_hooks` is also borrowed as mutable
  --> <anon>:21:26
   |
20 |         for hook in self.cpu_hooks.iter_mut() {
   |                     -------------- mutable borrow occurs here
21 |             hook.inspect(self);
   |                          ^^^^ immutable borrow occurs here
22 |         }
   |         - mutable borrow ends here

error: aborting due to previous error

Obviously I’m violating THE rule :

Second, you may have one or the other of these two kinds of borrows, but not both at the same time:

  • one or more references (&T) to a resource,
  • exactly one mutable reference (&mut T).

Seems unfair. What could go wrong borrowing immutably (in the loop) in that case ?

I can change this code, to make it pass. For instance, by passing a copy of self.register and make DebugHook.inspect()take the register. But in my real code, Cpu is a rather big datastructure (so I dont want to clone it) and DebugHook.inspect use a lot of bits of Cpu so I can’t copy small bits of it practically.

In the rust book, this kind of issues are solved by scoping the references, but it seems impossible to do in my case.

So I started to think that my design was wrong because there is a kind of cycle. So I changed the design. I wrapped the Cpu and Vector of DebugHook separately in another struct called Computer:

struct DebugHook {
    counter: u64
}

impl DebugHook {
    pub fn inspect(&mut self, cpu: &Cpu) {
        // here I don't use the cpu but I could log the cpu register in a mutable file for instance
        self.counter.wrapping_add(1);
    }
}

struct Cpu {
    register: u8
    // lot of other fields here
}

impl Cpu {
    fn run_1_instruction(&mut self) {
        // running an instruction modifies the Cpu
        self.register = 1
    }
}

struct Computer {
    cpu: Cpu,
    cpu_hooks: Vec<DebugHook>
}

impl Computer {
    
    pub fn one_tick(&mut self) {
        for hook in self.cpu_hooks.iter_mut() {
            hook.inspect(&self.cpu);
        }
        self.cpu.run_1_instruction()
        
    }
}

fn main() {
    let mut computer = Computer {
        cpu_hooks: vec!(DebugHook{counter:0}),
        cpu: Cpu{register: 3}
    };
    
    computer.one_tick();
    assert_eq!(computer.cpu.register, 1);
    assert_eq!(computer.cpu_hooks[0].counter, 1)
}

This works. And honestly, it feels better.

But I am a newcomer in Rust and someone more experimented would have solved the problem differently. What would you have done ?


#2

Not answering your main question here, but:

You could spawn a new thread passing the &Cpu. This thread could then continue running accessing this reference thinking it would not be mutated (since a mutable reference can’t co-exist with an immutable reference which this thread has), leading to data races.


#3

You could store cpu_hooks in a RefCell. This will change aliasing checking to happen at runtime. This way you can borrow Cpu immutably many times, both for the iterator and the inspect hook.


#4

In this case, you could end up with both a mutable reference and an immutable reference to hook and, in turn end up with, e.g., a dangling pointer. Here is a contrived example:

struct DebugHook {
    counter: u64,
    something_stupid: String, // added because I don't know how to do this with plain old data.
}
struct Cpu {
    register: u8,
    cpu_hooks: Vec<DebugHook>
    // lot of other fields here
}
impl DebugHook {
    fn inspect(&mut self, cpu: &Cpu) {
        // take an immutable reference to the something_stupid string.
        let first_debug_hook_something_stupid: &str = &cpu.cpu_hooks[0].something_stupid);
        self.something_stupid = String::from("bla bla bla"); // Hmm... What happens to our old string.
        println!("Oh No!: {}", first_debug_hook_something_stupid); // invalid memory access!
    }
}

To answer your real question, I’d use either a Cell or a RefCell inside DebugHook (to give it “interior mutability”) and iterate over the DebugHooks immutably. Alternatively, you could create a Cpu “handle” (name/id) and pass that around instead of a reference to the actual Cpu object.


#5

What I don’t understand is why you’re calling

for hook in self.cpu_hooks.iter_mut() {
            hook.inspect(&self.cpu);
        }

Rather than

for hook in self.cpu_hooks.iter() {
            hook.inspect(&self.cpu);
        }

The inspect call needs an immutable ref, so why iterate mutably?


#7

Hi @jethrogb ,

Thanks for the illustration of what could go wrong. Concurrency didn’t hit my mind yesterday.

About using RefCell. Warning : I’m still newbie in Rust, what I’ll say may be stupid. I browsed the documentation of RefCell and it states :

cell types enable mutation where it would otherwise be disallowed

I come from hardcore immutable FP-ish background. What made me try Rust is how it handles mutation. Rust brings back mutability into my program without loosing my ability to reason quickly about my code. But RefCell looks like a last resort in my case because I don’t want to enable mutabililty where I can’t, I want to reintroduce immutability. But this wish seems unrealistic after you mentioned the concurrency problem I would have.

Anyway I tried to use RefCell but could’nt figure out how to create an instance.

I modified DebugHook like this

impl DebugHook {
    pub fn inspect(&mut self, cpu: RefCell<Cpu>) {
        // here I don't use the cpu but I could log the cpu register in a mutable file for instance
        self.counter.wrapping_add(1);
    }
}

Then I attempted to use it :

  • This generates an error because new needs ownership, not a reference.
impl Cpu {
    fn run_1_instruction(&mut self) {
        for hook in self.cpu_hooks.iter_mut() {
            hook.inspect(RefCell::new(self));
        }
        // running an instruction modifies the Cpu
        self.register = 1
    }
} 
  • This generates an error because I can’t move out a borrowed content :
impl Cpu {
    fn run_1_instruction(&mut self) {
        for hook in self.cpu_hooks.iter_mut() {
            hook.inspect(RefCell::new(*self));
        }
        // running an instruction modifies the Cpu
        self.register = 1
    }
} 

How do you use a RefCell in such case ?


Hi @stebalien !

Thanks for your good illustration of what could go wrong. I even don’t need concurrency to mess things up !

For the use of RefCell like I said I have 2 (beginner!) problems with it :

  1. It feels inappropriate in my use case
  2. I can’t figure out how to use it in my case

Could you provide a quick code snippet ?


Hi @jjpe !

I do use iter_mut() because inspect needs to modify self.


#8

Oh I see ! You all told me to use RefCell on the counter to “introduce mutability on something immutable”.

I managed to get this working :

use std::cell::RefCell;

struct DebugHook {
    counter: RefCell<u64>
}

impl DebugHook {
    pub fn inspect(&self, cpu: &Cpu) {
        // here I don't use the cpu but I could log the cpu register in a mutable file for instance
        let mut counter = self.counter.borrow_mut();
        *counter = *counter + 1
    }
}

struct Cpu {
    register: u8,
    cpu_hooks: Vec<DebugHook>
    // lot of other fields here
}

impl Cpu {
    fn run_1_instruction(&mut self) {
        for hook in self.cpu_hooks.iter() {
            hook.inspect(self);
        }
        // running an instruction modifies the Cpu
        self.register = 1
    }
}

fn main() {
    let mut cpu = Cpu {
        cpu_hooks: vec!(DebugHook{counter:RefCell::new(0)}),
        register: 3
    };
    
    cpu.run_1_instruction();
    assert_eq!(cpu.register, 1);
    {
        assert_eq!(cpu.cpu_hooks[0].counter.get_mut(), &1)
    }
}

This compiles but seems heavy weaponry. Is this idiomatic in Rust ?


#9

This compiles but seems heavy weaponry. Is this idiomatic in Rust ?

Not really, no. The solution you used is probably the best one.