Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MIR borrowck: diagnostic for moves out of patterns should be improved #45699

Closed
arielb1 opened this issue Nov 1, 2017 · 2 comments
Closed

MIR borrowck: diagnostic for moves out of patterns should be improved #45699

arielb1 opened this issue Nov 1, 2017 · 2 comments
Assignees
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. NLL-diagnostics Working towards the "diagnostic parity" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@arielb1
Copy link
Contributor

arielb1 commented Nov 1, 2017

When a match expression or let statement is used on an lvalue with no ability to move things out, AST borrowck displays a single unified error message while MIR borrowck displays an error message for each binding:

#![feature(box_syntax)]

enum Foo {
    Foo1(Box<u32>, Box<u32>),
    Foo2(Box<u32>),
    Foo3,
}

fn blah() {
    let f = &Foo::Foo1(box 1, box 2);
    match *f {             //~ ERROR cannot move out of
                           //~| cannot move out
        Foo::Foo1(_num1,         //~ NOTE to prevent move
                  _num2) => (),  //~ NOTE and here
        Foo::Foo2(_num) => (),   //~ NOTE and here
        Foo::Foo3 => ()
    }
}

fn main() {}

Displays the following errors:

error[E0507]: cannot move out of borrowed content (Ast)
  --> x.rs:11:11
   |
11 |     match *f {             //~ ERROR cannot move out of
   |           ^^ cannot move out of borrowed content
12 |                            //~| cannot move out
13 |         Foo::Foo1(_num1,         //~ NOTE to prevent move
   |                   ----- hint: to prevent move, use `ref _num1` or `ref mut _num1`
14 |                   _num2) => (),  //~ NOTE and here
   |                   ----- ...and here (use `ref _num2` or `ref mut _num2`)
15 |         Foo::Foo2(_num) => (),   //~ NOTE and here
   |                   ---- ...and here (use `ref _num` or `ref mut _num`)

error[E0507]: cannot move out of borrowed_content (Mir)
  --> x.rs:13:19
   |
13 |         Foo::Foo1(_num1,         //~ NOTE to prevent move
   |                   ^^^^^ cannot move out of borrowed_content

error[E0507]: cannot move out of borrowed_content (Mir)
  --> x.rs:14:19
   |
14 |                   _num2) => (),  //~ NOTE and here
   |                   ^^^^^ cannot move out of borrowed_content

error[E0507]: cannot move out of borrowed_content (Mir)
  --> x.rs:15:19
   |
15 |         Foo::Foo2(_num) => (),   //~ NOTE and here
   |                   ^^^^ cannot move out of borrowed_content

error: aborting due to 4 previous errors

I think this is fairly low priority, but we do want error parity.

@arielb1 arielb1 added A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints WG-compiler-nll labels Nov 1, 2017
@arielb1 arielb1 changed the title MIR borrowck: diagnostic for match should be improved MIR borrowck: diagnostic for moves out of patterns should be improved Nov 1, 2017
@vramana
Copy link
Contributor

vramana commented Dec 12, 2017

I am working on this along with #46599

@XAMPPRocky XAMPPRocky added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Feb 12, 2018
@nikomatsakis nikomatsakis added the NLL-diagnostics Working towards the "diagnostic parity" goal label Apr 3, 2018
@nikomatsakis
Copy link
Contributor

I think that @matthewjasper has been working on this.

bors added a commit that referenced this issue Jun 29, 2018
[NLL] Better move errors

Make a number of changes to improve the quality of NLL cannot move errors.

* Group errors that occur in the same `match` with the same cause.
* Suggest `ref`, `&` or removing `*` to avoid the move.
* Show the place being matched on.

Differences from AST borrowck:

* `&` is suggested over `ref` when matching on a place that can't be moved from.
* Removing `*` is suggested instead of adding `&` when applicable.
* Sub-pattern spans aren't used, this would probably need Spans on Places.

Closes #45699
Closes #46627
Closes #51187
Closes #51189

r? @pnkfelix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. NLL-diagnostics Working towards the "diagnostic parity" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants