Collect move errors before reporting

[ktt3ja]

This commit changes the way move errors are reported when some value is
captured by a PatIdent. First, we collect all of the "cannot move out
of" errors before reporting them, and those errors with the same "move
source" are reported together. If the move is caused by a PatIdent (that
binds by value), we add a note indicating where it is and suggest the
user to put ref if they don't want the value to move. This makes the
"cannot move out of" error in match expression nicer (though the extra
note may not feel that helpful in other places :P). For example, with
the following code snippet,

enum Foo {
    Foo1(~u32, ~u32),
    Foo2(~u32),
    Foo3,
}

fn main() {
    let f = &Foo1(~1u32, ~2u32);
    match *f {
        Foo1(num1, num2) => (),
        Foo2(num) => (),
        Foo3 => ()
    }
}

Errors before the change:

test.rs:10:9: 10:25 error: cannot move out of dereference of `&`-pointer
test.rs:10         Foo1(num1, num2) => (),
                   ^~~~~~~~~~~~~~~~
test.rs:10:9: 10:25 error: cannot move out of dereference of `&`-pointer
test.rs:10         Foo1(num1, num2) => (),
                   ^~~~~~~~~~~~~~~~
test.rs:11:9: 11:18 error: cannot move out of dereference of `&`-pointer
test.rs:11         Foo2(num) => (),
                   ^~~~~~~~~

After:

test.rs:9:11: 9:13 error: cannot move out of dereference of `&`-pointer
test.rs:9     match *f {
                    ^~
test.rs:10:14: 10:18 note: attempting to move value to here (to prevent the move, use `ref num1` or `ref mut num1` to capture value by reference)
test.rs:10         Foo1(num1, num2) => (),
                        ^~~~
test.rs:10:20: 10:24 note: and here (use `ref num2` or `ref mut num2`)
test.rs:10         Foo1(num1, num2) => (),
                              ^~~~
test.rs:11:14: 11:17 note: and here (use `ref num` or `ref mut num`)
test.rs:11         Foo2(num) => (),
                        ^~~

Close #8064

[ktt3ja]

One caveat I have with this is, for the following function,

fn arg_item(&_x: &~str) {}

The reported error and note are

test.rs:20:14: 20:16 note: attempting to move value to here (to prevent the move, you can use `ref _x` to capture value by reference)
test.rs:20 fn arg_item(&_x: &~str) {}
                        ^~

The user may take it to mean "replace the function declaration with fn arg_item(ref _x: &~str) {}, which is not what is intended. I wonder if it would be better to change the parenthetical remark to "put ref before _x to capture value by reference".

Here are more comparisons of before and after. For the following file:

fn with(f: |&~str|) {}

fn arg_item(&_x: &~str) {}
    //~^ ERROR cannot move out of dereference of `&`-pointer

fn arg_closure() {
    with(|&_x| ())
    //~^ ERROR cannot move out of dereference of `&`-pointer
}

fn let_pat() {
    let &_x = &~"hi";
    //~^ ERROR cannot move out of dereference of `&`-pointer
}

// =================================================================

pub fn blah2() {
    use std::rc::Rc;
    let _x = *Rc::new(~"hi");
    //~^ ERROR cannot move out of dereference of `&`-pointer
}

// =================================================================

struct S {f:~str}
impl Drop for S {
    fn drop(&mut self) { println!("{}", self.f); }
}

fn move_in_match() {
    match S {f:~"foo"} {
        S {f:_s} => {}
        //~^ ERROR cannot move out of type `S`, which defines the `Drop` trait
    }
}

fn move_in_let() {
    let S {f:_s} = S {f:~"foo"};
    //~^ ERROR cannot move out of type `S`, which defines the `Drop` trait
}

fn move_in_fn_arg(S {f:_s}: S) {
    //~^ ERROR cannot move out of type `S`, which defines the `Drop` trait
}

// =================================================================

struct A {
    a: ~int
}

fn free<T>(_: T) {}

fn blah3() {
    let a = &A { a: ~1 };
    match a.a {
        n => { free(n) }
    }
    free(a)
}

// =================================================================

fn blah4() {
    let bar = ~3;
    let _g = || {
        let _h: proc() -> int = proc() *bar; //~ ERROR cannot move out of captured outer variable
    };
}

// =================================================================

fn main() {}

Before:

test.rs:3:13: 3:16 error: cannot move out of dereference of `&`-pointer
test.rs:3 fn arg_item(&_x: &~str) {}
                      ^~~
test.rs:7:11: 7:14 error: cannot move out of dereference of `&`-pointer
test.rs:7     with(|&_x| ())
                    ^~~
test.rs:12:9: 12:12 error: cannot move out of dereference of `&`-pointer
test.rs:12     let &_x = &~"hi";
                   ^~~
test.rs:20:14: 20:29 error: cannot move out of dereference of `&`-pointer
test.rs:20     let _x = *Rc::new(~"hi");
                        ^~~~~~~~~~~~~~~
test.rs:33:9: 33:17 error: cannot move out of type `S`, which defines the `Drop` trait
test.rs:33         S {f:_s} => {}
                   ^~~~~~~~
test.rs:39:9: 39:17 error: cannot move out of type `S`, which defines the `Drop` trait
test.rs:39     let S {f:_s} = S {f:~"foo"};
                   ^~~~~~~~
test.rs:43:19: 43:27 error: cannot move out of type `S`, which defines the `Drop` trait
test.rs:43 fn move_in_fn_arg(S {f:_s}: S) {
                             ^~~~~~~~
test.rs:57:11: 57:14 error: cannot move out of dereference of `&`-pointer
test.rs:57     match a.a {
                     ^~~
test.rs:68:33: 68:44 error: cannot move out of captured outer variable
test.rs:68         let _h: proc() -> int = proc() *bar; //~ ERROR cannot move out of captured outer variable
                                           ^~~~~~~~~~~

After:

test.rs:3:13: 3:16 error: cannot move out of dereference of `&`-pointer
test.rs:3 fn arg_item(&_x: &~str) {}
                      ^~~
test.rs:3:14: 3:16 note: attempting to move value to here (to prevent the move, you can use `ref _x` to capture value by reference)
test.rs:3 fn arg_item(&_x: &~str) {}
                       ^~
test.rs:7:11: 7:14 error: cannot move out of dereference of `&`-pointer
test.rs:7     with(|&_x| ())
                    ^~~
test.rs:7:12: 7:14 note: attempting to move value to here (to prevent the move, you can use `ref _x` to capture value by reference)
test.rs:7     with(|&_x| ())
                     ^~
test.rs:12:9: 12:12 error: cannot move out of dereference of `&`-pointer
test.rs:12     let &_x = &~"hi";
                   ^~~
test.rs:12:10: 12:12 note: attempting to move value to here (to prevent the move, you can use `ref _x` to capture value by reference)
test.rs:12     let &_x = &~"hi";
                    ^~
test.rs:20:14: 20:29 error: cannot move out of dereference of `&`-pointer
test.rs:20     let _x = *Rc::new(~"hi");
                        ^~~~~~~~~~~~~~~
test.rs:20:9: 20:11 note: attempting to move value to here (to prevent the move, you can use `ref _x` to capture value by reference)
test.rs:20     let _x = *Rc::new(~"hi");
                   ^~
test.rs:33:9: 33:17 error: cannot move out of type `S`, which defines the `Drop` trait
test.rs:33         S {f:_s} => {}
                   ^~~~~~~~
test.rs:33:14: 33:16 note: attempting to move value to here (to prevent the move, you can use `ref _s` to capture value by reference)
test.rs:33         S {f:_s} => {}
                        ^~
test.rs:39:9: 39:17 error: cannot move out of type `S`, which defines the `Drop` trait
test.rs:39     let S {f:_s} = S {f:~"foo"};
                   ^~~~~~~~
test.rs:39:14: 39:16 note: attempting to move value to here (to prevent the move, you can use `ref _s` to capture value by reference)
test.rs:39     let S {f:_s} = S {f:~"foo"};
                        ^~
test.rs:43:19: 43:27 error: cannot move out of type `S`, which defines the `Drop` trait
test.rs:43 fn move_in_fn_arg(S {f:_s}: S) {
                             ^~~~~~~~
test.rs:43:24: 43:26 note: attempting to move value to here (to prevent the move, you can use `ref _s` to capture value by reference)
test.rs:43 fn move_in_fn_arg(S {f:_s}: S) {
                                  ^~
test.rs:57:11: 57:12 error: cannot move out of dereference of `&`-pointer
test.rs:57     match a.a {
                     ^
test.rs:58:9: 58:10 note: attempting to move value to here (to prevent the move, you can use `ref n` to capture value by reference)
test.rs:58         n => { free(n) }
                   ^
test.rs:68:33: 68:44 error: cannot move out of captured outer variable
test.rs:68         let _h: proc() -> int = proc() *bar; //~ ERROR cannot move out of captured outer variable
                                           ^~~~~~~~~~~

[huonw]

Does this attempt to work out if ref mut is appropriate for the "to prevent the move..." bit? (If not, maybe ref mut should be mentioned too.)

[ktt3ja]

@huonw: I will change the message to mention ref mut, too, when I get back to working on this later. It just didn't occur to me, so thanks!

[ktt3ja]

Hmm, sadly, when we are matching against a vector, the error-reporting behavior is inconsistent with the rest (the error is on the arm instead of the expression we are matching against):

fn c() {
    let mut vec = vec!(~1, ~2, ~3);
    let vec: &mut [~int] = vec.as_mut_slice();
    match vec {
        [_a, ..b] => {}
        _ => {}
    }
}

test.rs:5:9: 5:18 error: cannot move out of dereference of `&mut`-pointer
test.rs:5         [_a, ..b] => {}
                  ^~~~~~~~~
test.rs:5:10: 5:12 note: attempting to move value to here (to prevent the move, you can use `ref _a` to capture value by reference)
test.rs:5         [_a, ..b] => {}
                   ^~

[brson]

I love these error reporting improvements. 👍

[ktt3ja]

The span note has been changed from:

attempting to move value to here (to prevent the move, you can use `ref num1` to capture value by reference)

to

attempting to move value to here (to prevent the move, use `ref num1` or `ref mut num1` to capture value by reference)