Skip to content

Commit d2234c0

Browse files
estebankestebank
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 | = help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value ```
1 parent 917a50a commit d2234c0

File tree

9 files changed

+149
-13
lines changed

9 files changed

+149
-13
lines changed

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs‎

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use core::ops::ControlFlow;
55

66
use hir::{ExprKind, Param};
77
use rustc_abi::FieldIdx;
8+
use rustc_data_structures::fx::FxHashMap;
89
use rustc_errors::{Applicability, Diag};
910
use rustc_hir::intravisit::Visitor;
1011
use rustc_hir::{self as hir, BindingMode, ByRef, Node};
@@ -122,8 +123,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
122123
}
123124
}
124125
}
125-
PlaceRef { local: _, projection: [proj_base @ .., ProjectionElem::Deref] } => {
126-
if the_place_err.local == ty::CAPTURE_STRUCT_LOCAL
126+
PlaceRef { local, projection: [proj_base @ .., ProjectionElem::Deref] } => {
127+
if local == ty::CAPTURE_STRUCT_LOCAL
127128
&& proj_base.is_empty()
128129
&& !self.upvars.is_empty()
129130
{
@@ -137,10 +138,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
137138
", as `Fn` closures cannot mutate their captured variables".to_string()
138139
}
139140
} else {
140-
let source = self.borrowed_content_source(PlaceRef {
141-
local: the_place_err.local,
142-
projection: proj_base,
143-
});
141+
let source =
142+
self.borrowed_content_source(PlaceRef { local, projection: proj_base });
144143
let pointer_type = source.describe_for_immutable_place(self.infcx.tcx);
145144
opt_source = Some(source);
146145
if let Some(desc) = self.describe_place(access_place.as_ref()) {
@@ -506,6 +505,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
506505
PlaceRef { local, projection: [ProjectionElem::Deref] }
507506
if local == ty::CAPTURE_STRUCT_LOCAL && !self.upvars.is_empty() =>
508507
{
508+
self.point_at_binding_outside_closure(&mut err, local, access_place);
509509
self.expected_fn_found_fn_mut_call(&mut err, span, act);
510510
}
511511

@@ -929,6 +929,76 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
929929
}
930930
}
931931

932+
/// When modifying a binding from inside of an `Fn` closure, point at the binding definition
933+
/// and suggest using an `std::sync` type that would allow the code to compile.
934+
fn point_at_binding_outside_closure(
935+
&self,
936+
err: &mut Diag<'_>,
937+
local: Local,
938+
access_place: Place<'tcx>,
939+
) {
940+
let place = access_place.as_ref();
941+
for (index, elem) in place.projection.into_iter().enumerate() {
942+
if let ProjectionElem::Deref = elem {
943+
if index == 0 {
944+
if self.body.local_decls[local].is_ref_for_guard() {
945+
continue;
946+
}
947+
if let LocalInfo::StaticRef { .. } = *self.body.local_decls[local].local_info()
948+
{
949+
continue;
950+
}
951+
}
952+
if let Some(field) = self.is_upvar_field_projection(PlaceRef {
953+
local,
954+
projection: place.projection.split_at(index + 1).0,
955+
}) {
956+
let var_index = field.index();
957+
let upvar = self.upvars[var_index];
958+
if let Some(hir_id) = upvar.info.capture_kind_expr_id {
959+
let node = self.infcx.tcx.hir_node(hir_id);
960+
if let hir::Node::Expr(expr) = node
961+
&& let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind
962+
&& let hir::def::Res::Local(hir_id) = path.res
963+
&& let hir::Node::Pat(pat) = self.infcx.tcx.hir_node(hir_id)
964+
{
965+
let hir = self.infcx.tcx.hir();
966+
let def_id = hir.enclosing_body_owner(self.mir_hir_id());
967+
let typeck_results = self.infcx.tcx.typeck(def_id);
968+
let ty = typeck_results.node_type_opt(expr.hir_id);
969+
if let Some(ty) = ty {
970+
let mutex = format!("std::sync::atomic::Mutex<{ty}>");
971+
let mutex = mutex.as_str();
972+
let suggestions: FxHashMap<_, _> = [
973+
(self.infcx.tcx.types.isize, "std::sync::atomic::AtomicIsize"),
974+
(self.infcx.tcx.types.usize, "std::sync::atomic::AtomicUsize"),
975+
(self.infcx.tcx.types.i64, "std::sync::atomic::AtomicI64"),
976+
(self.infcx.tcx.types.u64, "std::sync::atomic::AtomicU64"),
977+
(self.infcx.tcx.types.i32, "std::sync::atomic::AtomicI32"),
978+
(self.infcx.tcx.types.u32, "std::sync::atomic::AtomicU32"),
979+
(self.infcx.tcx.types.i16, "std::sync::atomic::AtomicI16"),
980+
(self.infcx.tcx.types.u16, "std::sync::atomic::AtomicU16"),
981+
(self.infcx.tcx.types.i8, "std::sync::atomic::AtomicI8"),
982+
(self.infcx.tcx.types.u8, "std::sync::atomic::AtomicU8"),
983+
(self.infcx.tcx.types.bool, "std::sync::atomic::AtomicBool"),
984+
]
985+
.into_iter()
986+
.collect();
987+
let ty = suggestions.get(&ty).unwrap_or(&mutex);
988+
err.help(format!("consider using `{ty}` instead, which allows for multiple threads to access and modify the value"));
989+
}
990+
let name = upvar.to_string(self.infcx.tcx);
991+
err.span_label(
992+
pat.span,
993+
format!("`{name}` declared here, outside the closure"),
994+
);
995+
break;
996+
}
997+
}
998+
}
999+
}
1000+
}
1001+
}
9321002
/// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected.
9331003
fn expected_fn_found_fn_mut_call(&self, err: &mut Diag<'_>, sp: Span, act: &str) {
9341004
err.span_label(sp, format!("cannot {act}"));
@@ -941,6 +1011,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
9411011
let def_id = hir.enclosing_body_owner(fn_call_id);
9421012
let mut look_at_return = true;
9431013

1014+
err.span_label(closure_span, "in this closure");
9441015
// If the HIR node is a function or method call gets the def ID
9451016
// of the called function or method and the span and args of the call expr
9461017
let get_call_details = || {
@@ -1009,7 +1080,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10091080
if let Some(span) = arg {
10101081
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
10111082
err.span_label(call_span, "expects `Fn` instead of `FnMut`");
1012-
err.span_label(closure_span, "in this closure");
10131083
look_at_return = false;
10141084
}
10151085
}
@@ -1034,7 +1104,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10341104
sig.decl.output.span(),
10351105
"change this to return `FnMut` instead of `Fn`",
10361106
);
1037-
err.span_label(closure_span, "in this closure");
10381107
}
10391108
_ => {}
10401109
}

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

Lines changed: 4 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 async Fn()) {}
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
| | |
@@ -34,6 +36,8 @@ LL | needs_async_fn(async || {
3436
LL |
3537
LL | x += 1;
3638
| - mutable borrow occurs due to use of `x` in closure
39+
|
40+
= help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
3741

3842
error: aborting due to 2 previous errors
3943

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,48 @@ 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
| | |
1012
| | in this closure
1113
| expects `Fn` instead of `FnMut`
14+
|
15+
= help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
1216

1317
error[E0596]: cannot borrow `y` as mutable, as it is a captured variable in a `Fn` closure
1418
--> $DIR/borrow-immutable-upvar-mutation.rs:24:31
1519
|
1620
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
1721
| - change this to accept `FnMut` instead of `Fn`
1822
...
23+
LL | let mut y = 0;
24+
| ----- `y` declared here, outside the closure
1925
LL | let _g = to_fn(|| set(&mut y));
2026
| ----- -- ^^^^^^ cannot borrow as mutable
2127
| | |
2228
| | in this closure
2329
| expects `Fn` instead of `FnMut`
30+
|
31+
= help: consider using `std::sync::atomic::AtomicUsize` instead, which allows for multiple threads to access and modify the value
2432

2533
error[E0594]: cannot assign to `z`, as it is a captured variable in a `Fn` closure
2634
--> $DIR/borrow-immutable-upvar-mutation.rs:29:22
2735
|
2836
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
2937
| - change this to accept `FnMut` instead of `Fn`
3038
...
39+
LL | let mut z = 0;
40+
| ----- `z` declared here, outside the closure
41+
...
3142
LL | to_fn(|| z = 42);
3243
| ----- -- ^^^^^^ cannot assign
3344
| | |
3445
| | in this closure
3546
| expects `Fn` instead of `FnMut`
47+
|
48+
= help: consider using `std::sync::atomic::AtomicUsize` instead, which allows for multiple threads to access and modify the value
3649

3750
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
3851
--> $DIR/borrow-immutable-upvar-mutation.rs:36:32

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,16 @@ 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
| |
4648
| expects `Fn` instead of `FnMut`
4749
LL | let y = &raw mut x;
4850
| ^^^^^^^^^^ cannot borrow as mutable
51+
|
52+
= help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
4953

5054
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
5155
--> $DIR/borrow-raw-address-of-mutability.rs:35:17

‎tests/ui/borrowck/mutability-errors.stderr‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,55 +139,71 @@ 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
| |
146148
| expects `Fn` instead of `FnMut`
147149
LL | x = (1,);
148150
| ^^^^^^^^ cannot assign
151+
|
152+
= help: consider using `std::sync::atomic::Mutex<(i32,)>` instead, which allows for multiple threads to access and modify the value
149153

150154
error[E0594]: cannot assign to `x.0`, as `Fn` closures cannot mutate their captured variables
151155
--> $DIR/mutability-errors.rs:41:9
152156
|
153157
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
154158
| - change this to accept `FnMut` instead of `Fn`
155-
...
159+
LL |
160+
LL | fn ref_closure(mut x: (i32,)) {
161+
| ----- `x` declared here, outside the closure
156162
LL | fn_ref(|| {
157163
| ------ -- in this closure
158164
| |
159165
| expects `Fn` instead of `FnMut`
160166
LL | x = (1,);
161167
LL | x.0 = 1;
162168
| ^^^^^^^ cannot assign
169+
|
170+
= help: consider using `std::sync::atomic::Mutex<(i32,)>` instead, which allows for multiple threads to access and modify the value
163171

164172
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
165173
--> $DIR/mutability-errors.rs:42:9
166174
|
167175
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
168176
| - change this to accept `FnMut` instead of `Fn`
169-
...
177+
LL |
178+
LL | fn ref_closure(mut x: (i32,)) {
179+
| ----- `x` declared here, outside the closure
170180
LL | fn_ref(|| {
171181
| ------ -- in this closure
172182
| |
173183
| expects `Fn` instead of `FnMut`
174184
...
175185
LL | &mut x;
176186
| ^^^^^^ cannot borrow as mutable
187+
|
188+
= help: consider using `std::sync::atomic::Mutex<(i32,)>` instead, which allows for multiple threads to access and modify the value
177189

178190
error[E0596]: cannot borrow `x.0` as mutable, as `Fn` closures cannot mutate their captured variables
179191
--> $DIR/mutability-errors.rs:43:9
180192
|
181193
LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
182194
| - change this to accept `FnMut` instead of `Fn`
183-
...
195+
LL |
196+
LL | fn ref_closure(mut x: (i32,)) {
197+
| ----- `x` declared here, outside the closure
184198
LL | fn_ref(|| {
185199
| ------ -- in this closure
186200
| |
187201
| expects `Fn` instead of `FnMut`
188202
...
189203
LL | &mut x.0;
190204
| ^^^^^^^^ cannot borrow as mutable
205+
|
206+
= help: consider using `std::sync::atomic::Mutex<(i32,)>` instead, which allows for multiple threads to access and modify the value
191207

192208
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
193209
--> $DIR/mutability-errors.rs:46:9

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,33 @@ 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
| | | |
1012
| | | cannot assign
1113
| | in this closure
1214
| expects `Fn` instead of `FnMut`
15+
|
16+
= help: consider using `std::sync::atomic::Mutex<()>` instead, which allows for multiple threads to access and modify the value
1317

1418
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
1519
--> $DIR/wrong-closure-arg-suggestion-125325.rs:25:13
1620
|
1721
LL | fn func(_f: impl Fn()) -> usize {
1822
| --------- change this to accept `FnMut` instead of `Fn`
1923
...
24+
LL | let mut x = ();
25+
| ----- `x` declared here, outside the closure
26+
...
2027
LL | func(|| x = ())
2128
| ---- -- ^^^^^^ cannot assign
2229
| | |
2330
| | in this closure
2431
| expects `Fn` instead of `FnMut`
32+
|
33+
= help: consider using `std::sync::atomic::Mutex<()>` instead, which allows for multiple threads to access and modify the value
2534

2635
error: aborting due to 2 previous errors
2736

‎tests/ui/issues/issue-21600.stderr‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,25 @@ 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
| | |
1013
| | in this closure
1114
| expects `Fn` instead of `FnMut`
15+
|
16+
= help: consider using `std::sync::atomic::Mutex<A>` instead, which allows for multiple threads to access and modify the value
1217

1318
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
1419
--> $DIR/issue-21600.rs:14:17
1520
|
1621
LL | fn call_it<F>(f: F) where F: Fn() { f(); }
1722
| - change this to accept `FnMut` instead of `Fn`
1823
...
24+
LL | let mut x = A;
25+
| ----- `x` declared here, outside the closure
1926
LL | call_it(|| {
2027
| ------- -- in this closure
2128
| |
@@ -25,6 +32,8 @@ LL | call_it(|| x.gen_mut());
2532
| ^^ - mutable borrow occurs due to use of `x` in closure
2633
| |
2734
| cannot borrow as mutable
35+
|
36+
= help: consider using `std::sync::atomic::Mutex<A>` instead, which allows for multiple threads to access and modify the value
2837

2938
error: aborting due to 2 previous errors
3039

0 commit comments

Comments
 (0)