Skip to content

Commit 109d5c1

Browse files
estebankestebank
committed
Tweak borrow error on FnMut when Fn is expected
1 parent 994e5e7 commit 109d5c1

11 files changed

+370
-238
lines changed

Diff for: ‎src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs

+96-6
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_span::Span;
1010
use crate::borrow_check::diagnostics::BorrowedContentSource;
1111
use crate::borrow_check::MirBorrowckCtxt;
1212
use crate::util::collect_writes::FindAssignments;
13-
use rustc_errors::Applicability;
13+
use rustc_errors::{Applicability, DiagnosticBuilder};
1414

1515
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
1616
pub(crate) enum AccessKind {
@@ -412,11 +412,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
412412
projection: [ProjectionElem::Deref],
413413
// FIXME document what is this 1 magic number about
414414
} if local == Local::new(1) && !self.upvars.is_empty() => {
415-
err.span_label(span, format!("cannot {ACT}", ACT = act));
416-
err.span_help(
417-
self.body.span,
418-
"consider changing this to accept closures that implement `FnMut`",
419-
);
415+
self.expected_fn_found_fn_mut_call(&mut err, span, act);
420416
}
421417

422418
PlaceRef { local: _, projection: [.., ProjectionElem::Deref] } => {
@@ -448,6 +444,100 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
448444

449445
err.buffer(&mut self.errors_buffer);
450446
}
447+
448+
/// Targetted error when encountering an `FnMut` closure where an `Fn` closure was expected.
449+
fn expected_fn_found_fn_mut_call(&self, err: &mut DiagnosticBuilder<'_>, sp: Span, act: &str) {
450+
err.span_label(sp, format!("cannot {ACT}", ACT = act));
451+
452+
let hir = self.infcx.tcx.hir();
453+
let closure_id = hir.as_local_hir_id(self.mir_def_id).unwrap();
454+
let fn_call_id = hir.get_parent_node(closure_id);
455+
let node = hir.get(fn_call_id);
456+
let item_id = hir.get_parent_item(fn_call_id);
457+
let mut look_at_return = true;
458+
// If we can detect the expression to be an `fn` call where the closure was an argument,
459+
// we point at the `fn` definition argument...
460+
match node {
461+
hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Call(func, args), .. }) => {
462+
let arg_pos = args
463+
.iter()
464+
.enumerate()
465+
.filter(|(_, arg)| arg.span == self.body.span)
466+
.map(|(pos, _)| pos)
467+
.next();
468+
let def_id = hir.local_def_id(item_id);
469+
let tables = self.infcx.tcx.typeck_tables_of(def_id);
470+
if let Some(ty::FnDef(def_id, _)) =
471+
tables.node_type_opt(func.hir_id).as_ref().map(|ty| &ty.kind)
472+
{
473+
let arg = match hir.get_if_local(*def_id) {
474+
Some(hir::Node::Item(hir::Item {
475+
ident,
476+
kind: hir::ItemKind::Fn(sig, ..),
477+
..
478+
}))
479+
| Some(hir::Node::TraitItem(hir::TraitItem {
480+
ident,
481+
kind: hir::TraitItemKind::Method(sig, _),
482+
..
483+
}))
484+
| Some(hir::Node::ImplItem(hir::ImplItem {
485+
ident,
486+
kind: hir::ImplItemKind::Method(sig, _),
487+
..
488+
})) => Some(
489+
arg_pos
490+
.and_then(|pos| {
491+
sig.decl.inputs.get(
492+
pos + if sig.decl.implicit_self.has_implicit_self() {
493+
1
494+
} else {
495+
0
496+
},
497+
)
498+
})
499+
.map(|arg| arg.span)
500+
.unwrap_or(ident.span),
501+
),
502+
_ => None,
503+
};
504+
if let Some(span) = arg {
505+
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
506+
err.span_label(func.span, "expects `Fn` instead of `FnMut`");
507+
if self.infcx.tcx.sess.source_map().is_multiline(self.body.span) {
508+
err.span_label(self.body.span, "in this closure");
509+
}
510+
look_at_return = false;
511+
}
512+
}
513+
}
514+
_ => {}
515+
}
516+
if look_at_return && hir.get_return_block(closure_id).is_some() {
517+
// ...otherwise we are probably in the tail expression of the function, point at the
518+
// return type.
519+
match hir.get(hir.get_parent_item(fn_call_id)) {
520+
hir::Node::Item(hir::Item { ident, kind: hir::ItemKind::Fn(sig, ..), .. })
521+
| hir::Node::TraitItem(hir::TraitItem {
522+
ident,
523+
kind: hir::TraitItemKind::Method(sig, _),
524+
..
525+
})
526+
| hir::Node::ImplItem(hir::ImplItem {
527+
ident,
528+
kind: hir::ImplItemKind::Method(sig, _),
529+
..
530+
}) => {
531+
err.span_label(ident.span, "you might have to change this...");
532+
err.span_label(sig.decl.output.span(), "...to return `FnMut` instead of `Fn`");
533+
err.span_label(self.body.span, "in this closure");
534+
}
535+
parent => {
536+
err.note(&format!("parent {:?}", parent));
537+
}
538+
}
539+
}
540+
}
451541
}
452542

453543
fn suggest_ampmut_self<'tcx>(

Diff for: ‎src/test/ui/borrowck/borrow-immutable-upvar-mutation.rs

+20-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ fn main() {
1818
let _g = to_fn(|| set(&mut y)); //~ ERROR cannot borrow
1919

2020
let mut z = 0;
21-
let _h = to_fn_mut(|| { set(&mut z); to_fn(|| z = 42); }); //~ ERROR cannot assign
21+
let _h = to_fn_mut(|| {
22+
set(&mut z);
23+
to_fn(|| z = 42); //~ ERROR cannot assign
24+
});
2225
}
2326

2427
// By-value captures
@@ -33,3 +36,19 @@ fn main() {
3336
let _h = to_fn_mut(move || { set(&mut z); to_fn(move || z = 42); }); //~ ERROR cannot assign
3437
}
3538
}
39+
40+
fn foo() -> Box<dyn Fn() -> usize> {
41+
let mut x = 0;
42+
Box::new(move || {
43+
x += 1; //~ ERROR cannot assign
44+
x
45+
})
46+
}
47+
48+
fn bar() -> impl Fn() -> usize {
49+
let mut x = 0;
50+
move || {
51+
x += 1; //~ ERROR cannot assign
52+
x
53+
}
54+
}

Diff for: ‎src/test/ui/borrowck/borrow-immutable-upvar-mutation.stderr

+74-49
Original file line numberDiff line numberDiff line change
@@ -1,76 +1,101 @@
11
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
22
--> $DIR/borrow-immutable-upvar-mutation.rs:15:27
33
|
4+
LL | fn to_fn<A,F:Fn<A>>(f: F) -> F { f }
5+
| - change this to accept `FnMut` instead of `Fn`
6+
...
47
LL | let _f = to_fn(|| x = 42);
5-
| ^^^^^^ cannot assign
6-
|
7-
help: consider changing this to accept closures that implement `FnMut`
8-
--> $DIR/borrow-immutable-upvar-mutation.rs:15:24
9-
|
10-
LL | let _f = to_fn(|| x = 42);
11-
| ^^^^^^^^^
8+
| ----- ^^^^^^ cannot assign
9+
| |
10+
| expects `Fn` instead of `FnMut`
1211

1312
error[E0596]: cannot borrow `y` as mutable, as it is a captured variable in a `Fn` closure
1413
--> $DIR/borrow-immutable-upvar-mutation.rs:18:31
1514
|
15+
LL | fn to_fn<A,F:Fn<A>>(f: F) -> F { f }
16+
| - change this to accept `FnMut` instead of `Fn`
17+
...
1618
LL | let _g = to_fn(|| set(&mut y));
17-
| ^^^^^^ cannot borrow as mutable
18-
|
19-
help: consider changing this to accept closures that implement `FnMut`
20-
--> $DIR/borrow-immutable-upvar-mutation.rs:18:24
21-
|
22-
LL | let _g = to_fn(|| set(&mut y));
23-
| ^^^^^^^^^^^^^^
19+
| ----- ^^^^^^ cannot borrow as mutable
20+
| |
21+
| expects `Fn` instead of `FnMut`
2422

2523
error[E0594]: cannot assign to `z`, as it is a captured variable in a `Fn` closure
26-
--> $DIR/borrow-immutable-upvar-mutation.rs:21:55
27-
|
28-
LL | let _h = to_fn_mut(|| { set(&mut z); to_fn(|| z = 42); });
29-
| ^^^^^^ cannot assign
30-
|
31-
help: consider changing this to accept closures that implement `FnMut`
32-
--> $DIR/borrow-immutable-upvar-mutation.rs:21:52
33-
|
34-
LL | let _h = to_fn_mut(|| { set(&mut z); to_fn(|| z = 42); });
35-
| ^^^^^^^^^
24+
--> $DIR/borrow-immutable-upvar-mutation.rs:23:22
25+
|
26+
LL | fn to_fn<A,F:Fn<A>>(f: F) -> F { f }
27+
| - change this to accept `FnMut` instead of `Fn`
28+
...
29+
LL | to_fn(|| z = 42);
30+
| ----- ^^^^^^ cannot assign
31+
| |
32+
| expects `Fn` instead of `FnMut`
3633

3734
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
38-
--> $DIR/borrow-immutable-upvar-mutation.rs:27:32
39-
|
40-
LL | let _f = to_fn(move || x = 42);
41-
| ^^^^^^ cannot assign
42-
|
43-
help: consider changing this to accept closures that implement `FnMut`
44-
--> $DIR/borrow-immutable-upvar-mutation.rs:27:24
35+
--> $DIR/borrow-immutable-upvar-mutation.rs:30:32
4536
|
37+
LL | fn to_fn<A,F:Fn<A>>(f: F) -> F { f }
38+
| - change this to accept `FnMut` instead of `Fn`
39+
...
4640
LL | let _f = to_fn(move || x = 42);
47-
| ^^^^^^^^^^^^^^
41+
| ----- ^^^^^^ cannot assign
42+
| |
43+
| expects `Fn` instead of `FnMut`
4844

4945
error[E0596]: cannot borrow `y` as mutable, as it is a captured variable in a `Fn` closure
50-
--> $DIR/borrow-immutable-upvar-mutation.rs:30:36
51-
|
52-
LL | let _g = to_fn(move || set(&mut y));
53-
| ^^^^^^ cannot borrow as mutable
54-
|
55-
help: consider changing this to accept closures that implement `FnMut`
56-
--> $DIR/borrow-immutable-upvar-mutation.rs:30:24
46+
--> $DIR/borrow-immutable-upvar-mutation.rs:33:36
5747
|
48+
LL | fn to_fn<A,F:Fn<A>>(f: F) -> F { f }
49+
| - change this to accept `FnMut` instead of `Fn`
50+
...
5851
LL | let _g = to_fn(move || set(&mut y));
59-
| ^^^^^^^^^^^^^^^^^^^
52+
| ----- ^^^^^^ cannot borrow as mutable
53+
| |
54+
| expects `Fn` instead of `FnMut`
6055

6156
error[E0594]: cannot assign to `z`, as it is a captured variable in a `Fn` closure
62-
--> $DIR/borrow-immutable-upvar-mutation.rs:33:65
57+
--> $DIR/borrow-immutable-upvar-mutation.rs:36:65
6358
|
59+
LL | fn to_fn<A,F:Fn<A>>(f: F) -> F { f }
60+
| - change this to accept `FnMut` instead of `Fn`
61+
...
6462
LL | let _h = to_fn_mut(move || { set(&mut z); to_fn(move || z = 42); });
65-
| ^^^^^^ cannot assign
66-
|
67-
help: consider changing this to accept closures that implement `FnMut`
68-
--> $DIR/borrow-immutable-upvar-mutation.rs:33:57
69-
|
70-
LL | let _h = to_fn_mut(move || { set(&mut z); to_fn(move || z = 42); });
71-
| ^^^^^^^^^^^^^^
63+
| ----- ^^^^^^ cannot assign
64+
| |
65+
| expects `Fn` instead of `FnMut`
66+
67+
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
68+
--> $DIR/borrow-immutable-upvar-mutation.rs:43:9
69+
|
70+
LL | fn foo() -> Box<dyn Fn() -> usize> {
71+
| --- ---------------------- ...to return `FnMut` instead of `Fn`
72+
| |
73+
| you might have to change this...
74+
LL | let mut x = 0;
75+
LL | Box::new(move || {
76+
| ______________-
77+
LL | | x += 1;
78+
| | ^^^^^^ cannot assign
79+
LL | | x
80+
LL | | })
81+
| |_____- in this closure
82+
83+
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
84+
--> $DIR/borrow-immutable-upvar-mutation.rs:51:9
85+
|
86+
LL | fn bar() -> impl Fn() -> usize {
87+
| --- ------------------ ...to return `FnMut` instead of `Fn`
88+
| |
89+
| you might have to change this...
90+
LL | let mut x = 0;
91+
LL | / move || {
92+
LL | | x += 1;
93+
| | ^^^^^^ cannot assign
94+
LL | | x
95+
LL | | }
96+
| |_____- in this closure
7297

73-
error: aborting due to 6 previous errors
98+
error: aborting due to 8 previous errors
7499

75100
Some errors have detailed explanations: E0594, E0596.
76101
For more information about an error, try `rustc --explain E0594`.

Diff for: ‎src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr

+16-16
Original file line numberDiff line numberDiff line change
@@ -27,32 +27,32 @@ LL | f();
2727
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
2828
--> $DIR/borrow-raw-address-of-mutability.rs:29:17
2929
|
30-
LL | let y = &raw mut x;
31-
| ^^^^^^^^^^ cannot borrow as mutable
32-
|
33-
help: consider changing this to accept closures that implement `FnMut`
34-
--> $DIR/borrow-raw-address-of-mutability.rs:28:21
35-
|
30+
LL | fn make_fn<F: Fn()>(f: F) -> F { f }
31+
| - change this to accept `FnMut` instead of `Fn`
32+
...
3633
LL | let f = make_fn(|| {
37-
| _____________________^
34+
| _____________-------_-
35+
| | |
36+
| | expects `Fn` instead of `FnMut`
3837
LL | | let y = &raw mut x;
38+
| | ^^^^^^^^^^ cannot borrow as mutable
3939
LL | | });
40-
| |_____^
40+
| |_____- in this closure
4141

4242
error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
4343
--> $DIR/borrow-raw-address-of-mutability.rs:37:17
4444
|
45-
LL | let y = &raw mut x;
46-
| ^^^^^^^^^^ cannot borrow as mutable
47-
|
48-
help: consider changing this to accept closures that implement `FnMut`
49-
--> $DIR/borrow-raw-address-of-mutability.rs:36:21
50-
|
45+
LL | fn make_fn<F: Fn()>(f: F) -> F { f }
46+
| - change this to accept `FnMut` instead of `Fn`
47+
...
5148
LL | let f = make_fn(move || {
52-
| _____________________^
49+
| _____________-------_-
50+
| | |
51+
| | expects `Fn` instead of `FnMut`
5352
LL | | let y = &raw mut x;
53+
| | ^^^^^^^^^^ cannot borrow as mutable
5454
LL | | });
55-
| |_____^
55+
| |_____- in this closure
5656

5757
error: aborting due to 5 previous errors
5858

0 commit comments

Comments
 (0)