Is "this represents potential undefined behavior in your code" error when using a match arm guard a bug in the compiler?

Question

I'm having an issue with a match expression in my code raising a warning when a guard is included. I believe that this warning has to do with the non-lexical lifetimes used by the borrow checker. My function either returns a mutable reference to an item from a collection, or a clone of the whole collection.

#[derive(Debug, Clone)]
enum Value {
    Int(i32),
    List(Vec<Value>),
}

#[derive(Debug)]
struct Error(&'static str, Value);

fn main() {
    let mut value = Value::List(vec![
        Value::Int(1),
        Value::Int(2),
        Value::Int(34),
        Value::Int(12),
    ]);
    let y = index_list(&mut value, 2);

    let _ = dbg!(y);
}

fn index_list<'a>(value: &'a mut Value, idx: usize) -> Result<&'a mut Value, Error> {
    match *value {
        Value::List(ref mut list) if idx < list.len() => Ok(&mut list[idx]),
        Value::List(_) => Err(Error("index out of range", value.clone())),
        _ => Err(Error("tried to index int", value.clone())),
    }
}

playground

It compiles and runs, but I get a very ominous looking warning:

warning[E0502]: cannot borrow `*value` as immutable because it is also borrowed as mutable
  --> src/main.rs:25:59
   |
22 | fn index_list<'a>(value: &'a mut Value, idx: usize) -> Result<&'a mut Value, Error> {
   |               -- lifetime `'a` defined here
23 |     match *value {
24 |         Value::List(ref mut list) if idx < list.len() => Ok(&mut list[idx]),
   |                     ------------                         ------------------ returning this value requires that `value.0` is borrowed for `'a`
   |                     |
   |                     mutable borrow occurs here
25 |         Value::List(_) => Err(Error("index out of range", value.clone())),
   |                                                           ^^^^^ immutable borrow occurs here
   |
   = warning: this error has been downgraded to a warning for backwards compatibility with previous releases
   = warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future

warning[E0502]: cannot borrow `*value` as immutable because it is also borrowed as mutable
  --> src/main.rs:26:46
   |
22 | fn index_list<'a>(value: &'a mut Value, idx: usize) -> Result<&'a mut Value, Error> {
   |               -- lifetime `'a` defined here
23 |     match *value {
24 |         Value::List(ref mut list) if idx < list.len() => Ok(&mut list[idx]),
   |                     ------------                         ------------------ returning this value requires that `value.0` is borrowed for `'a`
   |                     |
   |                     mutable borrow occurs here
25 |         Value::List(_) => Err(Error("index out of range", value.clone())),
26 |         _ => Err(Error("tried to index int", value.clone())),
   |                                              ^^^^^ immutable borrow occurs here
   |
   = warning: this error has been downgraded to a warning for backwards compatibility with previous releases
   = warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future

I don't understand why the Err(value.clone()) line requires that the Ok(&mut ...) borrow still be active, because they are mutually exclusive and both result in a function return. This warning goes away if I remove the guard on the first match arm, but I need that guard to be there. Is this a bug in the NLL system? I never had this issue with the old borrowck. How can I make this better?

Answer 1

This error looks like a limitation of the borrow checker to me, similar to Double mutable borrow error in a loop happens even with NLL on. I can't see how this could lead to a soundness hole, and believe the code is safe.

When using an if statementat instead of match, the warning can be avoided, and the code becomes more readable as well:

fn bar<'a>(
    map: &'a mut HashMap<String, Vec<i32>>,
    name: &str,
    idx: usize,
) -> Result<&'a mut i32, Vec<i32>> {
    let value = map.get_mut(name).unwrap();
    if idx < value.len() {
        Ok(&mut value[idx])
    } else {
        Err(value.clone())
    }
}