HashMap and_modify / or_insert not treated as exclusively alternate code paths

help
1

When getting a hash map Entry, the and_modify and or_insert methods are alternate code paths which cannot both occur. However, the compiler seems to not factor this into its ownership logic, because it prevents a non-copyable element from being moved into the hash map from either of the possible paths:

use std::collections::HashMap;
struct NonCopyable {
    name: String,
}

pub fn main() {
    let mut db = HashMap::<i32, NonCopyable>::new();
    let nc = NonCopyable { name: "foo".to_string() };
    db.entry(0).and_modify(|value| (*value) = nc).or_insert(nc);
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0382]: use of moved value: `nc`
 --> src/main.rs:9:61
  |
8 |     let nc = NonCopyable { name: "foo".to_string() };
  |         -- move occurs because `nc` has type `NonCopyable`, which does not implement the `Copy` trait
9 |     db.entry(0).and_modify(|value| (*value) = nc).or_insert(nc);
  |                            -------            --            ^^ value used here after move
  |                            |                  |
  |                            |                  variable moved due to use in closure
  |                            value moved into closure here

For more information about this error, try `rustc --explain E0382`.
error: could not compile `playground` due to previous error

This can be circumvented by having the type be Clone-able and cloning it in the and_modify closure, so that it can be moved in or_insert branch. However, this seems like it should be unnecessary. In practice, my non-copyable type would be more expensive to clone than just a single string. How can I move it into the map once in either of the update or insert cases? Should I match on the Entry to handle the branching Occupied and Vacant cases?

2

It can't! You are calling both methods – the compiler has no way of knowning that only one of them will actually be needed at runtime.

The "ownership logic" is not magic – it can only operate on information that function signatures contain, and the compiler intentionally doesn't perform global analysis. Both the closure argument of and_modify() and the argument of or_insert() move the value, and since function bodies must be compile-able separately, both arguments will cause the moved nc value to be dropped, regardless of the fact that one of the methods will not be useful dynamically.

Your code is equivalent with db.insert(0, nc).

7 Likes
3

In a case where you need the compiler to be able to see that the two branches are mutually exclusive, use a match on the Entry enum:

use std::collections::{hash_map::Entry, HashMap};

match db.entry(0) {
    Entry::Occupied(mut oe) => {
        oe.insert(nc);
    }
    Entry::Vacant(ve) => {
        ve.insert(nc);
    }
}

In this example, entry() is unnecessary as already mentioned, but this is how you'd handle it in the general case where you want to run different code depending on whether there is an existing entry.

4 Likes