Provide better borrow checker error message

[klnusbaum]

Fix for issue #45392

Given incorrect code like:

fn foo(a: &mut Bar) {}

fn baz(b: &mut Bar) {
    foo(&mut b)
}

Rust suggests:

fn baz(b: &mut Bar) {
       - consider changing this to `mut b`

This isn't the best suggestion in this case. We should instead have a suggestion that informs the user they should change their code to look like:

fn baz(b: &mut Bar) {
    foo(b)
}

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[klnusbaum]

Hey @estebank .

This code has a lot of dog science in it (i.e. I had very little idea of what I'm doing). There are a few things I'm not sure on:

1. In the description of this issue you said: "we must verify that the mutability (the mut arg in ty::BindValue(mut), maybe) and check if the first field in the tuple returned local_ty is Some(ty.node == TyPtr(mut)) (definitely). If ty.node is a mutable reference, then the error span should receive a suggestion". I'm not exactly sure what ty::BindValue is, so I assumed you mean ty::BindByValue. I'm also a little unclear as to what mut is and what I'm supposed to do with it. I was guessing I needed to do some sort of compare between the Mutability arg in TyPtr and BindByValue. So I added a check for that.
2. The nesting here has gotten pretty deep. I'm guessing I've done something wrong here.
3. The error span to pass to note_immutability_blame was pretty clear when it's being called in report_bckerror. However, it was less clear what to do in report_aliasability_violation. Please advise.
4. I have no idea how to write a proper test for this. Please advise.

[estebank]

1. In the description of this issue you said: "we must verify that the mutability (the mut arg in ty::BindValue(mut), maybe) and check if the first field in the tuple returned local_ty is Some(ty.node == TyPtr(mut)) (definitely). If ty.node is a mutable reference, then the error span should receive a suggestion". I'm not exactly sure what ty::BindValue is, so I assumed you mean ty::BindByValue.

Your assumption was correct.

I'm also a little unclear as to what mut is and what I'm supposed to do with it. I was guessing I needed to do some sort of compare between the Mutability arg in TyPtr and BindByValue. So I added a check for that.

mut in my example is the ty_ptr_mutability in your code.

2. The nesting here has gotten pretty deep. I'm guessing I've done something wrong here.

That sometimes happens, specially when working with very specific diagnostics. There are some things that can be done to reduce nesting, like the following:

match (x, y) {
    (Some(x), Some(y)) if x == y => { /* */ }
    _ => {}
}

instead of

if let Some(x) = x {
    if let Some(y) = y {
        if x == y { /* */ }
    }
}
match (x, y) {
    (Some(x), Some(y)) if x == y => { /* */ }
    _ => {}
}

but that depends a lot on the code being written.

3. The error span to pass to note_immutability_blame was pretty clear when it's being called in report_bckerror. However, it was less clear what to do in report_aliasability_violation. Please advise.

I'll take a look at it. Have to explore the code for it.

4. I have no idea how to write a proper test for this. Please advise.

Create a file in src/test/ui and run ./x.py test src/test/ui --stage 1. Detailed explanations in COMPILER_TESTS.

[pietroalbini]

Ping from the triage team @klnusbaum, and thanks for this PR! The reviewer left a few comments, could you address them so we can move forward with this?

Also, your PR is failing a few tidy tests, but they should be really easy to fix :)

tidy error: /checkout/src/librustc_borrowck/borrowck/mod.rs:1196: line longer than 100 chars
tidy error: /checkout/src/librustc_borrowck/borrowck/mod.rs:1199: line longer than 100 chars

[klnusbaum]

Hi @estebank I'll try to take a look at this in the next few days :)

[estebank]

Do not hesitate to ping me here or in gitter, but I might be slow to answer depending on availability.

Also, you have https://forge.rust-lang.org/x-py.html available in case you need a reference for the build tooling.

[klnusbaum]

started working on this again. Hope to have something new by the end of the weekend.

[klnusbaum]

alright, I've actually got a test going now. I've restructured some of the code but I've run into an interesting compile error that I'm not sure how best to fix. Could I get some advice here? The error is:

error[E0008]: cannot bind by-move into a pattern guard
    --> librustc_borrowck/borrowck/mod.rs:1195:73
     |
1195 |                             (hir::Ty_::TyPtr(ref ty_ptr_mutability), Ok(snippet)) if ty_ptr_mutability.mutbl == bind_value_mut => {
     |                                                                         ^^^^^^^ moves value into pattern guard

error: aborting due to previous error

error: Could not compile `rustc_borrowck`.

[TimNN]

Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

Resolving deltas: 100% (609997/609997), completed with 4802 local objects.
---
[00:00:46] configure: rust.quiet-tests     := True
---
[00:04:19] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:19] expected success, got: exit code: 1
[00:04:19]
[00:04:19]
[00:04:19] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:19] Build completed unsuccessfully in 0:01:44
[00:04:19] Makefile:79: recipe for target 'tidy' failed
[00:04:19] make: *** [tidy] Error 1

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN.

[shepmaster]

Ping from triage, @klnusbaum ! Are you still working on this?

cannot bind by-move into a pattern guard

In general, you have to reorganize the code. Sometimes you need to move the guard to a "normal" if statement inside the arm.

[pietroalbini]

Ping from triage @klnusbaum! We haven't heard from you in a while, are you still working on this?

[klnusbaum]

I've paused working on this for a bit (I'm trying to ramp up my Rust knowledge a little more). If this is urgent, someone else may want to take it over.

[pietroalbini]

Thanks for the PR @klnusbaum! Since there was no activity for more than two weeks on this, I'm closing the PR to keep things tidy. Don't worry though, if you'll have time to work on this again in the future please reopen the PR! We'll be happy to review it then!