Skip to content

Provide more context on Fn closure modifying binding #133149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 50 additions & 8 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
}
}
PlaceRef { local: _, projection: [proj_base @ .., ProjectionElem::Deref] } => {
if the_place_err.local == ty::CAPTURE_STRUCT_LOCAL
PlaceRef { local, projection: [proj_base @ .., ProjectionElem::Deref] } => {
if local == ty::CAPTURE_STRUCT_LOCAL
&& proj_base.is_empty()
&& !self.upvars.is_empty()
{
Expand All @@ -136,10 +136,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
", as `Fn` closures cannot mutate their captured variables".to_string()
}
} else {
let source = self.borrowed_content_source(PlaceRef {
local: the_place_err.local,
projection: proj_base,
});
let source =
self.borrowed_content_source(PlaceRef { local, projection: proj_base });
let pointer_type = source.describe_for_immutable_place(self.infcx.tcx);
opt_source = Some(source);
if let Some(desc) = self.describe_place(access_place.as_ref()) {
Expand Down Expand Up @@ -506,6 +504,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
PlaceRef { local, projection: [ProjectionElem::Deref] }
if local == ty::CAPTURE_STRUCT_LOCAL && !self.upvars.is_empty() =>
{
self.point_at_binding_outside_closure(&mut err, local, access_place);
self.expected_fn_found_fn_mut_call(&mut err, span, act);
}

Expand Down Expand Up @@ -923,6 +922,50 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
}

/// When modifying a binding from inside of an `Fn` closure, point at the binding definition.
fn point_at_binding_outside_closure(
&self,
err: &mut Diag<'_>,
local: Local,
access_place: Place<'tcx>,
) {
let place = access_place.as_ref();
for (index, elem) in place.projection.into_iter().enumerate() {
if let ProjectionElem::Deref = elem {
if index == 0 {
if self.body.local_decls[local].is_ref_for_guard() {
continue;
}
if let LocalInfo::StaticRef { .. } = *self.body.local_decls[local].local_info()
{
continue;
}
}
if let Some(field) = self.is_upvar_field_projection(PlaceRef {
local,
projection: place.projection.split_at(index + 1).0,
}) {
let var_index = field.index();
let upvar = self.upvars[var_index];
if let Some(hir_id) = upvar.info.capture_kind_expr_id {
let node = self.infcx.tcx.hir_node(hir_id);
if let hir::Node::Expr(expr) = node
&& let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind
&& let hir::def::Res::Local(hir_id) = path.res
&& let hir::Node::Pat(pat) = self.infcx.tcx.hir_node(hir_id)
{
let name = upvar.to_string(self.infcx.tcx);
err.span_label(
pat.span,
format!("`{name}` declared here, outside the closure"),
);
break;
}
}
}
}
}
}
/// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected.
fn expected_fn_found_fn_mut_call(&self, err: &mut Diag<'_>, sp: Span, act: &str) {
err.span_label(sp, format!("cannot {act}"));
Expand All @@ -935,6 +978,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let def_id = tcx.hir_enclosing_body_owner(fn_call_id);
let mut look_at_return = true;

err.span_label(closure_span, "in this closure");
// If the HIR node is a function or method call gets the def ID
// of the called function or method and the span and args of the call expr
let get_call_details = || {
Expand Down Expand Up @@ -1005,7 +1049,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
if let Some(span) = arg {
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
err.span_label(call_span, "expects `Fn` instead of `FnMut`");
err.span_label(closure_span, "in this closure");
look_at_return = false;
}
}
Expand All @@ -1032,7 +1075,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
sig.decl.output.span(),
"change this to return `FnMut` instead of `Fn`",
);
err.span_label(closure_span, "in this closure");
}
_ => {}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/async-await/async-closures/wrong-fn-kind.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
LL | fn needs_async_fn(_: impl AsyncFn()) {}
| -------------- change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = 1;
| ----- `x` declared here, outside the closure
LL | needs_async_fn(async || {
| -------------- ^^^^^^^^
| | |
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/borrowck/borrow-immutable-upvar-mutation.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = 0;
| ----- `x` declared here, outside the closure
LL | let _f = to_fn(|| x = 42);
| ----- -- ^^^^^^ cannot assign
| | |
Expand All @@ -16,6 +18,8 @@ error[E0596]: cannot borrow `y` as mutable, as it is a captured variable in a `F
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut y = 0;
| ----- `y` declared here, outside the closure
LL | let _g = to_fn(|| set(&mut y));
| ----- -- ^^^^^^ cannot borrow as mutable
| | |
Expand All @@ -28,6 +32,9 @@ error[E0594]: cannot assign to `z`, as it is a captured variable in a `Fn` closu
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut z = 0;
| ----- `z` declared here, outside the closure
...
LL | to_fn(|| z = 42);
| ----- -- ^^^^^^ cannot assign
| | |
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/borrowck/borrow-raw-address-of-mutability.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
LL | fn make_fn<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = 0;
| ----- `x` declared here, outside the closure
LL | let f = make_fn(|| {
| ------- -- in this closure
| |
Expand Down
16 changes: 12 additions & 4 deletions tests/ui/borrowck/mutability-errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
|
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL |
LL | fn ref_closure(mut x: (i32,)) {
| ----- `x` declared here, outside the closure
LL | fn_ref(|| {
| ------ -- in this closure
| |
Expand All @@ -152,7 +154,9 @@ error[E0594]: cannot assign to `x.0`, as `Fn` closures cannot mutate their captu
|
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL |
LL | fn ref_closure(mut x: (i32,)) {
| ----- `x` declared here, outside the closure
LL | fn_ref(|| {
| ------ -- in this closure
| |
Expand All @@ -166,7 +170,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
|
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL |
LL | fn ref_closure(mut x: (i32,)) {
| ----- `x` declared here, outside the closure
LL | fn_ref(|| {
| ------ -- in this closure
| |
Expand All @@ -180,7 +186,9 @@ error[E0596]: cannot borrow `x.0` as mutable, as `Fn` closures cannot mutate the
|
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL |
LL | fn ref_closure(mut x: (i32,)) {
| ----- `x` declared here, outside the closure
LL | fn_ref(|| {
| ------ -- in this closure
| |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
LL | fn call_it<F>(f: F) where F: Fn() { f(); }
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = A;
| ----- `x` declared here, outside the closure
...
LL | call_it(|| x.gen_mut());
| ------- -- ^ cannot borrow as mutable
| | |
Expand All @@ -16,6 +19,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
LL | fn call_it<F>(f: F) where F: Fn() { f(); }
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = A;
| ----- `x` declared here, outside the closure
LL | call_it(|| {
| ------- -- in this closure
| |
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/closures/wrong-closure-arg-suggestion-125325.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
LL | fn assoc_func(&self, _f: impl Fn()) -> usize {
| --------- change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = ();
| ----- `x` declared here, outside the closure
LL | s.assoc_func(|| x = ());
| --------------^^^^^^-
| | | |
Expand All @@ -17,6 +19,9 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
LL | fn func(_f: impl Fn()) -> usize {
| --------- change this to accept `FnMut` instead of `Fn`
...
LL | let mut x = ();
| ----- `x` declared here, outside the closure
...
LL | func(|| x = ())
| ---- -- ^^^^^^ cannot assign
| | |
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/nll/closure-captures.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
|
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL |
LL | fn two_closures_ref_mut(mut x: i32) {
| ----- `x` declared here, outside the closure
LL | fn_ref(|| {
| ------ -- in this closure
| |
Expand Down Expand Up @@ -89,6 +91,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
| - change this to accept `FnMut` instead of `Fn`
...
LL | fn two_closures_ref(x: i32) {
| - `x` declared here, outside the closure
LL | fn_ref(|| {
| ------ -- in this closure
| |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ error[E0594]: cannot assign to `counter`, as it is a captured variable in a `Fn`
LL | fn call<F>(f: F) where F : Fn() {
| - change this to accept `FnMut` instead of `Fn`
...
LL | let mut counter = 0;
| ----------- `counter` declared here, outside the closure
LL | call(|| {
| ---- -- in this closure
| |
Expand Down
Loading