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

NLL fails to suggest using ref when a pattern is otherwise an impossible move #51189

Closed
2 tasks
pnkfelix opened this issue May 29, 2018 · 1 comment
Closed
2 tasks
Assignees
Labels
NLL-diagnostics Working towards the "diagnostic parity" goal

Comments

@pnkfelix
Copy link
Member

pnkfelix commented May 29, 2018

In the case of ui/borrowck/borrowck-move-error-with-note.rs, the AST borrowck output (borrowck-move-error-with-note.stderr) includes suggestions like this:

LL |         Foo::Foo1(num1,
   |                   ---- hint: to prevent move, use `ref num1` or `ref mut num1`

but NLL (borrowck-move-error-with-note.nll.stderr) presents no such suggestion for errors it discovers in the MIR for match patterns.

Here are all the tests that I think involve this:

@pnkfelix
Copy link
Member Author

pnkfelix commented Jun 4, 2018

I suspect a good solution to this might tie into #51275, but I have not yet confirmed that. (The current form of that PR does not yet address any of the bugs above, but that might just be a matter of adding new cases to the match that PR is adding.)

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
NLL-diagnostics Working towards the "diagnostic parity" goal
Projects
None yet
Development

No branches or pull requests

3 participants