Skip to content

Commit 758869a

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 f5703d5 commit 758869a

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
@@ -6,6 +6,7 @@ use core::ops::ControlFlow;
66
use either::Either;
77
use hir::{ExprKind, Param};
88
use rustc_abi::FieldIdx;
9+
use rustc_data_structures::fx::FxHashMap;
910
use rustc_errors::{Applicability, Diag};
1011
use rustc_hir::intravisit::Visitor;
1112
use rustc_hir::{self as hir, BindingMode, ByRef, Node};
@@ -150,8 +151,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
150151
}
151152
}
152153
}
153-
PlaceRef { local: _, projection: [proj_base @ .., ProjectionElem::Deref] } => {
154-
if the_place_err.local == ty::CAPTURE_STRUCT_LOCAL
154+
PlaceRef { local, projection: [proj_base @ .., ProjectionElem::Deref] } => {
155+
if local == ty::CAPTURE_STRUCT_LOCAL
155156
&& proj_base.is_empty()
156157
&& !self.upvars.is_empty()
157158
{
@@ -165,10 +166,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
165166
", as `Fn` closures cannot mutate their captured variables".to_string()
166167
}
167168
} else {
168-
let source = self.borrowed_content_source(PlaceRef {
169-
local: the_place_err.local,
170-
projection: proj_base,
171-
});
169+
let source =
170+
self.borrowed_content_source(PlaceRef { local, projection: proj_base });
172171
let pointer_type = source.describe_for_immutable_place(self.infcx.tcx);
173172
opt_source = Some(source);
174173
if let Some(desc) = self.describe_place(access_place.as_ref()) {
@@ -534,6 +533,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
534533
PlaceRef { local, projection: [ProjectionElem::Deref] }
535534
if local == ty::CAPTURE_STRUCT_LOCAL && !self.upvars.is_empty() =>
536535
{
536+
self.point_at_binding_outside_closure(&mut err, local, access_place);
537537
self.expected_fn_found_fn_mut_call(&mut err, span, act);
538538
}
539539

@@ -952,6 +952,76 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
952952
}
953953
}
954954

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

1037+
err.span_label(closure_span, "in this closure");
9671038
// If the HIR node is a function or method call, get the DefId
9681039
// of the callee function or method, the span, and args of the call expr
9691040
let get_call_details = || {
@@ -1034,7 +1105,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10341105
if let Some(span) = arg {
10351106
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
10361107
err.span_label(call_span, "expects `Fn` instead of `FnMut`");
1037-
err.span_label(closure_span, "in this closure");
10381108
look_at_return = false;
10391109
}
10401110
}
@@ -1061,7 +1131,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10611131
sig.decl.output.span(),
10621132
"change this to return `FnMut` instead of `Fn`",
10631133
);
1064-
err.span_label(closure_span, "in this closure");
10651134
}
10661135
_ => {}
10671136
}

‎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 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
| | |
@@ -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/aliasability-violation-with-closure-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/aliasability-violation-with-closure-21600.rs:15: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

‎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

0 commit comments

Comments
 (0)