Modify items in HashMap with items already in it (Error: E0502)

Here is a simplified example of what I want to do, but essentially, I want to modify items in a HashMap with data from other items in the same HashMap.

This means I have to do call HashMap::get_mut to get reference to the item to modify and then a 2nd call to HashMap::get to get the item which to modify the 1st one.

let mut task_map: HashMap<u32, String> = HashMap::new();
    let id = 10;
    
    if let Some(string1) = task_map.get_mut(&id) {
        for id in 0..10 {
            if let Some(string2) = task_map.get(&id) {
                // Just an example. The intension is to modify the 
                // item from 1st 'get_mut' (string1) with items from
                // 2nd 'get' (string2).
                string1.push_str(&string2);
            }
        }
    }

But because the mutable reference is still in scope I get the following error:

error[E0502]: cannot borrow `task_map` as immutable because it is also borrowed as mutable
  --> src/main.rs:9:36
   |
7  |     if let Some(string1) = task_map.get_mut(&id) {
   |                            -------- mutable borrow occurs here
8  |         for id in 0..10 {
9  |             if let Some(string2) = task_map.get(&id) {
   |                                    ^^^^^^^^ immutable borrow occurs here
...
13 |                 string1.push_str(&string2);
   |                 ------- mutable borrow later used here

For more information about this error, try `rustc --explain E0502`

Require some help to understand what to solve this.

get_many_mut on HashMap is currently nightly, but does exactly what you need (and will probably soon be stable, if i read the tracking issue correctly).

If you don't want to use nightly you would have to do the borrows after each other and somehow remember the other data.

1 Like

This has overhead, but it will make the borrow checker happy:

let string1 = task_map.remove(id):
// modify
task_map.insert(id, string1);

Another option could be to use shared get() instead of exclusive get_mut, and wrap all strings in a RefCell.

Another option is to use hashbrown that has more flexible API:

5 Likes

Thanks! But I still have some questions. I used your first suggestion to use remove then insert and worked for the original example with String type. But fails if I use a more specific type. I am unable to see why

Just for reference, here is code as per your suggestion for String:

fn main() {
    let mut task_map: HashMap<u32, String> = HashMap::new();
    let id = 10;
    
    if let Some(mut  string1) = task_map.remove(&id) {
        for id in 0..10 {
            if let Some(string2) = task_map.get(&id) {
                string1.push_str(&string2);
            }
        }
        task_map.insert(id, string1);
    }
}

Playground: Rust Playground

Here is the code which does not compile. It's again similar to the above one but has a different type.

struct SomeFoo<'a>(Vec<&'a SomeFoo<'a>>);

fn main() {
    let mut task_map: HashMap<u32, SomeFoo> = HashMap::new();
    let id = 10;
    
    if let Some(mut item1) = task_map.remove(&id) {
        for id in 0..10 {
            if let Some(item2) = task_map.get(&id) {
                item1.0.push(item2);
            }
        }
        task_map.insert(id, item1);
    }
}

Playground: Rust Playground

The error I get is similar to the original one

error[E0502]: cannot borrow `task_map` as mutable because it is also borrowed as immutable
  --> src/main.rs:15:9
   |
11 |             if let Some(item2) = task_map.get(&id) {
   |                                  -------- immutable borrow occurs here
...
15 |         task_map.insert(id, item1);
   |         ^^^^^^^^^------^^^^^^^^^^^
   |         |        |
   |         |        immutable borrow later used by call
   |         mutable borrow occurs here

Thanks

The type of task_map in the latter example is HashMap<u32, SomeFoo<'_>>. The elided lifetime is the same for every value in the map. Removing a SomeFoo<'_> value doesn't transfer ownership like the String value does (which is why that workaround is valid in the first place).

In this case, RefCell might be appropriate.

I added a silly initialization and Debug impl so you can see it actually running instead of compiling a no-op just to pass the borrow checker.


Forgot to mention: I normally recommend against deriving Debug for potentially cyclic data structures like this. It can be implemented safely by hand to avoid infinite loops. But not with derive. In this case, I know it isn't going to print a cycle.

2 Likes

I think the error is because you're storing a reference in the map value, because SomeFoo contains a reference. This means that removing the item from the map causes the same borrowing problem as before, because a reference is returned when you remove the map entry.

I'm trying to understand what you're really trying to do, and of course the code is probably not a complete representation of what you're really doing. What's strange is that you're storing a reference to SomeFoo inside SomeFoo -- where is that SomeFoo value stored, the one that the reference refers to?

It's possible you're using references incorrectly. Normally when storing references in a struct, the reference refers to a value that is in scope. (Although there are exceptions, such as when the references have a 'static lifetime or when they are allocated using an arena.)

EDIT: @parasyte's solution is probably what you're really trying to do.

It's treating the HashMap as "basically an arena" already. But doing so with &'a Foo<'a> limits its usage to shared-borrows only.

1 Like

Got it, thanks!

I need to make a correction. Upon reflection, this is not what I meant to say.

What is happening here is that the &’a SomeFoo<'a> references prevent moving because it may be referenced elsewhere in the “arena”. This is a lot like the problem that Pin solves. The HashMap cannot be moved, nor any of its values as that would invalidate self-references.

There is something bothering me about the solution, although I can't articulate it exactly. I don't see how the RefCell can solve the borrowing problem completely, and I wonder if the &’a SomeFoo<'a> can still cause problems.

Following my intuition (all I've got, since I don't thoroughly understand lifetimes like some do here) I added another insertion just before the println and this causes a borrowing error.

error[E0502]: cannot borrow `task_map` as mutable because it is also borrowed as immutable
  --> src/main.rs:24:5
   |
18 |             if let Some(item2) = task_map.get(&id) {
   |                                  -------- immutable borrow occurs here
...
24 |     task_map.insert(12, SomeFoo(RefCell::default()));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
25 |
26 |     println!("{:#?}", task_map.get(&id).unwrap());
   |                       -------- immutable borrow later used here

playground

EDIT: One thing that bothered me is that I've never seen a HashMap used as an arena like this. And if that really works, it would be a very useful tool! But it seems too good to be true.

I also do not know how it's a full solution. Part of the problem with answering questions like this on a forum is that they are sometimes minimized too much (as we saw here) and then expanded with details that actually matter, but not enough to get the full picture of how the situation came about in the first place. Classic XY problem stuff.

The RefCell makes the lifetime invariant [1] on the cyclic loans. And invariant shared references have some limitations (only shared access is available, no moving, no non-trivial destructors) but they are actually ergonomic to use, as long as you can avoid the potholes.

The error you showed is just that: taking a mutable reference when only shared references are allowed. At that point, the invariant shared lifetime has been released into the wild, and there is no putting it back in the bottle. It's "shared forever".

To mutate the map after this point of no return, it needs interior mutability. Replacing HashMap with FrozenMap is a potential solution, here. (The values need to be boxed, per the FrozenMap constraints.)

If you would like to learn more about arenas, you might be interested in Arenas in Rust - In Pursuit of Laziness


  1. I was reminded of this fact when corrected in Incremental borrow accumulation = anti-pattern? - #8 by programmerjake. ↩︎

3 Likes

In this playground, one has a HashMap<_, SomeFoo<'a>> and needs to .get a &'a SomeFoo<'a>, which requires borrowing the HashMap for 'a. It's a borrowing forever situation -- of the shared variety, but that still prevents getting a &mut HashMap later.

RefCell avoids the need for a &mut HashMap -- provided you never need to insert or remove elements, etc.

It would also not work if the std collections weren't magic utilizing unstable functionality to unsafely tune the borrow checking of drops, because without that, being borrowed forever conflicts with the drop. For example.

3 Likes