Better suggested fix available for passing &mut (&mut val)

[FauxFaux]

Given code like:

fn foo(a: &mut Bar) {}

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

rustc correctly points out that you "cannot borrow immutable argument b as mutable", and offers a fix of:

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

A better fix, in this case (and many others where I make this error), is to simply drop the &mut:

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

I hit this error quite frequently, and remember finding the addition and removal of &mut until the compiler shut up about syntactic noise that I didn't care about (etc. etc.) pretty confusing; especially around Write.

---

Full code as a playground: https://play.rust-lang.org/?gist=9b69414d92287359bd6f1b91a53b254a&version=stable

If anyone can think of a better way to phrase the subject of this issue, please do change it.

[Freyskeyd]

I also found this disturbing.

I'm ok to work on a fix or an improvement of the suggestion.

If someone can mentor it... :)

[Mark-Simulacrum]

cc @estebank @rust-lang/compiler - could someone provide mentoring instructions?

[estebank]

I believe that in the following code:

rust/src/librustc_borrowck/borrowck/mod.rs

Lines 1208 to 1223 in 3dfbc88

	Some(ImmutabilityBlame::ImmLocal(node_id)) => {
	let let_span = self.tcx.hir.span(node_id);
	if let ty::BindByValue(..) = self.local_binding_mode(node_id) {
	if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) {
	let (_, is_implicit_self) = self.local_ty(node_id);
	if is_implicit_self && snippet != "self" {
	// avoid suggesting `mut &self`.
	return
	}
	db.span_label(
	let_span,
	format!("consider changing this to `mut {}`", snippet)
	);
	}
	}
	}

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, instead of the current one. In order to do this, you have to change the signature for note_immutability_blame to also accept the error span.

@Freyskeyd apologies for not having seen this before. If you're still interested, feel free to reach out again here or in gitter! The amount of code shouldn't be too much, but there's a bunch of machinery you'd get familiar with (the DiagnosticBuilder and the node tree).

I believe the output should resemble the following (by using db.span_suggestion(sp, msg, snippet):

error[E0596]: cannot borrow immutable argument `b` as mutable
 --> src/main.rs:4:14
  |
4 |     foo(&mut b)
  |              ^ cannot borrow mutably
help: consider removing the `&mut`, as it is an immutable binding to a mutable reference
4 |     foo(b)
  |         ^

[Freyskeyd]

@estebank I'm interested, I will work on it next week.

[klnusbaum]

@estebank This seemed kinda stale so I decided to try my hand at a fix. I opened a PR. I have a lot of questions, but I'll ask those in the PR.

[ytausky]

Since @klnusbaum mentioned on #48708 that they're dropping this issue, I would like to do it.

[ytausky]

I think this issue can be closed now -- what's the process to do that?