Point to span of upvar making closure FnMut

Add expected error

Add comment

Tweak comment wording

Fix after rebase to updated master

Fix after rebase to updated master

Distinguish mutation in normal and move closures

Tweak error message

Fix error message for nested closures

Refactor code showing mutated upvar in closure

Remove debug assert

B

Author: sledgehammervampire

compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs

@@ -1,9 +1,12 @@
 use rustc_hir as hir;
 use rustc_hir::Node;
 use rustc_index::vec::Idx;
-use rustc_middle::mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, Location};
 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, Location},
+};
 use rustc_span::source_map::DesugaringKind;
 use rustc_span::symbol::{kw, Symbol};
 use rustc_span::Span;
@@ -241,6 +244,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     format!("mut {}", self.local_names[local].unwrap()),
                     Applicability::MachineApplicable,
                 );
+                let tcx = self.infcx.tcx;
+                if let ty::Closure(id, _) = the_place_err.ty(self.body, tcx).ty.kind() {
+                    self.show_mutating_upvar(tcx, id, the_place_err, &mut err);
+                }
             }
 
             // Also suggest adding mut for upvars
@@ -271,6 +278,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         );
                     }
                 }
+
+                let tcx = self.infcx.tcx;
+                if let ty::Ref(_, ty, Mutability::Mut) = the_place_err.ty(self.body, tcx).ty.kind()
+                {
+                    if let ty::Closure(id, _) = ty.kind() {
+                        self.show_mutating_upvar(tcx, id, the_place_err, &mut err);
+                    }
+                }
             }
 
             // complete hack to approximate old AST-borrowck
@@ -463,6 +478,45 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         err.buffer(&mut self.errors_buffer);
     }
 
+    // point to span of upvar making closure call require mutable borrow
+    fn show_mutating_upvar(
+        &self,
+        tcx: TyCtxt<'_>,
+        id: &hir::def_id::DefId,
+        the_place_err: PlaceRef<'tcx>,
+        err: &mut DiagnosticBuilder<'_>,
+    ) {
+        let id = id.expect_local();
+        let tables = tcx.typeck(id);
+        let hir_id = tcx.hir().local_def_id_to_hir_id(id);
+        let (span, place) = &tables.closure_kind_origins()[hir_id];
+        let reason = if let PlaceBase::Upvar(upvar_id) = place.base {
+            let upvar = ty::place_to_string_for_capture(tcx, place);
+            match tables.upvar_capture(upvar_id) {
+                ty::UpvarCapture::ByRef(ty::UpvarBorrow {
+                    kind: ty::BorrowKind::MutBorrow,
+                    ..
+                }) => {
+                    format!("mutable borrow of `{}`", upvar)
+                }
+                ty::UpvarCapture::ByValue(_) => {
+                    format!("possible mutation of `{}`", upvar)
+                }
+                _ => bug!("upvar `{}` borrowed, but not mutably", upvar),
+            }
+        } else {
+            bug!("not an upvar")
+        };
+        err.span_label(
+            *span,
+            format!(
+                "calling `{}` requires mutable binding due to {}",
+                self.describe_place(the_place_err).unwrap(),
+                reason
+            ),
+        );
+    }
+
     /// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected.
     fn expected_fn_found_fn_mut_call(&self, err: &mut DiagnosticBuilder<'_>, sp: Span, act: &str) {
         err.span_label(sp, format!("cannot {}", act));

src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr

@@ -20,7 +20,9 @@ error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable
    |
 LL |     let f = || {
    |         - help: consider changing this to be mutable: `mut f`
-...
+LL |         let y = &raw mut x;
+   |                          - calling `f` requires mutable binding due to mutable borrow of `x`
+LL |     };
 LL |     f();
    |     ^ cannot borrow as mutable
 

src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs

@@ -0,0 +1,7 @@
+fn main() {
+    let mut my_var = false;
+    let callback = || {
+        &mut my_var;
+    };
+    callback(); //~ ERROR E0596
+}

src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr

@@ -0,0 +1,14 @@
+error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable
+  --> $DIR/issue-80313-mutable-borrow-in-closure.rs:6:5
+   |
+LL |     let callback = || {
+   |         -------- help: consider changing this to be mutable: `mut callback`
+LL |         &mut my_var;
+   |              ------ calling `callback` requires mutable binding due to mutable borrow of `my_var`
+LL |     };
+LL |     callback();
+   |     ^^^^^^^^ cannot borrow as mutable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0596`.

src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs

@@ -0,0 +1,7 @@
+fn main() {
+    let mut my_var = false;
+    let callback = move || {
+        &mut my_var;
+    };
+    callback(); //~ ERROR E0596
+}

src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr

@@ -0,0 +1,14 @@
+error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable
+  --> $DIR/issue-80313-mutable-borrow-in-move-closure.rs:6:5
+   |
+LL |     let callback = move || {
+   |         -------- help: consider changing this to be mutable: `mut callback`
+LL |         &mut my_var;
+   |              ------ calling `callback` requires mutable binding due to possible mutation of `my_var`
+LL |     };
+LL |     callback();
+   |     ^^^^^^^^ cannot borrow as mutable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0596`.

src/test/ui/closures/issue-80313-mutation-in-closure.rs

@@ -0,0 +1,7 @@
+fn main() {
+    let mut my_var = false;
+    let callback = || {
+        my_var = true;
+    };
+    callback(); //~ ERROR E0596
+}

src/test/ui/closures/issue-80313-mutation-in-closure.stderr

@@ -0,0 +1,14 @@
+error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable
+  --> $DIR/issue-80313-mutation-in-closure.rs:6:5
+   |
+LL |     let callback = || {
+   |         -------- help: consider changing this to be mutable: `mut callback`
+LL |         my_var = true;
+   |         ------ calling `callback` requires mutable binding due to mutable borrow of `my_var`
+LL |     };
+LL |     callback();
+   |     ^^^^^^^^ cannot borrow as mutable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0596`.

src/test/ui/closures/issue-80313-mutation-in-move-closure.rs

@@ -0,0 +1,7 @@
+fn main() {
+    let mut my_var = false;
+    let callback = move || {
+        my_var = true;
+    };
+    callback(); //~ ERROR E0596
+}

src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr

@@ -0,0 +1,14 @@
+error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable
+  --> $DIR/issue-80313-mutation-in-move-closure.rs:6:5
+   |
+LL |     let callback = move || {
+   |         -------- help: consider changing this to be mutable: `mut callback`
+LL |         my_var = true;
+   |         ------ calling `callback` requires mutable binding due to possible mutation of `my_var`
+LL |     };
+LL |     callback();
+   |     ^^^^^^^^ cannot borrow as mutable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0596`.

src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr

@@ -3,6 +3,8 @@ error[E0596]: cannot borrow `tick1` as mutable, as it is not declared as mutable
    |
 LL |     let tick1 = || {
    |         ----- help: consider changing this to be mutable: `mut tick1`
+LL |         counter += 1;
+   |         ------- calling `tick1` requires mutable binding due to mutable borrow of `counter`
 ...
 LL |         tick1();
    |         ^^^^^ cannot borrow as mutable
@@ -12,6 +14,8 @@ error[E0596]: cannot borrow `tick2` as mutable, as it is not declared as mutable
    |
 LL |     let tick2 = || {
    |         ----- help: consider changing this to be mutable: `mut tick2`
+LL |         tick1();
+   |         ----- calling `tick2` requires mutable binding due to mutable borrow of `tick1`
 ...
 LL |     tick2();
    |     ^^^^^ cannot borrow as mutable

src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr

@@ -2,7 +2,9 @@ error[E0596]: cannot borrow `tick` as mutable, as it is not declared as mutable
   --> $DIR/unboxed-closures-infer-fnmut-missing-mut.rs:7:5
    |
 LL |     let tick = || counter += 1;
-   |         ---- help: consider changing this to be mutable: `mut tick`
+   |         ----      ------- calling `tick` requires mutable binding due to mutable borrow of `counter`
+   |         |
+   |         help: consider changing this to be mutable: `mut tick`
 LL |     tick();
    |     ^^^^ cannot borrow as mutable
 

src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr

@@ -2,7 +2,9 @@ error[E0596]: cannot borrow `tick` as mutable, as it is not declared as mutable
   --> $DIR/unboxed-closures-infer-fnmut-move-missing-mut.rs:7:5
    |
 LL |     let tick = move || counter += 1;
-   |         ---- help: consider changing this to be mutable: `mut tick`
+   |         ----           ------- calling `tick` requires mutable binding due to possible mutation of `counter`
+   |         |
+   |         help: consider changing this to be mutable: `mut tick`
 LL |     tick();
    |     ^^^^ cannot borrow as mutable