user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #133149 - estebank:niko-rustnation, r=wesleywiser

Browse source 
Provide more context on `Fn` closure modifying binding

When modifying a binding from inside of an `Fn` closure, point at the binding definition and suggest using an `std::sync` type that would allow the code to compile.

```
error[E0594]: cannot assign to `counter`, as it is a captured variable in a `Fn` closure
 --> f703.rs:6:9
  |
4 |     let mut counter = 0;
  |         ----------- `counter` declared here, outside the closure
5 |     let x: Box<dyn Fn()> = Box::new(|| {
  |                                     -- in this closure
6 |         counter += 1;
  |         ^^^^^^^^^^^^ cannot assign
  |
  = help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
```

This provides more context on where the binding being modified was declared, and more importantly, guides newcomers towards `std::sync` when encountering cases like these.

When the requirement comes from an argument of a local function, we already tell the user that they might want to change from `Fn` to `FnMut`:

```
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
  --> $DIR/borrow-immutable-upvar-mutation.rs:21:27
   |
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
   |                  |     |
   |                  |     in this closure
   |                  expects `Fn` instead of `FnMut`
   |
   = help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
```

We might want to avoid the `help` in this case.

_Inspired by a [part of Niko's keynote at RustNation UK 2024](https://youtu.be/04gTQmLETFI?si=dgJL2OJRtuShkxdD&t=600)._
This commit is contained in:
Stuart Cook 2025-11-04 13:44:47 +11:00 • committed by GitHub
commit ea4e037dd7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 90 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

58
compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View file

@ -150,8 +150,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()
{
@ -165,10 +165,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()) {
@ -540,6 +538,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);
}
@ -958,6 +957,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}"));
@ -970,6 +1013,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, get the DefId
// of the callee function or method, the span, and args of the call expr
let get_call_details = || {
@ -1040,7 +1084,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;
}
}
@ -1067,7 +1110,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");
}
_ => {}
}

2
tests/ui/async-await/async-closures/wrong-fn-kind.stderr
View file

@ -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 || {
| -------------- ^^^^^^^^
| | |

7
tests/ui/borrowck/borrow-immutable-upvar-mutation.stderr
View file

@ -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
| | |
@ -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
| | |
@ -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
| | |

2
tests/ui/borrowck/borrow-raw-address-of-mutability.stderr
View file

@ -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
| |

16
tests/ui/borrowck/mutability-errors.stderr
View file

@ -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
| |
@ -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
| |
@ -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
| |
@ -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
| |

5
tests/ui/closures/aliasability-violation-with-closure-21600.stderr
View file

@ -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
| | |
@ -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
| |

5
tests/ui/closures/wrong-closure-arg-suggestion-125325.stderr
View file

@ -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 = ());
| --------------^^^^^^-
| | | |
@ -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
| | |

6
tests/ui/nll/closure-captures.stderr
View file

@ -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
| |
@ -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
| |

2
tests/ui/unboxed-closures/unboxed-closures-mutated-upvar-from-fn-closure.stderr
View file

@ -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
| |