From fd8aa5ec7d26c8e08d13b38213391c757045e51d Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Wed, 19 Apr 2023 18:22:03 +1200 Subject: [PATCH 1/7] tweak "make mut" spans when assigning to locals --- .../src/diagnostics/mutability_errors.rs | 56 ++++++++++--------- tests/ui/array-slice-vec/slice-mut-2.stderr | 2 +- ...row-raw-address-of-deref-mutability.stderr | 4 +- .../borrowck-access-permissions.stderr | 6 +- tests/ui/borrowck/borrowck-issue-14498.stderr | 2 +- tests/ui/borrowck/issue-85765.stderr | 2 +- .../diagnostics/mut_ref.stderr | 2 +- tests/ui/did_you_mean/issue-40823.stderr | 2 +- tests/ui/error-codes/E0389.stderr | 2 +- tests/ui/issues/issue-51515.rs | 1 - tests/ui/issues/issue-51515.stderr | 6 +- tests/ui/nll/issue-47388.stderr | 2 +- 12 files changed, 46 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 7558247948fe4..b861a4f28617e 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1202,36 +1202,42 @@ fn suggest_ampmut<'tcx>( opt_assignment_rhs_span: Option, opt_ty_info: Option, ) -> (bool, Span, String) { + // if there is a RHS and it starts with a `&` from it, then check if it is + // mutable, and if not, put suggest putting `mut ` to make it mutable. + // we don't have to worry about lifetime annotations here because they are + // not valid when taking a reference. For example, the following is not valid Rust: + // + // let x: &i32 = &'a 5; + // ^^ lifetime annotation not allowed + // if let Some(assignment_rhs_span) = opt_assignment_rhs_span && let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span) + && let Some(stripped) = src.strip_prefix('&') { - let is_mutbl = |ty: &str| -> bool { - if let Some(rest) = ty.strip_prefix("mut") { - match rest.chars().next() { - // e.g. `&mut x` - Some(c) if c.is_whitespace() => true, - // e.g. `&mut(x)` - Some('(') => true, - // e.g. `&mut{x}` - Some('{') => true, - // e.g. `&mutablevar` - _ => false, - } - } else { - false + let is_mut = if let Some(rest) = stripped.trim_start().strip_prefix("mut") { + match rest.chars().next() { + // e.g. `&mut x` + Some(c) if c.is_whitespace() => true, + // e.g. `&mut(x)` + Some('(') => true, + // e.g. `&mut{x}` + Some('{') => true, + // e.g. `&mutablevar` + _ => false, } + } else { + false }; - if let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(char::is_whitespace)) { - let lt_name = &src[1..ws_pos]; - let ty = src[ws_pos..].trim_start(); - if !is_mutbl(ty) { - return (true, assignment_rhs_span, format!("&{lt_name} mut {ty}")); - } - } else if let Some(stripped) = src.strip_prefix('&') { - let stripped = stripped.trim_start(); - if !is_mutbl(stripped) { - return (true, assignment_rhs_span, format!("&mut {stripped}")); - } + // if the reference is already mutable then there is nothing we can do + // here. + if !is_mut { + let span = assignment_rhs_span; + // shrink the span to just after the `&` in `&variable` + let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo(); + + // FIXME(Ezrashaw): returning is bad because we still might want to + // update the annotated type, see #106857. + return (true, span, "mut ".to_owned()); } } diff --git a/tests/ui/array-slice-vec/slice-mut-2.stderr b/tests/ui/array-slice-vec/slice-mut-2.stderr index 5b040d3e4d31d..c33919c41cdc2 100644 --- a/tests/ui/array-slice-vec/slice-mut-2.stderr +++ b/tests/ui/array-slice-vec/slice-mut-2.stderr @@ -7,7 +7,7 @@ LL | let _ = &mut x[2..4]; help: consider changing this to be a mutable reference | LL | let x: &[isize] = &mut [1, 2, 3, 4, 5]; - | ~~~~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/borrowck/borrow-raw-address-of-deref-mutability.stderr b/tests/ui/borrowck/borrow-raw-address-of-deref-mutability.stderr index 4cc1d821d0a06..cfc86ff0dc121 100644 --- a/tests/ui/borrowck/borrow-raw-address-of-deref-mutability.stderr +++ b/tests/ui/borrowck/borrow-raw-address-of-deref-mutability.stderr @@ -7,7 +7,7 @@ LL | let q = &raw mut *x; help: consider changing this to be a mutable reference | LL | let x = &mut 0; - | ~~~~~~ + | +++ error[E0596]: cannot borrow `*x` as mutable, as it is behind a `*const` pointer --> $DIR/borrow-raw-address-of-deref-mutability.rs:14:13 @@ -18,7 +18,7 @@ LL | let q = &raw mut *x; help: consider changing this to be a mutable pointer | LL | let x = &mut 0 as *const i32; - | ~~~~~~ + | +++ error: aborting due to 2 previous errors diff --git a/tests/ui/borrowck/borrowck-access-permissions.stderr b/tests/ui/borrowck/borrowck-access-permissions.stderr index 26f3e2bbdb775..c161e2d95b43a 100644 --- a/tests/ui/borrowck/borrowck-access-permissions.stderr +++ b/tests/ui/borrowck/borrowck-access-permissions.stderr @@ -35,7 +35,7 @@ LL | let _y1 = &mut *ref_x; help: consider changing this to be a mutable reference | LL | let ref_x = &mut x; - | ~~~~~~ + | +++ error[E0596]: cannot borrow `*ptr_x` as mutable, as it is behind a `*const` pointer --> $DIR/borrowck-access-permissions.rs:39:23 @@ -46,7 +46,7 @@ LL | let _y1 = &mut *ptr_x; help: consider changing this to be a mutable pointer | LL | let ptr_x : *const _ = &mut x; - | ~~~~~~ + | +++ error[E0596]: cannot borrow `*foo_ref.f` as mutable, as it is behind a `&` reference --> $DIR/borrowck-access-permissions.rs:48:18 @@ -57,7 +57,7 @@ LL | let _y = &mut *foo_ref.f; help: consider changing this to be a mutable reference | LL | let foo_ref = &mut foo; - | ~~~~~~~~ + | +++ error: aborting due to 6 previous errors diff --git a/tests/ui/borrowck/borrowck-issue-14498.stderr b/tests/ui/borrowck/borrowck-issue-14498.stderr index 374c5ee3ed27b..12d67d536d951 100644 --- a/tests/ui/borrowck/borrowck-issue-14498.stderr +++ b/tests/ui/borrowck/borrowck-issue-14498.stderr @@ -7,7 +7,7 @@ LL | ***p = 2; help: consider changing this to be a mutable reference | LL | let p = &mut y; - | ~~~~~~ + | +++ error[E0506]: cannot assign to `**y` because it is borrowed --> $DIR/borrowck-issue-14498.rs:25:5 diff --git a/tests/ui/borrowck/issue-85765.stderr b/tests/ui/borrowck/issue-85765.stderr index b4bb128cbb420..2985a658fddbe 100644 --- a/tests/ui/borrowck/issue-85765.stderr +++ b/tests/ui/borrowck/issue-85765.stderr @@ -18,7 +18,7 @@ LL | *r = 0; help: consider changing this to be a mutable reference | LL | let r = &mut mutvar; - | ~~~~~~~~~~~ + | +++ error[E0594]: cannot assign to `*x`, which is behind a `&` reference --> $DIR/issue-85765.rs:19:5 diff --git a/tests/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr b/tests/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr index 95f36fc042c4f..1904faa959861 100644 --- a/tests/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr +++ b/tests/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr @@ -10,7 +10,7 @@ LL | **ref_mref_x = y; help: consider changing this to be a mutable reference | LL | let ref_mref_x = &mut mref_x; - | ~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference --> $DIR/mut_ref.rs:26:13 diff --git a/tests/ui/did_you_mean/issue-40823.stderr b/tests/ui/did_you_mean/issue-40823.stderr index aadd698891edc..ba94a57025639 100644 --- a/tests/ui/did_you_mean/issue-40823.stderr +++ b/tests/ui/did_you_mean/issue-40823.stderr @@ -7,7 +7,7 @@ LL | buf.iter_mut(); help: consider changing this to be a mutable reference | LL | let mut buf = &mut [1, 2, 3, 4]; - | ~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/error-codes/E0389.stderr b/tests/ui/error-codes/E0389.stderr index 51c4c92addf28..e4001856c388b 100644 --- a/tests/ui/error-codes/E0389.stderr +++ b/tests/ui/error-codes/E0389.stderr @@ -7,7 +7,7 @@ LL | fancy_ref.num = 6; help: consider changing this to be a mutable reference | LL | let fancy_ref = &mut (&mut fancy); - | ~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/issues/issue-51515.rs b/tests/ui/issues/issue-51515.rs index 84e09afac0a2d..33a9bf85e23ea 100644 --- a/tests/ui/issues/issue-51515.rs +++ b/tests/ui/issues/issue-51515.rs @@ -1,7 +1,6 @@ fn main() { let foo = &16; //~^ HELP consider changing this to be a mutable reference - //~| SUGGESTION &mut 16 *foo = 32; //~^ ERROR cannot assign to `*foo`, which is behind a `&` reference let bar = foo; diff --git a/tests/ui/issues/issue-51515.stderr b/tests/ui/issues/issue-51515.stderr index 94e5c9f1b832a..88b8d21090886 100644 --- a/tests/ui/issues/issue-51515.stderr +++ b/tests/ui/issues/issue-51515.stderr @@ -1,5 +1,5 @@ error[E0594]: cannot assign to `*foo`, which is behind a `&` reference - --> $DIR/issue-51515.rs:5:5 + --> $DIR/issue-51515.rs:4:5 | LL | *foo = 32; | ^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written @@ -7,10 +7,10 @@ LL | *foo = 32; help: consider changing this to be a mutable reference | LL | let foo = &mut 16; - | ~~~~~~~ + | +++ error[E0594]: cannot assign to `*bar`, which is behind a `&` reference - --> $DIR/issue-51515.rs:9:5 + --> $DIR/issue-51515.rs:8:5 | LL | *bar = 64; | ^^^^^^^^^ `bar` is a `&` reference, so the data it refers to cannot be written diff --git a/tests/ui/nll/issue-47388.stderr b/tests/ui/nll/issue-47388.stderr index c780451dfa935..09b9d638afb03 100644 --- a/tests/ui/nll/issue-47388.stderr +++ b/tests/ui/nll/issue-47388.stderr @@ -7,7 +7,7 @@ LL | fancy_ref.num = 6; help: consider changing this to be a mutable reference | LL | let fancy_ref = &mut (&mut fancy); - | ~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error From 9624d2b08e87995978899f236db1857e85097b37 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Wed, 19 Apr 2023 19:29:28 +1200 Subject: [PATCH 2/7] tweak "make mut" spans (No. 2) --- .../src/diagnostics/mutability_errors.rs | 48 +++++++++++-------- tests/ui/did_you_mean/issue-39544.stderr | 2 +- tests/ui/issues/issue-61623.stderr | 2 +- ...ck-borrow-overloaded-auto-deref-mut.stderr | 4 +- ...orrowck-borrow-overloaded-deref-mut.stderr | 4 +- tests/ui/span/mut-arg-hint.stderr | 2 +- ...l-bounds-inconsistent-copy-reborrow.stderr | 4 +- 7 files changed, 36 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index b861a4f28617e..27f700ebbdaf8 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1241,35 +1241,41 @@ fn suggest_ampmut<'tcx>( } } - let (suggestibility, highlight_span) = match opt_ty_info { + let (binding_exists, span) = match opt_ty_info { // if this is a variable binding with an explicit type, - // try to highlight that for the suggestion. + // then we will suggest changing it to be mutable. + // this is `Applicability::MachineApplicable`. Some(ty_span) => (true, ty_span), - // otherwise, just highlight the span associated with - // the (MIR) LocalDecl. + // otherwise, we'll suggest *adding* an annotated type, we'll suggest + // the RHS's type for that. + // this is `Applicability::HasPlaceholders`. None => (false, local_decl.source_info.span), }; - if let Ok(src) = tcx.sess.source_map().span_to_snippet(highlight_span) - && let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(char::is_whitespace)) + // if the binding already exists and is a reference with a explicit + // lifetime, then we can suggest adding ` mut`. this is special-cased from + // the path without a explicit lifetime. + if let Ok(src) = tcx.sess.source_map().span_to_snippet(span) + && src.starts_with("&'") + // note that `& 'a T` is invalid so this is correct. + && let Some(ws_pos) = src.find(char::is_whitespace) { - let lt_name = &src[1..ws_pos]; - let ty = &src[ws_pos..]; - return (true, highlight_span, format!("&{lt_name} mut{ty}")); + let span = span.with_lo(span.lo() + BytePos(ws_pos as u32)).shrink_to_lo(); + (true, span, " mut".to_owned()) + } else { + let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); + assert_eq!(ty_mut.mutbl, hir::Mutability::Not); + ( + binding_exists, + span, + if local_decl.ty.is_ref() { + format!("&mut {}", ty_mut.ty) + } else { + format!("*mut {}", ty_mut.ty) + }, + ) } - - let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); - assert_eq!(ty_mut.mutbl, hir::Mutability::Not); - ( - suggestibility, - highlight_span, - if local_decl.ty.is_ref() { - format!("&mut {}", ty_mut.ty) - } else { - format!("*mut {}", ty_mut.ty) - }, - ) } fn is_closure_or_generator(ty: Ty<'_>) -> bool { diff --git a/tests/ui/did_you_mean/issue-39544.stderr b/tests/ui/did_you_mean/issue-39544.stderr index 8dc0512a9453d..97f0ec60603c5 100644 --- a/tests/ui/did_you_mean/issue-39544.stderr +++ b/tests/ui/did_you_mean/issue-39544.stderr @@ -73,7 +73,7 @@ LL | let _ = &mut self.x; help: consider changing this to be a mutable reference | LL | fn foo3<'a>(self: &'a mut Self, other: &Z) { - | ~~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `other.x` as mutable, as it is behind a `&` reference --> $DIR/issue-39544.rs:31:17 diff --git a/tests/ui/issues/issue-61623.stderr b/tests/ui/issues/issue-61623.stderr index 5fcc338557c58..bedea3890a32e 100644 --- a/tests/ui/issues/issue-61623.stderr +++ b/tests/ui/issues/issue-61623.stderr @@ -7,7 +7,7 @@ LL | f2(|| x.0, f1(x.1)) help: consider changing this to be a mutable reference | LL | fn f3<'a>(x: &'a mut ((), &'a mut ())) { - | ~~~~~~~~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr b/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr index 570328fc211f9..e640d40913e37 100644 --- a/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr +++ b/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr @@ -50,7 +50,7 @@ LL | x.y = 3; help: consider changing this to be a mutable reference | LL | fn assign_field2<'a>(x: &'a mut Own) { - | ~~~~~~~~~~~~~~~~~~ + | +++ error[E0499]: cannot borrow `*x` as mutable more than once at a time --> $DIR/borrowck-borrow-overloaded-auto-deref-mut.rs:101:5 @@ -104,7 +104,7 @@ LL | *x.y_mut() = 3; help: consider changing this to be a mutable reference | LL | fn assign_method2<'a>(x: &'a mut Own) { - | ~~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to 10 previous errors diff --git a/tests/ui/span/borrowck-borrow-overloaded-deref-mut.stderr b/tests/ui/span/borrowck-borrow-overloaded-deref-mut.stderr index 3fed7b3f4dcf3..dbd52dc2d38df 100644 --- a/tests/ui/span/borrowck-borrow-overloaded-deref-mut.stderr +++ b/tests/ui/span/borrowck-borrow-overloaded-deref-mut.stderr @@ -18,7 +18,7 @@ LL | &mut **x help: consider changing this to be a mutable reference | LL | fn deref_extend_mut1<'a>(x: &'a mut Own) -> &'a mut isize { - | ~~~~~~~~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable --> $DIR/borrowck-borrow-overloaded-deref-mut.rs:49:6 @@ -40,7 +40,7 @@ LL | **x = 3; help: consider changing this to be a mutable reference | LL | fn assign2<'a>(x: &'a mut Own) { - | ~~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to 4 previous errors diff --git a/tests/ui/span/mut-arg-hint.stderr b/tests/ui/span/mut-arg-hint.stderr index 96ce4d5bc6c33..9b82012291cff 100644 --- a/tests/ui/span/mut-arg-hint.stderr +++ b/tests/ui/span/mut-arg-hint.stderr @@ -18,7 +18,7 @@ LL | a.push_str("foo"); help: consider changing this to be a mutable reference | LL | pub fn foo<'a>(mut a: &'a mut String) { - | ~~~~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `*a` as mutable, as it is behind a `&` reference --> $DIR/mut-arg-hint.rs:15:9 diff --git a/tests/ui/trivial-bounds/trivial-bounds-inconsistent-copy-reborrow.stderr b/tests/ui/trivial-bounds/trivial-bounds-inconsistent-copy-reborrow.stderr index 39b60c3119727..c054ddb893d5c 100644 --- a/tests/ui/trivial-bounds/trivial-bounds-inconsistent-copy-reborrow.stderr +++ b/tests/ui/trivial-bounds/trivial-bounds-inconsistent-copy-reborrow.stderr @@ -7,7 +7,7 @@ LL | *t help: consider changing this to be a mutable reference | LL | fn reborrow_mut<'a>(t: &'a mut &'a mut i32) -> &'a mut i32 where &'a mut i32: Copy { - | ~~~~~~~~~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `**t` as mutable, as it is behind a `&` reference --> $DIR/trivial-bounds-inconsistent-copy-reborrow.rs:10:6 @@ -18,7 +18,7 @@ LL | {*t} help: consider changing this to be a mutable reference | LL | fn copy_reborrow_mut<'a>(t: &'a mut &'a mut i32) -> &'a mut i32 where &'a mut i32: Copy { - | ~~~~~~~~~~~~~~~~~~~ + | +++ error: aborting due to 2 previous errors From 57c6a3183c17e9f889adda1faca894a7cd692b02 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Wed, 19 Apr 2023 19:50:08 +1200 Subject: [PATCH 3/7] tweak "make mut" spans (No. 3) --- .../src/diagnostics/mutability_errors.rs | 4 ++++ ...k-assign-to-andmut-in-aliasable-loc.stderr | 8 +++---- ...orrow-mut-base-ptr-in-aliasable-loc.stderr | 4 ++-- .../borrowck-reborrow-from-mut.stderr | 2 +- tests/ui/borrowck/mutability-errors.stderr | 24 +++++++++---------- tests/ui/did_you_mean/issue-38147-4.stderr | 4 ++-- tests/ui/did_you_mean/issue-39544.stderr | 10 ++++---- tests/ui/nll/issue-57989.stderr | 2 +- ...ck-borrow-overloaded-auto-deref-mut.stderr | 4 ++-- ...borrowck-call-is-borrow-issue-12224.stderr | 6 ++--- ...owck-call-method-from-mut-aliasable.stderr | 2 +- tests/ui/span/borrowck-fn-in-const-b.stderr | 2 +- .../ui/span/borrowck-object-mutability.stderr | 2 +- tests/ui/span/mut-arg-hint.stderr | 4 ++-- tests/ui/suggestions/issue-68049-2.stderr | 4 ++-- 15 files changed, 43 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 27f700ebbdaf8..50f9e7095f93b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1263,6 +1263,10 @@ fn suggest_ampmut<'tcx>( { let span = span.with_lo(span.lo() + BytePos(ws_pos as u32)).shrink_to_lo(); (true, span, " mut".to_owned()) + } else if binding_exists { + // shrink the span to just after the `&` in `&variable` + let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo(); + (true, span, "mut ".to_owned()) } else { let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); assert_eq!(ty_mut.mutbl, hir::Mutability::Not); diff --git a/tests/ui/borrowck/borrowck-assign-to-andmut-in-aliasable-loc.stderr b/tests/ui/borrowck/borrowck-assign-to-andmut-in-aliasable-loc.stderr index cbacc87a0e858..cf0c4127d82f4 100644 --- a/tests/ui/borrowck/borrowck-assign-to-andmut-in-aliasable-loc.stderr +++ b/tests/ui/borrowck/borrowck-assign-to-andmut-in-aliasable-loc.stderr @@ -6,8 +6,8 @@ LL | *s.pointer += 1; | help: consider changing this to be a mutable reference | -LL | fn a(s: &mut S<'_>) { - | ~~~~~~~~~~ +LL | fn a(s: &mut S) { + | +++ error[E0594]: cannot assign to `*s.pointer`, which is behind a `&` reference --> $DIR/borrowck-assign-to-andmut-in-aliasable-loc.rs:17:5 @@ -17,8 +17,8 @@ LL | *s.pointer += 1; | help: consider changing this to be a mutable reference | -LL | fn c(s: &mut &mut S<'_>) { - | ~~~~~~~~~~~~~~~ +LL | fn c(s: &mut &mut S) { + | +++ error: aborting due to 2 previous errors diff --git a/tests/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr b/tests/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr index dd0817ff23313..59ef61b19d501 100644 --- a/tests/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr +++ b/tests/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr @@ -27,8 +27,8 @@ LL | let x: &mut isize = &mut **t0; | help: consider changing this to be a mutable reference | -LL | fn foo4(t0: &mut &mut isize) { - | ~~~~~~~~~~~~~~~ +LL | fn foo4(t0: &mut &mut isize) { + | +++ error: aborting due to 3 previous errors diff --git a/tests/ui/borrowck/borrowck-reborrow-from-mut.stderr b/tests/ui/borrowck/borrowck-reborrow-from-mut.stderr index d9590e446c756..fb3db4e144635 100644 --- a/tests/ui/borrowck/borrowck-reborrow-from-mut.stderr +++ b/tests/ui/borrowck/borrowck-reborrow-from-mut.stderr @@ -111,7 +111,7 @@ LL | let _bar1 = &mut foo.bar1; help: consider changing this to be a mutable reference | LL | fn borrow_mut_from_imm(foo: &mut Foo) { - | ~~~~~~~~ + | +++ error: aborting due to 11 previous errors diff --git a/tests/ui/borrowck/mutability-errors.stderr b/tests/ui/borrowck/mutability-errors.stderr index d7c602718f173..b39e57d70ec65 100644 --- a/tests/ui/borrowck/mutability-errors.stderr +++ b/tests/ui/borrowck/mutability-errors.stderr @@ -7,7 +7,7 @@ LL | *x = (1,); help: consider changing this to be a mutable reference | LL | fn named_ref(x: &mut (i32,)) { - | ~~~~~~~~~~~ + | +++ error[E0594]: cannot assign to `x.0`, which is behind a `&` reference --> $DIR/mutability-errors.rs:10:5 @@ -18,7 +18,7 @@ LL | x.0 = 1; help: consider changing this to be a mutable reference | LL | fn named_ref(x: &mut (i32,)) { - | ~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `*x` as mutable, as it is behind a `&` reference --> $DIR/mutability-errors.rs:11:5 @@ -29,7 +29,7 @@ LL | &mut *x; help: consider changing this to be a mutable reference | LL | fn named_ref(x: &mut (i32,)) { - | ~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `x.0` as mutable, as it is behind a `&` reference --> $DIR/mutability-errors.rs:12:5 @@ -40,7 +40,7 @@ LL | &mut x.0; help: consider changing this to be a mutable reference | LL | fn named_ref(x: &mut (i32,)) { - | ~~~~~~~~~~~ + | +++ error[E0594]: cannot assign to data in a `&` reference --> $DIR/mutability-errors.rs:16:5 @@ -74,8 +74,8 @@ LL | *x = (1,); | help: consider changing this to be a mutable pointer | -LL | unsafe fn named_ptr(x: *mut (i32,)) { - | ~~~~~~~~~~~ +LL | unsafe fn named_ptr(x: *mut const (i32,)) { + | +++ error[E0594]: cannot assign to `x.0`, which is behind a `*const` pointer --> $DIR/mutability-errors.rs:24:5 @@ -85,8 +85,8 @@ LL | (*x).0 = 1; | help: consider changing this to be a mutable pointer | -LL | unsafe fn named_ptr(x: *mut (i32,)) { - | ~~~~~~~~~~~ +LL | unsafe fn named_ptr(x: *mut const (i32,)) { + | +++ error[E0596]: cannot borrow `*x` as mutable, as it is behind a `*const` pointer --> $DIR/mutability-errors.rs:25:5 @@ -96,8 +96,8 @@ LL | &mut *x; | help: consider changing this to be a mutable pointer | -LL | unsafe fn named_ptr(x: *mut (i32,)) { - | ~~~~~~~~~~~ +LL | unsafe fn named_ptr(x: *mut const (i32,)) { + | +++ error[E0596]: cannot borrow `x.0` as mutable, as it is behind a `*const` pointer --> $DIR/mutability-errors.rs:26:5 @@ -107,8 +107,8 @@ LL | &mut (*x).0; | help: consider changing this to be a mutable pointer | -LL | unsafe fn named_ptr(x: *mut (i32,)) { - | ~~~~~~~~~~~ +LL | unsafe fn named_ptr(x: *mut const (i32,)) { + | +++ error[E0594]: cannot assign to data in a `*const` pointer --> $DIR/mutability-errors.rs:30:5 diff --git a/tests/ui/did_you_mean/issue-38147-4.stderr b/tests/ui/did_you_mean/issue-38147-4.stderr index d333998936169..43647fa562b9a 100644 --- a/tests/ui/did_you_mean/issue-38147-4.stderr +++ b/tests/ui/did_you_mean/issue-38147-4.stderr @@ -6,8 +6,8 @@ LL | f.s.push('x'); | help: consider changing this to be a mutable reference | -LL | fn f(x: usize, f: &mut Foo<'_>) { - | ~~~~~~~~~~~~ +LL | fn f(x: usize, f: &mut Foo) { + | +++ error: aborting due to previous error diff --git a/tests/ui/did_you_mean/issue-39544.stderr b/tests/ui/did_you_mean/issue-39544.stderr index 97f0ec60603c5..8ccb4cbb0c167 100644 --- a/tests/ui/did_you_mean/issue-39544.stderr +++ b/tests/ui/did_you_mean/issue-39544.stderr @@ -40,7 +40,7 @@ LL | let _ = &mut other.x; help: consider changing this to be a mutable reference | LL | fn foo1(&self, other: &mut Z) { - | ~~~~~~ + | +++ error[E0596]: cannot borrow `self.x` as mutable, as it is behind a `&` reference --> $DIR/issue-39544.rs:25:17 @@ -62,7 +62,7 @@ LL | let _ = &mut other.x; help: consider changing this to be a mutable reference | LL | fn foo2<'a>(&'a self, other: &mut Z) { - | ~~~~~~ + | +++ error[E0596]: cannot borrow `self.x` as mutable, as it is behind a `&` reference --> $DIR/issue-39544.rs:30:17 @@ -84,7 +84,7 @@ LL | let _ = &mut other.x; help: consider changing this to be a mutable reference | LL | fn foo3<'a>(self: &'a Self, other: &mut Z) { - | ~~~~~~ + | +++ error[E0596]: cannot borrow `other.x` as mutable, as it is behind a `&` reference --> $DIR/issue-39544.rs:35:17 @@ -95,7 +95,7 @@ LL | let _ = &mut other.x; help: consider changing this to be a mutable reference | LL | fn foo4(other: &mut Z) { - | ~~~~~~ + | +++ error[E0596]: cannot borrow `z.x` as mutable, as `z` is not declared as mutable --> $DIR/issue-39544.rs:41:13 @@ -117,7 +117,7 @@ LL | let _ = &mut w.x; help: consider changing this to be a mutable reference | LL | pub fn with_arg(z: Z, w: &mut Z) { - | ~~~~~~ + | +++ error[E0594]: cannot assign to `*x.0`, which is behind a `&` reference --> $DIR/issue-39544.rs:48:5 diff --git a/tests/ui/nll/issue-57989.stderr b/tests/ui/nll/issue-57989.stderr index d5effd6f346d1..6062b31d6883c 100644 --- a/tests/ui/nll/issue-57989.stderr +++ b/tests/ui/nll/issue-57989.stderr @@ -7,7 +7,7 @@ LL | *x = 0; help: consider changing this to be a mutable reference | LL | fn f(x: &mut i32) { - | ~~~~~~~~ + | +++ error[E0506]: cannot assign to `*x` because it is borrowed --> $DIR/issue-57989.rs:5:5 diff --git a/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr b/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr index e640d40913e37..80c5f9da40cea 100644 --- a/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr +++ b/tests/ui/span/borrowck-borrow-overloaded-auto-deref-mut.stderr @@ -18,7 +18,7 @@ LL | &mut x.y help: consider changing this to be a mutable reference | LL | fn deref_extend_mut_field1(x: &mut Own) -> &mut isize { - | ~~~~~~~~~~~~~~~ + | +++ error[E0499]: cannot borrow `*x` as mutable more than once at a time --> $DIR/borrowck-borrow-overloaded-auto-deref-mut.rs:78:19 @@ -82,7 +82,7 @@ LL | x.y_mut() help: consider changing this to be a mutable reference | LL | fn deref_extend_mut_method1(x: &mut Own) -> &mut isize { - | ~~~~~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable --> $DIR/borrowck-borrow-overloaded-auto-deref-mut.rs:129:6 diff --git a/tests/ui/span/borrowck-call-is-borrow-issue-12224.stderr b/tests/ui/span/borrowck-call-is-borrow-issue-12224.stderr index 9711dad80785b..99c8fa1f932d2 100644 --- a/tests/ui/span/borrowck-call-is-borrow-issue-12224.stderr +++ b/tests/ui/span/borrowck-call-is-borrow-issue-12224.stderr @@ -19,7 +19,7 @@ LL | (*f)(); help: consider changing this to be a mutable reference | LL | fn test2(f: &mut F) where F: FnMut() { - | ~~~~~~ + | +++ error[E0596]: cannot borrow `f.f` as mutable, as it is behind a `&` reference --> $DIR/borrowck-call-is-borrow-issue-12224.rs:34:5 @@ -29,8 +29,8 @@ LL | f.f.call_mut(()) | help: consider changing this to be a mutable reference | -LL | fn test4(f: &mut Test<'_>) { - | ~~~~~~~~~~~~~ +LL | fn test4(f: &mut Test) { + | +++ error[E0507]: cannot move out of `f`, a captured variable in an `FnMut` closure --> $DIR/borrowck-call-is-borrow-issue-12224.rs:57:13 diff --git a/tests/ui/span/borrowck-call-method-from-mut-aliasable.stderr b/tests/ui/span/borrowck-call-method-from-mut-aliasable.stderr index 2a842f5a2a9f8..328197ae9f429 100644 --- a/tests/ui/span/borrowck-call-method-from-mut-aliasable.stderr +++ b/tests/ui/span/borrowck-call-method-from-mut-aliasable.stderr @@ -7,7 +7,7 @@ LL | x.h(); help: consider changing this to be a mutable reference | LL | fn b(x: &mut Foo) { - | ~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/span/borrowck-fn-in-const-b.stderr b/tests/ui/span/borrowck-fn-in-const-b.stderr index 1df19deb12f6b..17fdcc622f776 100644 --- a/tests/ui/span/borrowck-fn-in-const-b.stderr +++ b/tests/ui/span/borrowck-fn-in-const-b.stderr @@ -7,7 +7,7 @@ LL | x.push(format!("this is broken")); help: consider changing this to be a mutable reference | LL | fn broken(x: &mut Vec) { - | ~~~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/span/borrowck-object-mutability.stderr b/tests/ui/span/borrowck-object-mutability.stderr index b6517e0b3095d..805a8034c184a 100644 --- a/tests/ui/span/borrowck-object-mutability.stderr +++ b/tests/ui/span/borrowck-object-mutability.stderr @@ -7,7 +7,7 @@ LL | x.borrowed_mut(); help: consider changing this to be a mutable reference | LL | fn borrowed_receiver(x: &mut dyn Foo) { - | ~~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `*x` as mutable, as `x` is not declared as mutable --> $DIR/borrowck-object-mutability.rs:18:5 diff --git a/tests/ui/span/mut-arg-hint.stderr b/tests/ui/span/mut-arg-hint.stderr index 9b82012291cff..06011eac674c1 100644 --- a/tests/ui/span/mut-arg-hint.stderr +++ b/tests/ui/span/mut-arg-hint.stderr @@ -7,7 +7,7 @@ LL | a.push_str("bar"); help: consider changing this to be a mutable reference | LL | fn foo(mut a: &mut String) { - | ~~~~~~~~~~~ + | +++ error[E0596]: cannot borrow `*a` as mutable, as it is behind a `&` reference --> $DIR/mut-arg-hint.rs:8:5 @@ -29,7 +29,7 @@ LL | a.push_str("foo"); help: consider changing this to be a mutable reference | LL | pub fn foo(mut a: &mut String) { - | ~~~~~~~~~~~ + | +++ error: aborting due to 3 previous errors diff --git a/tests/ui/suggestions/issue-68049-2.stderr b/tests/ui/suggestions/issue-68049-2.stderr index de35aa5b186b5..833799081598f 100644 --- a/tests/ui/suggestions/issue-68049-2.stderr +++ b/tests/ui/suggestions/issue-68049-2.stderr @@ -6,8 +6,8 @@ LL | *input = self.0; | help: consider changing that to be a mutable reference | -LL | fn example(&self, input: &mut i32); // should suggest here - | ~~~~~~~~ +LL | fn example(&self, input: mut ); // should suggest here + | ~~~ error[E0594]: cannot assign to `self.0`, which is behind a `&` reference --> $DIR/issue-68049-2.rs:17:5 From 336a6569f5477d36151500bbd7c57e3bba357cec Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Wed, 19 Apr 2023 20:00:49 +1200 Subject: [PATCH 4/7] tweak "make mut" spans (No. 4) --- .../src/diagnostics/mutability_errors.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 50f9e7095f93b..e842afced8199 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1263,21 +1263,21 @@ fn suggest_ampmut<'tcx>( { let span = span.with_lo(span.lo() + BytePos(ws_pos as u32)).shrink_to_lo(); (true, span, " mut".to_owned()) + // if there is already a binding, we modify it to be `mut` } else if binding_exists { // shrink the span to just after the `&` in `&variable` let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo(); (true, span, "mut ".to_owned()) + // otherwise, suggest that the user annotates the binding; we provide the + // type of the local. } else { let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); assert_eq!(ty_mut.mutbl, hir::Mutability::Not); + ( - binding_exists, + false, span, - if local_decl.ty.is_ref() { - format!("&mut {}", ty_mut.ty) - } else { - format!("*mut {}", ty_mut.ty) - }, + format!("{}mut {}", if local_decl.ty.is_ref() {"&"} else {"*"}, ty_mut.ty) ) } } From 87a1b3840ecfebdcd22313ed37f0609732d8cf83 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Thu, 20 Apr 2023 18:58:43 +1200 Subject: [PATCH 5/7] tweak spans for `ref mut` suggestion --- .../src/diagnostics/mutability_errors.rs | 16 +++++++++------- tests/ui/nll/issue-51244.stderr | 2 +- .../borrowck-move-ref-pattern.stderr | 4 ++-- tests/ui/suggestions/suggest-ref-mut.rs | 3 --- tests/ui/suggestions/suggest-ref-mut.stderr | 12 ++++++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index e842afced8199..0a89eb072455b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -559,9 +559,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { binding_mode: ty::BindingMode::BindByReference(_), .. })) => { - let pattern_span = local_decl.source_info.span; + let pattern_span: Span = local_decl.source_info.span; suggest_ref_mut(self.infcx.tcx, pattern_span) - .map(|replacement| (true, pattern_span, replacement)) + .map(|span| (true, span, "mut ".to_owned())) } _ => unreachable!(), @@ -1316,11 +1316,13 @@ fn get_mut_span_in_struct_field<'tcx>( } /// If possible, suggest replacing `ref` with `ref mut`. -fn suggest_ref_mut(tcx: TyCtxt<'_>, binding_span: Span) -> Option { - let hi_src = tcx.sess.source_map().span_to_snippet(binding_span).ok()?; - if hi_src.starts_with("ref") && hi_src["ref".len()..].starts_with(rustc_lexer::is_whitespace) { - let replacement = format!("ref mut{}", &hi_src["ref".len()..]); - Some(replacement) +fn suggest_ref_mut(tcx: TyCtxt<'_>, span: Span) -> Option { + let pattern_str = tcx.sess.source_map().span_to_snippet(span).ok()?; + if pattern_str.starts_with("ref") + && pattern_str["ref".len()..].starts_with(rustc_lexer::is_whitespace) + { + let span = span.with_lo(span.lo() + BytePos(4)).shrink_to_lo(); + Some(span) } else { None } diff --git a/tests/ui/nll/issue-51244.stderr b/tests/ui/nll/issue-51244.stderr index 03d8acc81886a..8ccb5809e3972 100644 --- a/tests/ui/nll/issue-51244.stderr +++ b/tests/ui/nll/issue-51244.stderr @@ -7,7 +7,7 @@ LL | *my_ref = 0; help: consider changing this to be a mutable reference | LL | let ref mut my_ref @ _ = 0; - | ~~~~~~~~~~~~~~ + | +++ error: aborting due to previous error diff --git a/tests/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr b/tests/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr index c7c7c074f7cd0..a033cc0655ef9 100644 --- a/tests/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr +++ b/tests/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr @@ -112,7 +112,7 @@ LL | *_x0 = U; help: consider changing this to be a mutable reference | LL | let (ref mut _x0, _x1, ref _x2, ..) = tup; - | ~~~~~~~~~~~ + | +++ error[E0594]: cannot assign to `*_x2`, which is behind a `&` reference --> $DIR/borrowck-move-ref-pattern.rs:27:5 @@ -123,7 +123,7 @@ LL | *_x2 = U; help: consider changing this to be a mutable reference | LL | let (ref _x0, _x1, ref mut _x2, ..) = tup; - | ~~~~~~~~~~~ + | +++ error[E0382]: use of moved value: `tup.1` --> $DIR/borrowck-move-ref-pattern.rs:28:10 diff --git a/tests/ui/suggestions/suggest-ref-mut.rs b/tests/ui/suggestions/suggest-ref-mut.rs index d04113ffccc3a..b40439b8e372c 100644 --- a/tests/ui/suggestions/suggest-ref-mut.rs +++ b/tests/ui/suggestions/suggest-ref-mut.rs @@ -12,12 +12,10 @@ impl X { fn main() { let ref foo = 16; //~^ HELP - //~| SUGGESTION ref mut foo *foo = 32; //~^ ERROR if let Some(ref bar) = Some(16) { //~^ HELP - //~| SUGGESTION ref mut bar *bar = 32; //~^ ERROR } @@ -25,6 +23,5 @@ fn main() { ref quo => { *quo = 32; }, //~^ ERROR //~| HELP - //~| SUGGESTION ref mut quo } } diff --git a/tests/ui/suggestions/suggest-ref-mut.stderr b/tests/ui/suggestions/suggest-ref-mut.stderr index 7973759bf5ec7..cc00022ab8e3d 100644 --- a/tests/ui/suggestions/suggest-ref-mut.stderr +++ b/tests/ui/suggestions/suggest-ref-mut.stderr @@ -10,7 +10,7 @@ LL | fn zap(&mut self) { | ~~~~~~~~~ error[E0594]: cannot assign to `*foo`, which is behind a `&` reference - --> $DIR/suggest-ref-mut.rs:16:5 + --> $DIR/suggest-ref-mut.rs:15:5 | LL | *foo = 32; | ^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written @@ -18,10 +18,10 @@ LL | *foo = 32; help: consider changing this to be a mutable reference | LL | let ref mut foo = 16; - | ~~~~~~~~~~~ + | +++ error[E0594]: cannot assign to `*bar`, which is behind a `&` reference - --> $DIR/suggest-ref-mut.rs:21:9 + --> $DIR/suggest-ref-mut.rs:19:9 | LL | *bar = 32; | ^^^^^^^^^ `bar` is a `&` reference, so the data it refers to cannot be written @@ -29,10 +29,10 @@ LL | *bar = 32; help: consider changing this to be a mutable reference | LL | if let Some(ref mut bar) = Some(16) { - | ~~~~~~~~~~~ + | +++ error[E0594]: cannot assign to `*quo`, which is behind a `&` reference - --> $DIR/suggest-ref-mut.rs:25:22 + --> $DIR/suggest-ref-mut.rs:23:22 | LL | ref quo => { *quo = 32; }, | ^^^^^^^^^ `quo` is a `&` reference, so the data it refers to cannot be written @@ -40,7 +40,7 @@ LL | ref quo => { *quo = 32; }, help: consider changing this to be a mutable reference | LL | ref mut quo => { *quo = 32; }, - | ~~~~~~~~~~~ + | +++ error: aborting due to 4 previous errors From d2608dfabb3a353fd2ab25f8bb1abf04b497af3b Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Fri, 21 Apr 2023 13:29:15 +1200 Subject: [PATCH 6/7] implement review comment Co-authored-by: Esteban Kuber --- compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 0a89eb072455b..a98a07a74e584 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1268,9 +1268,9 @@ fn suggest_ampmut<'tcx>( // shrink the span to just after the `&` in `&variable` let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo(); (true, span, "mut ".to_owned()) - // otherwise, suggest that the user annotates the binding; we provide the - // type of the local. } else { + // otherwise, suggest that the user annotates the binding; we provide the + // type of the local. let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); assert_eq!(ty_mut.mutbl, hir::Mutability::Not); From 3e64e986fe1cbaa3679cd228a6900304ebf81018 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Fri, 21 Apr 2023 23:49:05 +1200 Subject: [PATCH 7/7] fix trait definition spans in "make mut" suggestion --- .../src/diagnostics/mutability_errors.rs | 393 +++++++++--------- tests/ui/suggestions/issue-68049-2.stderr | 8 +- 2 files changed, 201 insertions(+), 200 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index a98a07a74e584..6286033e0672d 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1,4 +1,4 @@ -use rustc_errors::{Applicability, Diagnostic}; +use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::intravisit::Visitor; use rustc_hir::Node; @@ -478,179 +478,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { match self.local_names[local] { Some(name) if !local_decl.from_compiler_desugaring() => { - let label = match *local_decl.local_info() { - LocalInfo::User(mir::BindingForm::ImplicitSelf(_)) => { - let (span, suggestion) = - suggest_ampmut_self(self.infcx.tcx, local_decl); - Some((true, span, suggestion)) - } - - LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm { - binding_mode: ty::BindingMode::BindByValue(_), - opt_ty_info, - .. - })) => { - // check if the RHS is from desugaring - let opt_assignment_rhs_span = - self.body.find_assignments(local).first().map(|&location| { - if let Some(mir::Statement { - source_info: _, - kind: - mir::StatementKind::Assign(box ( - _, - mir::Rvalue::Use(mir::Operand::Copy(place)), - )), - }) = self.body[location.block] - .statements - .get(location.statement_index) - { - self.body.local_decls[place.local].source_info.span - } else { - self.body.source_info(location).span - } - }); - match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) { - // on for loops, RHS points to the iterator part - Some(DesugaringKind::ForLoop) => { - self.suggest_similar_mut_method_for_for_loop(&mut err); - err.span_label(opt_assignment_rhs_span.unwrap(), format!( - "this iterator yields `{pointer_sigil}` {pointer_desc}s", - )); - None - } - // don't create labels for compiler-generated spans - Some(_) => None, - None => { - let label = if name != kw::SelfLower { - suggest_ampmut( - self.infcx.tcx, - local_decl, - opt_assignment_rhs_span, - opt_ty_info, - ) - } else { - match local_decl.local_info() { - LocalInfo::User(mir::BindingForm::Var( - mir::VarBindingForm { - opt_ty_info: None, .. - }, - )) => { - let (span, sugg) = suggest_ampmut_self( - self.infcx.tcx, - local_decl, - ); - (true, span, sugg) - } - // explicit self (eg `self: &'a Self`) - _ => suggest_ampmut( - self.infcx.tcx, - local_decl, - opt_assignment_rhs_span, - opt_ty_info, - ), - } - }; - Some(label) - } - } - } - - LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm { - binding_mode: ty::BindingMode::BindByReference(_), - .. - })) => { - let pattern_span: Span = local_decl.source_info.span; - suggest_ref_mut(self.infcx.tcx, pattern_span) - .map(|span| (true, span, "mut ".to_owned())) - } - - _ => unreachable!(), - }; - - match label { - Some((true, err_help_span, suggested_code)) => { - let (is_trait_sig, local_trait) = self.is_error_in_trait(local); - if !is_trait_sig { - err.span_suggestion_verbose( - err_help_span, - format!( - "consider changing this to be a mutable {pointer_desc}" - ), - suggested_code, - Applicability::MachineApplicable, - ); - } else if let Some(x) = local_trait { - err.span_suggestion_verbose( - x, - format!( - "consider changing that to be a mutable {pointer_desc}" - ), - suggested_code, - Applicability::MachineApplicable, - ); - } - } - Some((false, err_label_span, message)) => { - struct BindingFinder { - span: Span, - hir_id: Option, - } - - impl<'tcx> Visitor<'tcx> for BindingFinder { - fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) { - if let hir::StmtKind::Local(local) = s.kind { - if local.pat.span == self.span { - self.hir_id = Some(local.hir_id); - } - } - hir::intravisit::walk_stmt(self, s); - } - } - let hir_map = self.infcx.tcx.hir(); - let def_id = self.body.source.def_id(); - let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local()); - let node = hir_map.find(hir_id); - let hir_id = if let Some(hir::Node::Item(item)) = node - && let hir::ItemKind::Fn(.., body_id) = item.kind - { - let body = hir_map.body(body_id); - let mut v = BindingFinder { - span: err_label_span, - hir_id: None, - }; - v.visit_body(body); - v.hir_id - } else { - None - }; - if let Some(hir_id) = hir_id - && let Some(hir::Node::Local(local)) = hir_map.find(hir_id) - { - let (changing, span, sugg) = match local.ty { - Some(ty) => ("changing", ty.span, message), - None => ( - "specifying", - local.pat.span.shrink_to_hi(), - format!(": {message}"), - ), - }; - err.span_suggestion_verbose( - span, - format!("consider {changing} this binding's type"), - sugg, - Applicability::HasPlaceholders, - ); - } else { - err.span_label( - err_label_span, - format!( - "consider changing this binding's type to be: `{message}`" - ), - ); - } - } - None => {} - } err.span_label( span, format!( @@ -658,6 +485,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { so the data it refers to cannot be {acted_on}", ), ); + + self.suggest_make_local_mut(&mut err, local, name); } _ => { err.span_label( @@ -1131,6 +960,184 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } } + + fn suggest_make_local_mut( + &self, + err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>, + local: Local, + name: Symbol, + ) { + let local_decl = &self.body.local_decls[local]; + + let (pointer_sigil, pointer_desc) = + if local_decl.ty.is_ref() { ("&", "reference") } else { ("*const", "pointer") }; + + let (is_trait_sig, local_trait) = self.is_error_in_trait(local); + if is_trait_sig && local_trait.is_none() { + return; + } + + let decl_span = match local_trait { + Some(span) => span, + None => local_decl.source_info.span, + }; + + let label = match *local_decl.local_info() { + LocalInfo::User(mir::BindingForm::ImplicitSelf(_)) => { + let suggestion = suggest_ampmut_self(self.infcx.tcx, decl_span); + Some((true, decl_span, suggestion)) + } + + LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByValue(_), + opt_ty_info, + .. + })) => { + // check if the RHS is from desugaring + let opt_assignment_rhs_span = + self.body.find_assignments(local).first().map(|&location| { + if let Some(mir::Statement { + source_info: _, + kind: + mir::StatementKind::Assign(box ( + _, + mir::Rvalue::Use(mir::Operand::Copy(place)), + )), + }) = self.body[location.block].statements.get(location.statement_index) + { + self.body.local_decls[place.local].source_info.span + } else { + self.body.source_info(location).span + } + }); + match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) { + // on for loops, RHS points to the iterator part + Some(DesugaringKind::ForLoop) => { + self.suggest_similar_mut_method_for_for_loop(err); + err.span_label( + opt_assignment_rhs_span.unwrap(), + format!("this iterator yields `{pointer_sigil}` {pointer_desc}s",), + ); + None + } + // don't create labels for compiler-generated spans + Some(_) => None, + None => { + let label = if name != kw::SelfLower { + suggest_ampmut( + self.infcx.tcx, + local_decl.ty, + decl_span, + opt_assignment_rhs_span, + opt_ty_info, + ) + } else { + match local_decl.local_info() { + LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm { + opt_ty_info: None, + .. + })) => { + let sugg = suggest_ampmut_self(self.infcx.tcx, decl_span); + (true, decl_span, sugg) + } + // explicit self (eg `self: &'a Self`) + _ => suggest_ampmut( + self.infcx.tcx, + local_decl.ty, + decl_span, + opt_assignment_rhs_span, + opt_ty_info, + ), + } + }; + Some(label) + } + } + } + + LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByReference(_), + .. + })) => { + let pattern_span: Span = local_decl.source_info.span; + suggest_ref_mut(self.infcx.tcx, pattern_span) + .map(|span| (true, span, "mut ".to_owned())) + } + + _ => unreachable!(), + }; + + match label { + Some((true, err_help_span, suggested_code)) => { + err.span_suggestion_verbose( + err_help_span, + format!("consider changing this to be a mutable {pointer_desc}"), + suggested_code, + Applicability::MachineApplicable, + ); + } + Some((false, err_label_span, message)) => { + struct BindingFinder { + span: Span, + hir_id: Option, + } + + impl<'tcx> Visitor<'tcx> for BindingFinder { + fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) { + if let hir::StmtKind::Local(local) = s.kind { + if local.pat.span == self.span { + self.hir_id = Some(local.hir_id); + } + } + hir::intravisit::walk_stmt(self, s); + } + } + let hir_map = self.infcx.tcx.hir(); + let def_id = self.body.source.def_id(); + let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local()); + let node = hir_map.find(hir_id); + let hir_id = if let Some(hir::Node::Item(item)) = node + && let hir::ItemKind::Fn(.., body_id) = item.kind + { + let body = hir_map.body(body_id); + let mut v = BindingFinder { + span: err_label_span, + hir_id: None, + }; + v.visit_body(body); + v.hir_id + } else { + None + }; + if let Some(hir_id) = hir_id + && let Some(hir::Node::Local(local)) = hir_map.find(hir_id) + { + let (changing, span, sugg) = match local.ty { + Some(ty) => ("changing", ty.span, message), + None => ( + "specifying", + local.pat.span.shrink_to_hi(), + format!(": {message}"), + ), + }; + err.span_suggestion_verbose( + span, + format!("consider {changing} this binding's type"), + sugg, + Applicability::HasPlaceholders, + ); + } else { + err.span_label( + err_label_span, + format!( + "consider changing this binding's type to be: `{message}`" + ), + ); + } + } + None => {} + } + } } pub fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option) -> bool { @@ -1160,25 +1167,18 @@ pub fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option< } } -fn suggest_ampmut_self<'tcx>( - tcx: TyCtxt<'tcx>, - local_decl: &mir::LocalDecl<'tcx>, -) -> (Span, String) { - let sp = local_decl.source_info.span; - ( - sp, - match tcx.sess.source_map().span_to_snippet(sp) { - Ok(snippet) => { - let lt_pos = snippet.find('\''); - if let Some(lt_pos) = lt_pos { - format!("&{}mut self", &snippet[lt_pos..snippet.len() - 4]) - } else { - "&mut self".to_string() - } +fn suggest_ampmut_self<'tcx>(tcx: TyCtxt<'tcx>, span: Span) -> String { + match tcx.sess.source_map().span_to_snippet(span) { + Ok(snippet) => { + let lt_pos = snippet.find('\''); + if let Some(lt_pos) = lt_pos { + format!("&{}mut self", &snippet[lt_pos..snippet.len() - 4]) + } else { + "&mut self".to_string() } - _ => "&mut self".to_string(), - }, - ) + } + _ => "&mut self".to_string(), + } } // When we want to suggest a user change a local variable to be a `&mut`, there @@ -1198,7 +1198,8 @@ fn suggest_ampmut_self<'tcx>( // by trying (3.), then (2.) and finally falling back on (1.). fn suggest_ampmut<'tcx>( tcx: TyCtxt<'tcx>, - local_decl: &mir::LocalDecl<'tcx>, + decl_ty: Ty<'tcx>, + decl_span: Span, opt_assignment_rhs_span: Option, opt_ty_info: Option, ) -> (bool, Span, String) { @@ -1250,7 +1251,7 @@ fn suggest_ampmut<'tcx>( // otherwise, we'll suggest *adding* an annotated type, we'll suggest // the RHS's type for that. // this is `Applicability::HasPlaceholders`. - None => (false, local_decl.source_info.span), + None => (false, decl_span), }; // if the binding already exists and is a reference with a explicit @@ -1271,13 +1272,13 @@ fn suggest_ampmut<'tcx>( } else { // otherwise, suggest that the user annotates the binding; we provide the // type of the local. - let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); + let ty_mut = decl_ty.builtin_deref(true).unwrap(); assert_eq!(ty_mut.mutbl, hir::Mutability::Not); ( false, span, - format!("{}mut {}", if local_decl.ty.is_ref() {"&"} else {"*"}, ty_mut.ty) + format!("{}mut {}", if decl_ty.is_ref() {"&"} else {"*"}, ty_mut.ty) ) } } diff --git a/tests/ui/suggestions/issue-68049-2.stderr b/tests/ui/suggestions/issue-68049-2.stderr index 833799081598f..6f3c78443f829 100644 --- a/tests/ui/suggestions/issue-68049-2.stderr +++ b/tests/ui/suggestions/issue-68049-2.stderr @@ -4,10 +4,10 @@ error[E0594]: cannot assign to `*input`, which is behind a `&` reference LL | *input = self.0; | ^^^^^^^^^^^^^^^ `input` is a `&` reference, so the data it refers to cannot be written | -help: consider changing that to be a mutable reference +help: consider changing this to be a mutable reference | -LL | fn example(&self, input: mut ); // should suggest here - | ~~~ +LL | fn example(&self, input: &mut i32) { // should not suggest here + | +++ error[E0594]: cannot assign to `self.0`, which is behind a `&` reference --> $DIR/issue-68049-2.rs:17:5 @@ -15,7 +15,7 @@ error[E0594]: cannot assign to `self.0`, which is behind a `&` reference LL | self.0 += *input; | ^^^^^^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be written | -help: consider changing that to be a mutable reference +help: consider changing this to be a mutable reference | LL | fn example(&mut self, input: &i32); // should suggest here | ~~~~~~~~~