[WIP] Suggest use mut when mutating a non mutable local

[spastorino]

Closes #51031

There's some things that are wrong in this PR, opened to start discussing with code.

Check-list:

• no suggestions for ref mut
• check on suggestions for other cases ({ let x = 3; x = 4; })
  • oh, that seems to generate a slightly different message...maybe that's ok?
  • { let x = (3, 4); x.0 = 5; } generates a sort-of strange message maybe? (playground — compare NLL and not NLL)
• suggestions for upvars (let x = 3; let c = || { &mut x; }) (playground)

[rust-highfive]

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

[spastorino]

r? @nikomatsakis

[pnkfelix]

yeah we definitely don't want to suggest this...

[spastorino]

Yes, need to look at this more carefully.

[rust-highfive]

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). 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.

[00:04:52] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:53] tidy error: /checkout/src/librustc_mir/borrow_check/mod.rs:1721: line longer than 100 chars
[00:04:53] tidy error: /checkout/src/librustc_mir/borrow_check/mod.rs:1723: line longer than 100 chars
[00:04:54] some tidy checks failed
[00:04:54] 
[00:04:54] 
[00:04:54] 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:54] 
[00:04:54] 
[00:04:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:54] Build completed unsuccessfully in 0:01:46
[00:04:54] Build completed unsuccessfully in 0:01:46
[00:04:54] make: *** [tidy] Error 1
[00:04:54] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0079fecc
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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. (Feature Requests)

[nikomatsakis]

Here we should use the span_suggestion method. Something like this (I took the prompt etc from AST borrowck):

let local_name = local_decl.name.unwrap();
err.span_suggestion(
    local_decl.source_info.span,
    &format!("consider making `{}` mutable", local_name),
    format!("mut {}", local_name),
);

This will encode the suggestion into the JSON etc so that tools like rustfix or IDEs can apply it automatically.

[nikomatsakis]

Also, I was noting on Zulip at 4:40pm (ET) yesterday, I think we want to move this suggestion code so that it comes after the match. That will require restructuring this method somewhat. The problem with the current setup is that your suggestion will only fire when you are trying to borrow something, but not — I think — in tests like let x = 3; x = 4; (try it!). I would combine it with some of the other suggestion-like code in the method too, yielding something like this:

let mut err = match kind {
  Reservation(...) | Write(...) => {
    if no error { return } else { construct_err_object() }
  }
  Reservation(...) | Write(...) => /* some variant of the preivous arm */,
  Reservation(...) | Write(...) => ...,
};

// If we get here, then there *is* an error
match place_err {
    Place::Local(l) => {
        // suggest changing to `mut` variable
    }
    Place::Projection(...) => {
        // suggest changing to `&mut` borrow (see [link 1])
    }
}

err.emit()

link 1 is

rust/src/librustc_mir/borrow_check/mod.rs

Lines 1723 to 1728 in 61f35e5

	let err_info = if let Place::Projection(
	box Projection {
	base: Place::Local(local),
	elem: ProjectionElem::Deref
	}
	) = *place_err {

[nikomatsakis]

We are also going to want to watch out for "upvars" — those are cases where the place_err will not be a Place::Local but we want to make a similar suggestion. We can discuss on Zulip though, I'm just going to edit your PR description to have a check-list so things don't get forgotten.

[nikomatsakis]

I think I am going to close this in favor of @pnkfelix's PR #51275, which — if I am not mistaken — subsumes this work and handles some of my suggestions.