From 26b4baf46ea66d3651281c8b66d76853e6362c65 Mon Sep 17 00:00:00 2001 From: 1000teslas <47207223+1000teslas@users.noreply.github.com> Date: Mon, 18 Jan 2021 19:04:55 +1100 Subject: [PATCH] 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 --- .../diagnostics/mutability_errors.rs | 56 ++++++++++++++++++- .../borrow-raw-address-of-mutability.stderr | 4 +- .../issue-80313-mutable-borrow-in-closure.rs | 7 +++ ...sue-80313-mutable-borrow-in-closure.stderr | 14 +++++ ...ue-80313-mutable-borrow-in-move-closure.rs | 7 +++ ...0313-mutable-borrow-in-move-closure.stderr | 14 +++++ .../issue-80313-mutation-in-closure.rs | 7 +++ .../issue-80313-mutation-in-closure.stderr | 14 +++++ .../issue-80313-mutation-in-move-closure.rs | 7 +++ ...ssue-80313-mutation-in-move-closure.stderr | 14 +++++ ...es-infer-fnmut-calling-fnmut-no-mut.stderr | 4 ++ ...ed-closures-infer-fnmut-missing-mut.stderr | 4 +- ...osures-infer-fnmut-move-missing-mut.stderr | 4 +- 13 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr create mode 100644 src/test/ui/closures/issue-80313-mutation-in-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutation-in-closure.stderr create mode 100644 src/test/ui/closures/issue-80313-mutation-in-move-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr 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 e1af6fc07cf8f..73196c732f5bb 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/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)); diff --git a/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr b/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr index 44dde0fd80b0d..ea74fb966846f 100644 --- a/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr +++ b/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 diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs b/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs new file mode 100644 index 0000000000000..ff210ae06a3bd --- /dev/null +++ b/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 +} diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr b/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr new file mode 100644 index 0000000000000..bf9e1febdbba4 --- /dev/null +++ b/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`. diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs b/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs new file mode 100644 index 0000000000000..8f2d8a676302c --- /dev/null +++ b/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 +} diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr b/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr new file mode 100644 index 0000000000000..b67cec6a609f0 --- /dev/null +++ b/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`. diff --git a/src/test/ui/closures/issue-80313-mutation-in-closure.rs b/src/test/ui/closures/issue-80313-mutation-in-closure.rs new file mode 100644 index 0000000000000..e082ea562ef22 --- /dev/null +++ b/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 +} diff --git a/src/test/ui/closures/issue-80313-mutation-in-closure.stderr b/src/test/ui/closures/issue-80313-mutation-in-closure.stderr new file mode 100644 index 0000000000000..6e98549f6b84f --- /dev/null +++ b/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`. diff --git a/src/test/ui/closures/issue-80313-mutation-in-move-closure.rs b/src/test/ui/closures/issue-80313-mutation-in-move-closure.rs new file mode 100644 index 0000000000000..f66bf4e062831 --- /dev/null +++ b/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 +} diff --git a/src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr b/src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr new file mode 100644 index 0000000000000..edd55422a0bd4 --- /dev/null +++ b/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`. diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr index 5dea424596e9c..a0ed56d4bcf7b 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr +++ b/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 diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr index eb398628846dd..27d23e3fa044b 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr +++ b/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 diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr index b9d76d9a752ce..c00f986c397a7 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr +++ b/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