Skip to content

Commit 4ab1fc5

Browse files
committed
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 ```
1 parent 35ebdf9 commit 4ab1fc5

File tree

9 files changed

+90
-13
lines changed

9 files changed

+90
-13
lines changed

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
150150
}
151151
}
152152
}
153-
PlaceRef { local: _, projection: [proj_base @ .., ProjectionElem::Deref] } => {
154-
if the_place_err.local == ty::CAPTURE_STRUCT_LOCAL
153+
PlaceRef { local, projection: [proj_base @ .., ProjectionElem::Deref] } => {
154+
if local == ty::CAPTURE_STRUCT_LOCAL
155155
&& proj_base.is_empty()
156156
&& !self.upvars.is_empty()
157157
{
@@ -165,10 +165,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
165165
", as `Fn` closures cannot mutate their captured variables".to_string()
166166
}
167167
} else {
168-
let source = self.borrowed_content_source(PlaceRef {
169-
local: the_place_err.local,
170-
projection: proj_base,
171-
});
168+
let source =
169+
self.borrowed_content_source(PlaceRef { local, projection: proj_base });
172170
let pointer_type = source.describe_for_immutable_place(self.infcx.tcx);
173171
opt_source = Some(source);
174172
if let Some(desc) = self.describe_place(access_place.as_ref()) {
@@ -532,6 +530,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
532530
PlaceRef { local, projection: [ProjectionElem::Deref] }
533531
if local == ty::CAPTURE_STRUCT_LOCAL && !self.upvars.is_empty() =>
534532
{
533+
self.point_at_binding_outside_closure(&mut err, local, access_place);
535534
self.expected_fn_found_fn_mut_call(&mut err, span, act);
536535
}
537536

@@ -950,6 +949,50 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
950949
}
951950
}
952951

952+
/// When modifying a binding from inside of an `Fn` closure, point at the binding definition.
953+
fn point_at_binding_outside_closure(
954+
&self,
955+
err: &mut Diag<'_>,
956+
local: Local,
957+
access_place: Place<'tcx>,
958+
) {
959+
let place = access_place.as_ref();
960+
for (index, elem) in place.projection.into_iter().enumerate() {
961+
if let ProjectionElem::Deref = elem {
962+
if index == 0 {
963+
if self.body.local_decls[local].is_ref_for_guard() {
964+
continue;
965+
}
966+
if let LocalInfo::StaticRef { .. } = *self.body.local_decls[local].local_info()
967+
{
968+
continue;
969+
}
970+
}
971+
if let Some(field) = self.is_upvar_field_projection(PlaceRef {
972+
local,
973+
projection: place.projection.split_at(index + 1).0,
974+
}) {
975+
let var_index = field.index();
976+
let upvar = self.upvars[var_index];
977+
if let Some(hir_id) = upvar.info.capture_kind_expr_id {
978+
let node = self.infcx.tcx.hir_node(hir_id);
979+
if let hir::Node::Expr(expr) = node
980+
&& let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind
981+
&& let hir::def::Res::Local(hir_id) = path.res
982+
&& let hir::Node::Pat(pat) = self.infcx.tcx.hir_node(hir_id)
983+
{
984+
let name = upvar.to_string(self.infcx.tcx);
985+
err.span_label(
986+
pat.span,
987+
format!("`{name}` declared here, outside the closure"),
988+
);
989+
break;
990+
}
991+
}
992+
}
993+
}
994+
}
995+
}
953996
/// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected.
954997
fn expected_fn_found_fn_mut_call(&self, err: &mut Diag<'_>, sp: Span, act: &str) {
955998
err.span_label(sp, format!("cannot {act}"));
@@ -962,6 +1005,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
9621005
let def_id = tcx.hir_enclosing_body_owner(fn_call_id);
9631006
let mut look_at_return = true;
9641007

1008+
err.span_label(closure_span, "in this closure");
9651009
// If the HIR node is a function or method call, get the DefId
9661010
// of the callee function or method, the span, and args of the call expr
9671011
let get_call_details = || {
@@ -1032,7 +1076,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10321076
if let Some(span) = arg {
10331077
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
10341078
err.span_label(call_span, "expects `Fn` instead of `FnMut`");
1035-
err.span_label(closure_span, "in this closure");
10361079
look_at_return = false;
10371080
}
10381081
}
@@ -1059,7 +1102,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10591102
sig.decl.output.span(),
10601103
"change this to return `FnMut` instead of `Fn`",
10611104
);
1062-
err.span_label(closure_span, "in this closure");
10631105
}
10641106
_ => {}
10651107
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
2525
LL | fn needs_async_fn(_: impl AsyncFn()) {}
2626
| -------------- change this to accept `FnMut` instead of `Fn`
2727
...
28+
LL | let mut x = 1;
29+
| ----- `x` declared here, outside the closure
2830
LL | needs_async_fn(async || {
2931
| -------------- ^^^^^^^^
3032
| | |

tests/ui/borrowck/borrow-immutable-upvar-mutation.stderr

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
44
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
55
| - change this to accept `FnMut` instead of `Fn`
66
...
7+
LL | let mut x = 0;
8+
| ----- `x` declared here, outside the closure
79
LL | let _f = to_fn(|| x = 42);
810
| ----- -- ^^^^^^ cannot assign
911
| | |
@@ -16,6 +18,8 @@ error[E0596]: cannot borrow `y` as mutable, as it is a captured variable in a `F
1618
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
1719
| - change this to accept `FnMut` instead of `Fn`
1820
...
21+
LL | let mut y = 0;
22+
| ----- `y` declared here, outside the closure
1923
LL | let _g = to_fn(|| set(&mut y));
2024
| ----- -- ^^^^^^ cannot borrow as mutable
2125
| | |
@@ -28,6 +32,9 @@ error[E0594]: cannot assign to `z`, as it is a captured variable in a `Fn` closu
2832
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
2933
| - change this to accept `FnMut` instead of `Fn`
3034
...
35+
LL | let mut z = 0;
36+
| ----- `z` declared here, outside the closure
37+
...
3138
LL | to_fn(|| z = 42);
3239
| ----- -- ^^^^^^ cannot assign
3340
| | |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
4040
LL | fn make_fn<F: Fn()>(f: F) -> F { f }
4141
| - change this to accept `FnMut` instead of `Fn`
4242
...
43+
LL | let mut x = 0;
44+
| ----- `x` declared here, outside the closure
4345
LL | let f = make_fn(|| {
4446
| ------- -- in this closure
4547
| |

tests/ui/borrowck/mutability-errors.stderr

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
139139
|
140140
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
141141
| - change this to accept `FnMut` instead of `Fn`
142-
...
142+
LL |
143+
LL | fn ref_closure(mut x: (i32,)) {
144+
| ----- `x` declared here, outside the closure
143145
LL | fn_ref(|| {
144146
| ------ -- in this closure
145147
| |
@@ -152,7 +154,9 @@ error[E0594]: cannot assign to `x.0`, as `Fn` closures cannot mutate their captu
152154
|
153155
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
154156
| - change this to accept `FnMut` instead of `Fn`
155-
...
157+
LL |
158+
LL | fn ref_closure(mut x: (i32,)) {
159+
| ----- `x` declared here, outside the closure
156160
LL | fn_ref(|| {
157161
| ------ -- in this closure
158162
| |
@@ -166,7 +170,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
166170
|
167171
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
168172
| - change this to accept `FnMut` instead of `Fn`
169-
...
173+
LL |
174+
LL | fn ref_closure(mut x: (i32,)) {
175+
| ----- `x` declared here, outside the closure
170176
LL | fn_ref(|| {
171177
| ------ -- in this closure
172178
| |
@@ -180,7 +186,9 @@ error[E0596]: cannot borrow `x.0` as mutable, as `Fn` closures cannot mutate the
180186
|
181187
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
182188
| - change this to accept `FnMut` instead of `Fn`
183-
...
189+
LL |
190+
LL | fn ref_closure(mut x: (i32,)) {
191+
| ----- `x` declared here, outside the closure
184192
LL | fn_ref(|| {
185193
| ------ -- in this closure
186194
| |

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
44
LL | fn call_it<F>(f: F) where F: Fn() { f(); }
55
| - change this to accept `FnMut` instead of `Fn`
66
...
7+
LL | let mut x = A;
8+
| ----- `x` declared here, outside the closure
9+
...
710
LL | call_it(|| x.gen_mut());
811
| ------- -- ^ cannot borrow as mutable
912
| | |
@@ -16,6 +19,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
1619
LL | fn call_it<F>(f: F) where F: Fn() { f(); }
1720
| - change this to accept `FnMut` instead of `Fn`
1821
...
22+
LL | let mut x = A;
23+
| ----- `x` declared here, outside the closure
1924
LL | call_it(|| {
2025
| ------- -- in this closure
2126
| |

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
44
LL | fn assoc_func(&self, _f: impl Fn()) -> usize {
55
| --------- change this to accept `FnMut` instead of `Fn`
66
...
7+
LL | let mut x = ();
8+
| ----- `x` declared here, outside the closure
79
LL | s.assoc_func(|| x = ());
810
| --------------^^^^^^-
911
| | | |
@@ -17,6 +19,9 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
1719
LL | fn func(_f: impl Fn()) -> usize {
1820
| --------- change this to accept `FnMut` instead of `Fn`
1921
...
22+
LL | let mut x = ();
23+
| ----- `x` declared here, outside the closure
24+
...
2025
LL | func(|| x = ())
2126
| ---- -- ^^^^^^ cannot assign
2227
| | |

tests/ui/nll/closure-captures.stderr

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
4747
|
4848
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
4949
| - change this to accept `FnMut` instead of `Fn`
50-
...
50+
LL |
51+
LL | fn two_closures_ref_mut(mut x: i32) {
52+
| ----- `x` declared here, outside the closure
5153
LL | fn_ref(|| {
5254
| ------ -- in this closure
5355
| |
@@ -89,6 +91,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
8991
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
9092
| - change this to accept `FnMut` instead of `Fn`
9193
...
94+
LL | fn two_closures_ref(x: i32) {
95+
| - `x` declared here, outside the closure
9296
LL | fn_ref(|| {
9397
| ------ -- in this closure
9498
| |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ error[E0594]: cannot assign to `counter`, as it is a captured variable in a `Fn`
44
LL | fn call<F>(f: F) where F : Fn() {
55
| - change this to accept `FnMut` instead of `Fn`
66
...
7+
LL | let mut counter = 0;
8+
| ----------- `counter` declared here, outside the closure
79
LL | call(|| {
810
| ---- -- in this closure
911
| |

0 commit comments

Comments
 (0)