Mutable borrow too long when mutating in a loop

Question

I have a piece of code that needs to store Strings and access references to those strings. I first wrote it as follows:

struct Pool {
    strings : Vec<String>
}

impl Pool {
    pub fn new() -> Self {
        Self {
            strings: vec![]
        }
    }

    pub fn some_f(&mut self) -> Vec<&str> {
        let mut v = vec![];
        
        for i in 1..10 {
            let string = format!("{}", i);
            let string_ref = self.new_string(string);
            v.push(string_ref);
        }
    
        v
    }
    
    fn new_string(&mut self, string : String) -> &str {
        self.strings.push(string);
        &self.strings.last().unwrap()[..]
    }
}

This does not pass the borrow checker:

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/main.rs:19:30
   |
14 |     pub fn some_f(&mut self) -> Vec<&str> {
   |                   - let's call the lifetime of this reference `'1`
...
19 |             let string_ref = self.new_string(string);
   |                              ^^^^ mutable borrow starts here in previous iteration of loop
...
23 |         v
   |         - returning this value requires that `*self` is borrowed for `'1`

So apparently the borrow-checker isn't smart enough to realize that the mutable borrow doesn't extend beyond the call to new_string. I tried separating the part that mutates the structure from retrieving references, arriving at this code:

use std::vec::*;

struct Pool {
    strings : Vec<String>
}

impl Pool {
    pub fn new() -> Self {
        Self {
            strings: vec![]
        }
    }

    pub fn some_f(&mut self) -> Vec<&str> {
        let mut v = vec![];
        
        for i in 1..10 {
            let string = format!("{}", i);
            self.new_string(string);
        }
        for i in 1..10 {
            let string = &self.strings[i - 1];
            v.push(&string[..]);
        }
    
        v
    }
    
    fn new_string(&mut self, string : String) {
        self.strings.push(string);
    }
}

This is semantically equivalent (hopefully) and does compile. However doing as much as combining the two for loops into one:

for i in 1..10 {
    let string = format!("{}", i);
    self.new_string(string);
    let string = &self.strings[i - 1];
    v.push(&string[..]);
}

gives a similar borrowing error:

error[E0502]: cannot borrow `*self` as mutable because it is also borrowed as immutable
  --> src/main.rs:19:13
   |
14 |     pub fn some_f(&mut self) -> Vec<&str> {
   |                   - let's call the lifetime of this reference `'1`
...
19 |             self.new_string(string);
   |             ^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
20 |             let string = &self.strings[i - 1];
   |                           ------------ immutable borrow occurs here
...
24 |         v
   |         - returning this value requires that `self.strings` is borrowed for `'1`

I have several questions:

1. Why is the borrow checker so strict as to extend the mutable borrow for the entire duration of the loop in this case? Is it impossible/very hard to analyse that the &mut passed to new_string does not leak beyond that function call?

2. Is it possible to fix this issue with custom lifetimes so that I can go back to my original helper that both mutates and returns a reference?

3. Is there a different, more Rust-idiomatic way that doesn't upset the borrow checker in which I could achieve what I want, i.e. have a structure that mutates and returns references to itself?

I found this question, but I don't understand the answer (is it a negative answer to #2? no idea) and most other questions have issues with explicit lifetime parameters. My code uses only inferred lifetimes.

Answer 1

In this case the borrow checker is correct in not allowing this:

    self.new_string(string);
    let string = &self.strings[i - 1];
    v.push(&string[..]);

self.new_string could result in all previous references that you pushed to v to become invalid, since it might need to allocate memory for strings and move its contents. The borrow checker catches this because the references you push to v need a lifetime to match v's, so &self.strings (and therefore &self) must be borrowed for the whole method, which prevents your mutable borrow.

If you use two loops, there's no shared borrow active at the time that you call new_string.

You can see that it's not the mutable borrow being extended that's the issue, in this (completely useless) version of the loop, which compiles:

for i in 1..10 {
    let string = format!("{}", i);
    self.new_string(string);
    let mut v2 = vec![];
    let string = &self.strings[i - 1];
    v2.push(&string[..]);
}

As for a more idiomatic way, the Vec class is free to invalidate references in mutable operations, so you can't do what you want in safe rust. You wouldn't want to do it with a c++ vector either, even if the compiler lets you, unless you preallocate the vector and manually ensure you never push more elements than what you initially allocated. Obviously rust doesn't want you to be manually verifying the memory safety of your program; the size of a preallocation is not visible in the type system, and cannot be checked by the borrow checker, so this approach is not possible.

You cannot solve this even if you use a fixed sized container like [String; 10]. In that case there could be no allocation, but what would actually make this safe is the fact that you're never updating an index from which you've already taken a reference. But rust has no concept of partial borrows from containers, so there's no way to tell it "there's a shared borrow up to index n so it's ok for me to do a mutable borrow from index n + 1".

If you really need a single loop for performance reasons, you'll need to preallocate, and use an unsafe block, eg.:

struct Pool {
    strings: Vec<String>,
}

const SIZE: usize = 10;

impl Pool {
    pub fn new() -> Self {
        Self {
            strings: Vec::with_capacity(SIZE),
        }
    }

    pub fn some_f(&mut self) -> Vec<&str> {
        let mut v: Vec<&str> = vec![];

        // We've allocated 10 elements, but the loop uses 9, it's OK as long as it's not the other way around!
        for i in 1..SIZE {
            let string = format!("{}", i);
            self.strings.push(string);
            let raw = &self.strings as *const Vec<String>;
            unsafe {
                let last = (*raw).last().unwrap();
                v.push(last);
            }
        }

        v
    }
}

A possible alternative is to use Rc, though if your reason for wanting a single loop is performance, using the heap + the runtime costs of reference counting might be a bad tradeoff. Here's the code in any case:

use std::rc::Rc;

struct Pool {
    strings: Vec<Rc<String>>,
}

impl Pool {
    pub fn new() -> Self {
        Self { strings: vec![] }
    }

    pub fn some_f(&mut self) -> Vec<Rc<String>> {
        let mut v = vec![];

        for i in 1..10 {
            let string = format!("{}", i);
            let rc = Rc::new(string);
            let result = rc.clone();
            self.strings.push(rc);
            v.push(result);
        }

        v
    }
}