Skip to content

Commit

Permalink
Point to span of upvar making closure FnMut
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sledgehammervampire committed Jan 25, 2021
1 parent 66eb982 commit 26b4baf
Show file tree
Hide file tree
Showing 13 changed files with 152 additions and 4 deletions.
@@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions 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
}
14 changes: 14 additions & 0 deletions 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`.
@@ -0,0 +1,7 @@
fn main() {
let mut my_var = false;
let callback = move || {
&mut my_var;
};
callback(); //~ ERROR E0596
}
@@ -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`.
7 changes: 7 additions & 0 deletions 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
}
14 changes: 14 additions & 0 deletions 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`.
7 changes: 7 additions & 0 deletions 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
}
14 changes: 14 additions & 0 deletions 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`.
Expand Up @@ -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
Expand All @@ -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
Expand Down
Expand Up @@ -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

Expand Down
Expand Up @@ -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

Expand Down

0 comments on commit 26b4baf

Please sign in to comment.