From e4368de7b1a70ca2c9f140570fdbbe974c737bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 17 Mar 2021 18:45:49 -0700 Subject: [PATCH 1/3] Suggestion for call on immutable binding of mutable type When calling a method requiring a mutable self borrow on an inmutable to a mutable borrow of the type, suggest making the binding mutable. Fix #83241. --- .../diagnostics/mutability_errors.rs | 60 +++++++++++++++-- src/test/ui/borrowck/mut-borrow-of-mut-ref.rs | 28 +++++++- .../ui/borrowck/mut-borrow-of-mut-ref.stderr | 64 ++++++++++++++++--- src/test/ui/did_you_mean/issue-31424.stderr | 28 ++++++-- src/test/ui/did_you_mean/issue-34126.stderr | 14 +++- src/test/ui/nll/issue-51191.stderr | 28 ++++++-- 6 files changed, 188 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 7be3f541487..f8a1b05f063 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -5,7 +5,10 @@ use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::{ hir::place::PlaceBase, - mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, LocalKind, Location}, + mir::{ + self, BindingForm, ClearCrossCrate, ImplicitSelfKind, Local, LocalDecl, LocalInfo, + LocalKind, Location, + }, }; use rustc_span::source_map::DesugaringKind; use rustc_span::symbol::{kw, Symbol}; @@ -241,13 +244,56 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { .map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local])) .unwrap_or(false) => { + let decl = &self.body.local_decls[local]; err.span_label(span, format!("cannot {ACT}", ACT = act)); - err.span_suggestion( - span, - "try removing `&mut` here", - String::new(), - Applicability::MaybeIncorrect, - ); + if let Some(mir::Statement { + source_info, + kind: + mir::StatementKind::Assign(box ( + _, + mir::Rvalue::Ref( + _, + mir::BorrowKind::Mut { allow_two_phase_borrow: false }, + _, + ), + )), + .. + }) = &self.body[location.block].statements.get(location.statement_index) + { + match decl.local_info { + Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByValue(Mutability::Not), + opt_ty_info: Some(sp), + opt_match_place: _, + pat_span: _, + }, + )))) => { + err.span_note(sp, "the binding is already a mutable borrow"); + } + _ => { + err.span_note( + decl.source_info.span, + "the binding is already a mutable borrow", + ); + } + } + err.span_help(source_info.span, "try removing `&mut` here"); + } else if decl.mutability == Mutability::Not + && !matches!( + decl.local_info, + Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf( + ImplicitSelfKind::MutRef + )))) + ) + { + err.span_suggestion_verbose( + decl.source_info.span.shrink_to_lo(), + "consider making the binding mutable", + "mut ".to_string(), + Applicability::MachineApplicable, + ); + } } // We want to suggest users use `let mut` for local (user diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs index 3f092846dd4..7cdb16b282d 100644 --- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs @@ -2,12 +2,36 @@ #![crate_type = "rlib"] pub fn f(b: &mut i32) { - g(&mut b); + //~^ NOTE the binding is already a mutable borrow + //~| NOTE the binding is already a mutable borrow + h(&mut b); //~^ ERROR cannot borrow + //~| NOTE cannot borrow as mutable //~| HELP try removing `&mut` here g(&mut &mut b); //~^ ERROR cannot borrow + //~| NOTE cannot borrow as mutable //~| HELP try removing `&mut` here } -pub fn g(_: &mut i32) {} +pub fn g(b: &mut i32) { //~ NOTE the binding is already a mutable borrow + h(&mut &mut b); + //~^ ERROR cannot borrow + //~| NOTE cannot borrow as mutable + //~| HELP try removing `&mut` here +} + +pub fn h(_: &mut i32) {} + +trait Foo { + fn bar(&mut self); +} + +impl Foo for &mut String { + fn bar(&mut self) {} +} + +pub fn baz(f: &mut String) { //~ HELP consider making the binding mutable + f.bar(); //~ ERROR cannot borrow `f` as mutable, as it is not declared as mutable + //~^ NOTE cannot borrow as mutable +} diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr index cb7355b2335..3ed54179b98 100644 --- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr @@ -1,21 +1,65 @@ error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable - --> $DIR/mut-borrow-of-mut-ref.rs:5:7 + --> $DIR/mut-borrow-of-mut-ref.rs:7:7 | -LL | g(&mut b); +LL | h(&mut b); + | ^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/mut-borrow-of-mut-ref.rs:4:13 + | +LL | pub fn f(b: &mut i32) { + | ^^^^^^^^ +help: try removing `&mut` here + --> $DIR/mut-borrow-of-mut-ref.rs:7:7 + | +LL | h(&mut b); | ^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable - --> $DIR/mut-borrow-of-mut-ref.rs:8:12 + --> $DIR/mut-borrow-of-mut-ref.rs:11:12 + | +LL | g(&mut &mut b); + | ^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/mut-borrow-of-mut-ref.rs:4:13 + | +LL | pub fn f(b: &mut i32) { + | ^^^^^^^^ +help: try removing `&mut` here + --> $DIR/mut-borrow-of-mut-ref.rs:11:12 | LL | g(&mut &mut b); | ^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here -error: aborting due to 2 previous errors +error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable + --> $DIR/mut-borrow-of-mut-ref.rs:18:12 + | +LL | h(&mut &mut b); + | ^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/mut-borrow-of-mut-ref.rs:17:13 + | +LL | pub fn g(b: &mut i32) { + | ^^^^^^^^ +help: try removing `&mut` here + --> $DIR/mut-borrow-of-mut-ref.rs:18:12 + | +LL | h(&mut &mut b); + | ^^^^^^ + +error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable + --> $DIR/mut-borrow-of-mut-ref.rs:35:5 + | +LL | f.bar(); + | ^ cannot borrow as mutable + | +help: consider making the binding mutable + | +LL | pub fn baz(mut f: &mut String) { + | ^^^ + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/did_you_mean/issue-31424.stderr b/src/test/ui/did_you_mean/issue-31424.stderr index 838e81043db..88617381236 100644 --- a/src/test/ui/did_you_mean/issue-31424.stderr +++ b/src/test/ui/did_you_mean/issue-31424.stderr @@ -1,11 +1,19 @@ error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable --> $DIR/issue-31424.rs:7:9 | +LL | (&mut self).bar(); + | ^^^^^^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/issue-31424.rs:6:12 + | +LL | fn foo(&mut self) { + | ^^^^^^^^^ +help: try removing `&mut` here + --> $DIR/issue-31424.rs:7:9 + | LL | (&mut self).bar(); | ^^^^^^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here warning: function cannot return without recursing --> $DIR/issue-31424.rs:13:5 @@ -22,11 +30,19 @@ LL | (&mut self).bar(); error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable --> $DIR/issue-31424.rs:16:9 | +LL | (&mut self).bar(); + | ^^^^^^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/issue-31424.rs:13:18 + | +LL | fn bar(self: &mut Self) { + | ^^^^^^^^^ +help: try removing `&mut` here + --> $DIR/issue-31424.rs:16:9 + | LL | (&mut self).bar(); | ^^^^^^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here error: aborting due to 2 previous errors; 1 warning emitted diff --git a/src/test/ui/did_you_mean/issue-34126.stderr b/src/test/ui/did_you_mean/issue-34126.stderr index 669684fb3dd..a8c031686c0 100644 --- a/src/test/ui/did_you_mean/issue-34126.stderr +++ b/src/test/ui/did_you_mean/issue-34126.stderr @@ -1,11 +1,19 @@ error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable --> $DIR/issue-34126.rs:6:18 | +LL | self.run(&mut self); + | ^^^^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/issue-34126.rs:5:14 + | +LL | fn start(&mut self) { + | ^^^^^^^^^ +help: try removing `&mut` here + --> $DIR/issue-34126.rs:6:18 + | LL | self.run(&mut self); | ^^^^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here error[E0502]: cannot borrow `self` as mutable because it is also borrowed as immutable --> $DIR/issue-34126.rs:6:18 diff --git a/src/test/ui/nll/issue-51191.stderr b/src/test/ui/nll/issue-51191.stderr index 450993425e2..18696f57c44 100644 --- a/src/test/ui/nll/issue-51191.stderr +++ b/src/test/ui/nll/issue-51191.stderr @@ -13,11 +13,19 @@ LL | (&mut self).bar(); error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable --> $DIR/issue-51191.rs:7:9 | +LL | (&mut self).bar(); + | ^^^^^^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/issue-51191.rs:4:18 + | +LL | fn bar(self: &mut Self) { + | ^^^^^^^^^ +help: try removing `&mut` here + --> $DIR/issue-51191.rs:7:9 + | LL | (&mut self).bar(); | ^^^^^^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable --> $DIR/issue-51191.rs:13:9 @@ -42,11 +50,19 @@ LL | (&mut self).bar(); error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable --> $DIR/issue-51191.rs:28:9 | +LL | (&mut self).bar(); + | ^^^^^^^^^^^ cannot borrow as mutable + | +note: the binding is already a mutable borrow + --> $DIR/issue-51191.rs:27:16 + | +LL | fn mtblref(&mut self) { + | ^^^^^^^^^ +help: try removing `&mut` here + --> $DIR/issue-51191.rs:28:9 + | LL | (&mut self).bar(); | ^^^^^^^^^^^ - | | - | cannot borrow as mutable - | help: try removing `&mut` here error: aborting due to 5 previous errors; 1 warning emitted From a6af6d6506e0420f2a62ce519db029c97f8cdd0b Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Mon, 9 Aug 2021 11:05:50 +0000 Subject: [PATCH 2/3] Provide structured suggestion for removal of `&mut` --- .../diagnostics/mutability_errors.rs | 22 +++++++++++++++++-- .../ui/borrowck/mut-borrow-of-mut-ref.stderr | 15 +++++-------- src/test/ui/did_you_mean/issue-34126.stderr | 5 ++--- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index f8a1b05f063..4e079ed865a 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -12,7 +12,7 @@ use rustc_middle::{ }; use rustc_span::source_map::DesugaringKind; use rustc_span::symbol::{kw, Symbol}; -use rustc_span::Span; +use rustc_span::{BytePos, Span}; use crate::borrow_check::diagnostics::BorrowedContentSource; use crate::borrow_check::MirBorrowckCtxt; @@ -278,7 +278,25 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ); } } - err.span_help(source_info.span, "try removing `&mut` here"); + if let Ok(snippet) = + self.infcx.tcx.sess.source_map().span_to_snippet(source_info.span) + { + if snippet.starts_with("&mut ") { + // We don't have access to the HIR to get accurate spans, but we can + // give a best effort structured suggestion. + err.span_suggestion_verbose( + source_info.span.with_hi(source_info.span.lo() + BytePos(5)), + "try removing `&mut` here", + String::new(), + Applicability::MachineApplicable, + ); + } else { + // This can occur with things like `(&mut self).foo()`. + err.span_help(source_info.span, "try removing `&mut` here"); + } + } else { + err.span_help(source_info.span, "try removing `&mut` here"); + } } else if decl.mutability == Mutability::Not && !matches!( decl.local_info, diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr index 3ed54179b98..aa7771a736c 100644 --- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr @@ -10,10 +10,9 @@ note: the binding is already a mutable borrow LL | pub fn f(b: &mut i32) { | ^^^^^^^^ help: try removing `&mut` here - --> $DIR/mut-borrow-of-mut-ref.rs:7:7 | -LL | h(&mut b); - | ^^^^^^ +LL | h(b); + | -- error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable --> $DIR/mut-borrow-of-mut-ref.rs:11:12 @@ -27,10 +26,9 @@ note: the binding is already a mutable borrow LL | pub fn f(b: &mut i32) { | ^^^^^^^^ help: try removing `&mut` here - --> $DIR/mut-borrow-of-mut-ref.rs:11:12 | -LL | g(&mut &mut b); - | ^^^^^^ +LL | g(&mut b); + | -- error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable --> $DIR/mut-borrow-of-mut-ref.rs:18:12 @@ -44,10 +42,9 @@ note: the binding is already a mutable borrow LL | pub fn g(b: &mut i32) { | ^^^^^^^^ help: try removing `&mut` here - --> $DIR/mut-borrow-of-mut-ref.rs:18:12 | -LL | h(&mut &mut b); - | ^^^^^^ +LL | h(&mut b); + | -- error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable --> $DIR/mut-borrow-of-mut-ref.rs:35:5 diff --git a/src/test/ui/did_you_mean/issue-34126.stderr b/src/test/ui/did_you_mean/issue-34126.stderr index a8c031686c0..86bc27106a9 100644 --- a/src/test/ui/did_you_mean/issue-34126.stderr +++ b/src/test/ui/did_you_mean/issue-34126.stderr @@ -10,10 +10,9 @@ note: the binding is already a mutable borrow LL | fn start(&mut self) { | ^^^^^^^^^ help: try removing `&mut` here - --> $DIR/issue-34126.rs:6:18 | -LL | self.run(&mut self); - | ^^^^^^^^^ +LL | self.run(self); + | -- error[E0502]: cannot borrow `self` as mutable because it is also borrowed as immutable --> $DIR/issue-34126.rs:6:18 From 5b6f4b94c5679c9cce70d42cb4cbdbd55da2e0f7 Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Sun, 29 Aug 2021 08:48:36 +0000 Subject: [PATCH 3/3] rebase: fix test output --- .../ui/borrowck/mut-borrow-of-mut-ref.stderr | 17 ++++++++++------- src/test/ui/did_you_mean/issue-34126.stderr | 5 +++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr index aa7771a736c..e4c51bb77c9 100644 --- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr @@ -11,8 +11,9 @@ LL | pub fn f(b: &mut i32) { | ^^^^^^^^ help: try removing `&mut` here | -LL | h(b); - | -- +LL - h(&mut b); +LL + h(b); + | error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable --> $DIR/mut-borrow-of-mut-ref.rs:11:12 @@ -27,8 +28,9 @@ LL | pub fn f(b: &mut i32) { | ^^^^^^^^ help: try removing `&mut` here | -LL | g(&mut b); - | -- +LL - g(&mut &mut b); +LL + g(&mut b); + | error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable --> $DIR/mut-borrow-of-mut-ref.rs:18:12 @@ -43,8 +45,9 @@ LL | pub fn g(b: &mut i32) { | ^^^^^^^^ help: try removing `&mut` here | -LL | h(&mut b); - | -- +LL - h(&mut &mut b); +LL + h(&mut b); + | error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable --> $DIR/mut-borrow-of-mut-ref.rs:35:5 @@ -55,7 +58,7 @@ LL | f.bar(); help: consider making the binding mutable | LL | pub fn baz(mut f: &mut String) { - | ^^^ + | +++ error: aborting due to 4 previous errors diff --git a/src/test/ui/did_you_mean/issue-34126.stderr b/src/test/ui/did_you_mean/issue-34126.stderr index 86bc27106a9..666172197ce 100644 --- a/src/test/ui/did_you_mean/issue-34126.stderr +++ b/src/test/ui/did_you_mean/issue-34126.stderr @@ -11,8 +11,9 @@ LL | fn start(&mut self) { | ^^^^^^^^^ help: try removing `&mut` here | -LL | self.run(self); - | -- +LL - self.run(&mut self); +LL + self.run(self); + | error[E0502]: cannot borrow `self` as mutable because it is also borrowed as immutable --> $DIR/issue-34126.rs:6:18