How is mutably borrowing twice in separate scopes an issue?

1

In my code below, I (thought I) correctly scoped my mutable borrows, but Rust compiler insists there's more than 1 borrow occurring "at the same time". I'm boggled. This is synchronous code and scope should kill the prior references, no?

        {
            let r = &mut roots;
            let s = &mut store;
            let l = &mut rels;
            build(&mut items, r, s, l);
        }
        {
            let r = &mut roots;
            let s = &mut store;
            build_top(lt, r, s);
        }
        {
            let s = &mut store;
            let l = &mut rels;
            build_instance(1, "2", "4", s, l);
        }

error[E0499]: cannot borrow `roots` as mutable more than once at a time
  --> src/clue/tree_tests.rs:26:21
   |
20 |             let r = &mut roots;
   |                     ---------- first mutable borrow occurs here
...
26 |             let r = &mut roots;
   |                     ^^^^^^^^^^
   |                     |
   |                     second mutable borrow occurs here
   |                     first borrow later used here

error[E0499]: cannot borrow `store` as mutable more than once at a time
  --> src/clue/tree_tests.rs:27:21
   |
21 |             let s = &mut store;
   |                     ---------- first mutable borrow occurs here
...
27 |             let s = &mut store;
   |                     ^^^^^^^^^^
   |                     |
   |                     second mutable borrow occurs here
   |                     first borrow later used here

error[E0499]: cannot borrow `store` as mutable more than once at a time
  --> src/clue/tree_tests.rs:31:21
   |
21 |             let s = &mut store;
   |                     ---------- first mutable borrow occurs here
...
31 |             let s = &mut store;
   |                     ^^^^^^^^^^
   |                     |
   |                     second mutable borrow occurs here
   |                     first borrow later used here

error[E0499]: cannot borrow `rels` as mutable more than once at a time
  --> src/clue/tree_tests.rs:32:21
   |
22 |             let l = &mut rels;
   |                     --------- first mutable borrow occurs here
...
32 |             let l = &mut rels;
   |                     ^^^^^^^^^
   |                     |
   |                     second mutable borrow occurs here
   |                     first borrow later used here
2

It usually isn't an issue.

What's the definition of your build* functions? They are probably extending the scope of the loan by declaring that it can be put inside of the longer-lived objects.

6 Likes
3

Note that this kind of scoping is no longer needed for references since NLL were implemented in the 2018 edition. It can still be helpful for types implementing Drop though.

As the other comment pointed out you're likely defined the build* functions in a way such that the lifetime of the first argument is extended somehow. Probably by reusing the same lifetime twice (e.g. with something like s: &'a mut Store<'a>). Hard to say anything more without those functions definitions though.

5 Likes
4

Wow, had no idea the same lifetime twice was an issue? Couldn't compiler clarify that? Do I just use 2 lifetimes I suppose?

yup, just needed e.g.:

pub fn build<'a, 'b>(
    // other parms...
    roots: &'b mut Vec<&'a str>,

Now why is that?

Well... I guess by only using 'a I'm telling the compiler that the mutable reference exists for as long as the data inside it (which stays in scope much much longer).

Yet... it's impossible for that reference to exist once exited from scope still isn't it? Shouldn't the compiler instead say: 'a lifetime isn't possible within scope this reference exists in or something?

1 Like
5

Well... I guess by only using 'a I'm telling the compiler that the mutable reference exists for as long as the data inside it (which stays in scope much much longer).

Yes, almost exactly. When you write &'a mut Vec<&'a str>, you’re saying that, for the purposes of this function call, the strs are borrowed for exactly as long as the Vec containing them is. (Not that the &'a mut ... must exist that long, but it can exist that long.) Since the Vec would be invalid if it contained expired borrows, this leads the borrow checker to concluding that the Vec is borrowed for its entire existence. Therefore, once you attribute this type to it, you can only use it once.

In general, the type &'a mut SomeTypeHavingALifetimeInIt<'a> is useless.

(&'a SomeType<'a> is acceptable[1], because without mutation involved, 'a can stay covariant; that is, it's valid to treat &'long SomeType<'long> as if it were &'short SomeType<'short>.)

As a rule of thumb, when writing down function parameters, it is wise to let lifetime parameters be different until you know they must be made the same. In particular, that’s what happens automatically when lifetimes are elided, and that’s because it’s the right default. When you find yourself needing to name a lifetime instead of eliding it, don't assume that it should be equal to another lifetime unless you know why it should be equal. For an example of such a reason, if you’re going to push() an item into a Vec<&'a str>, then the function must accept an &'a str. But you don’t have any such requirement on a mutable reference to the Vec you’re pushing into — that borrow can and should be short.

Shouldn't the compiler instead say: 'a lifetime isn't possible within scope this reference exists in or something?

The borrow checker can’t really distinguish “this is an impossible situation” from an ordinary use-after-invalidity; it doesn’t have enough information about “what you meant”. That said, it could probably manage to recognize the &'a mut Vec<&'a str>, which is a quite common mistake, but no one has made it do that yet.

Also, it is technically possible to make use of a &'a mut Vec<&'a str>, but only once. I’ve never seen a use case for this, but there probably is one somewhere — at least for some type other than Vec.

  1. unless SomeType has interior mutability ↩︎

13 Likes
6

ugh... but I am trying to do an insert and so that requires where 'b : 'a

And once I set that I'm right back where I started...

So how can I possibly do this mutation? This is for lalrpop - while parsing a file I want to build Vecs and HashMaps full of action code results. So multiple action code needs to be able to mutate.

What's bothering me about this is I can't imagine a race condition. The code executes synchronously. I would understand in asynchronous code two mutable references could stomp/race each other. But how is that a concern here?

I know the classic problem of reallocation breaking references, but I'm not referencing anything in the same scope as inserting. So I just don't get it, sorry...

Example:

pub fn build_top<'a, 'b>(
    top: &'b str,
    roots: &'b mut Vec<&'a str>,
    store: &'b mut HashMap<&'a str, TreeItem<'a, &'a str>>
) where 'b : 'a {
    store.insert(top, TreeItem::new_leaf(None, top, top, None));
    roots.push(top);
}
7

Don't you need top: &'a str here? It doesn't have to be here for as long as references roots and store exist - it has to be here for as long as Vec and HashMap exist.

...Although it's also possible that you don't need &strs at all and can use something owned, like Rc<str>, to avoid lifetime management at all.

2 Likes
8

The insert doesn't require where 'b: 'a. You’ve created that requirement yourself by making the value you’re inserting be &'b str instead of &'a str. That ends up requiring 'b and 'a to be the same lifetime, more or less. The correct signature is:

pub fn build_top<'a, 'b>(
    top: &'a str,                       // <-- changed this
    roots: &'b mut Vec<&'a str>,
    store: &'b mut HashMap<&'a str, TreeItem<'a, &'a str>>
) {

Or, following my previous advice to let lifetimes be different and even elided:

pub fn build_top<'a>(
    top: &'a str,
    roots: &mut Vec<&'a str>,
    store: &mut HashMap<&'a str, TreeItem<'a, &'a str>>
) {

Notice that every string reference involved is &'a str; there are no &'b strs at all. This is a good sign — types should match, and lifetimes are parts of types, so this suggests consistency.

6 Likes
9

oh boy how embarrassing. Yes, thank you.

And you bring up a good point: why not allocate to the heap? Well, I've found it is much simpler but then when I eventually want to optimize it's a pretty big rewrite switching to memory on the stack. And I can't really think of guidelines for when heap allocation is no big deal without benchmarking, which requires writing the code 2 different ways.

So I've kinda resolved myself to just avoiding the heap when necessary as a general rule of thumb.

10

Same as Cerber-Ursi - that was an oversight on my part. Thanks for the hint about eliding though!

11

&str doesn’t mean not allocating. It means borrowing from some other allocation. That other allocation might be heap, stack, static, or something else, but we can rule out stack allocation for strings since they are (almost always) variable length.

So, you already have some allocation for each string. Unless all the &strs are borrowed from a single input string, using Rc<str> instead of &str is just a different choice of how to manage that the strings won’t be deallocated before they’re no longer in use, and usually a much easier one. The choice which would be high-allocation is using String everywhere, because you’d have to clone the strings a lot to build your data structure — but that’s not at all the only non-reference option.

The right choice depends on the use case:

  • If it is the case that all the &strs are borrowed from a single input string (which might be common for a compiler or some other kind of batch data processor), then keeping &'a str in your data structure is a good choice.
  • If the data structure is temporary, lasting only while a specific function runs and thrown out at the end, then keeping &'a str in your data structure is also a good idea then.
  • But if your strings are coming from interactive or streaming input (UI, API, etc) and the data structure is mutated as new input arrives, then trying to keep &'a str will in the end be impossible.[1]

So I've kinda resolved myself to just avoiding the heap when necessary as a general rule of thumb.

Doing this universally is a good way to create unsolvable problems for yourself, as a beginner still learning how to write lifetimes. Avoiding heap allocation is a fine plan when designing the input to a single function. It is, usually, not a good plan for your application’s primary data structures (unless your application is batch-oriented and keeps all input in memory).

  1. or rather, it forces you to leak memory to make it work ↩︎

7 Likes
12

Nice. Definitely my use-case is the former - building an AST from 10+ MB of string data.

13

In that case, here is a principle to follow: pick a specific lifetime name for use with your inputs (say 'input or 'inp depending on your preferences about verbosity), and use that lifetime name always for borrows from the input (and never for things that are shorter-lived than the input, but we already covered that). That is, write code that looks like:

enum Tree<'input> {
    String(&'input str),
    ...
}

pub fn build_top<'input>(
    top: &'input str,
    roots: &mut Vec<&'input str>,
    store: &mut HashMap<&'input str, Tree<'input>>
) {
    ...
}

This will make it easier to review, and maintain, because instead of 'a meaning something only from context, 'input tells you how to expect it should be used.

7 Likes
14

There's still something I'm missing...

How does this borrow of a Vec owned by the scope of its function live long enough?:

Won't it be dropped upon function return?

The only way I'd imagine this working is by calling leak() on items instead of borrowing it. Does my confusion make sense?

If I had to guess there's some special case with static lifetimes?

15

Do you mean, why does it work even though Tree<'a, _>::new requires a &'a [TreeItem<'a, _>]? If so, it's this scenario again:

It's not 'static being special here.

fn render(width: u16, height: u16, state: &mut TreeState<&'static str>) -> Buffer {
    let items: Vec<TreeItem<'static, &'static str>> = TreeItem::example();

    // Let's call  vv  this lifetime `'local`
    let tree: Tree<'_, &'static str> = Tree::new(&items).unwrap();
    //                                           ^^^^^^                  
    // Then we must have &'local [TreeItem<'local, &'static str>] here
    // `&'local [TreeItem<'static, &'static str>]` can coerce to
    // `&'local [TreeItem<'local, &'static str>]` because `TreeItem<'a, _>`
    // is *covariant* in `'a`.
    
    let area: Rect = Rect::new(0, 0, width, height);
    let mut buffer: Buffer = Buffer::empty(area);
    StatefulWidget::render(tree, area, &mut buffer, state);
    
    // We don't make use of any variables with `'local` in their
    // type after this point, so the `'local` borrow can expire
    
    buffer

    // All the non-returned variables can drop here; there are
    // no local borrows left
}

(Scratchpad.)

4 Likes