Allow *local* mutation and assignments in pattern guards

[Stebalien]

Due to rust-lang/rust#15989, it isn't possible to assign/mutate in pattern guards. However, assignment/mutation of variables local to the pattern guard should be perfectly safe.

As reported on reddit: https://pay.reddit.com/r/rust/comments/2ztiag/cannot_mutably_borrow_in_a_pattern_guard/ by qthree:

This code is failing: http://is.gd/cAYb5r But that's ok: http://is.gd/7pF28q

For completeness, the failing code is:

fn main() {
    let v = vec!["1".to_string(), "2".to_string(), "3".to_string()];
    let q = match Some(&v) {
        Some(iv) if iv.iter().any(|x| &x[..]=="2") => true,
        _ => false
    };
}

However, the following works:

fn main() {
    let v = vec!["1".to_string(), "2".to_string(), "3".to_string()];
    let q = match Some(&v) {
        Some(iv) if (|| iv.iter().any(|x| &x[..]=="2"))() => true,
        _ => false
    };
}

[lilyball]

The failing code fails with the following error:

unnamed.rs:4:21: 4:30 error: cannot mutably borrow in a pattern guard [E0301]
unnamed.rs:4         Some(iv) if iv.iter().any(|x| &x[..]=="2") => true,
                                 ^~~~~~~~~
error: aborting due to previous error

But there's no mutation here. Something seems rather confused.

[Stebalien]

`any()` is mutably borrowing `iv.iter()`.

[lilyball]

Oh I suppose it is. I was interpreting it as trying to mutably borrow the iv value, but it's obvious now what's going on.

[durka]

It would be really nice to make this check more discriminating. This code (nearly the same as @Stebalien's) shows why it's nonsensical:

#[inline(always)]
fn check<'a, I: Iterator<Item=&'a i32>>(mut i: I) -> bool {
    i.any(|&x| x == 2)
}

fn main() {
    let slice = [1, 2, 3];

    let x = match 42 {
        _ if slice.iter().any(|&x| x == 2) => { true },
        _ if check(slice.iter()) => { true },
        _ => { false }
    };
    println!("{}", x);
}

There's an error when the match checker can "see" the mutable borrow, but when the exact same code is "hidden" away in a function, it's allowed. It's a mutable borrow of a temporary, so there should be no problem.

[aidanhs]

It's a strange error to exist at all given how easy it is to subvert.

use std::cell::Cell;
enum A {
    X(Cell<u8>),
}
fn main() {
    let x = A::X(Cell::new(5));
    match x {
        A::X(ref b) if { b.set(6); b.get() == 6 } => (),
        _ => panic!(),
    }
}

It seems like something that would be better served as a warning if the intention is to prevent accidental mutation in pattern guards?

[comex]

Ping. I'm pretty sure this rule is essentially a compiler deficiency rather than anything with a principled reason to exist. The problems mentioned in #15989 would be solved by treating the expression being matched on as borrowed (in full) during the pattern guard. I suppose this would probably come for free with non-lexical lifetimes?...

cc @pcwalton

[pcwalton]

See @pnkfelix' comments in the original PR (#15989). It is something we should be able to relax.

[durka]

This just came up again on IRC. Is it something that's actually difficult to implement (so an RFC would require a lot of Detailed Design), or can the proposal simply be "turn off this part of the mutation check"?

[eddyb]

@durka It seems like a "MIR borrowck allows us to check complex interaction correctly" kind of issue.
cc @nikomatsakis

[nikomatsakis]

This is more or less a duplicate of rust-lang/rust#27282, rust-lang/rust#31287, etc. We plan to support mutation in match arms, we're working on making it sound. I think I'll just close the issue if that's ok =)