Skip to content

Commit e47ea38

Browse files
gurrygurry
committed
Make suggestion to change Fn to FnMut work with methods well
1 parent d2fb97f commit e47ea38

File tree

3 files changed

+126
-44
lines changed

3 files changed

+126
-44
lines changed

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

+69-44
Original file line numberDiff line numberDiff line change
@@ -937,56 +937,81 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
937937
let node = self.infcx.tcx.hir_node(fn_call_id);
938938
let def_id = hir.enclosing_body_owner(fn_call_id);
939939
let mut look_at_return = true;
940-
// If we can detect the expression to be an `fn` call where the closure was an argument,
941-
// we point at the `fn` definition argument...
942-
if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Call(func, args), .. }) = node {
943-
let arg_pos = args
940+
941+
// If the HIR node is a function or method call gets the def ID
942+
// of the called function or method and the span and args of the call expr
943+
let get_call_details = || {
944+
let hir::Node::Expr(hir::Expr { hir_id, kind, .. }) = node else {
945+
return None;
946+
};
947+
948+
let typeck_results = self.infcx.tcx.typeck(def_id);
949+
950+
match kind {
951+
hir::ExprKind::Call(expr, args) => {
952+
if let Some(ty::FnDef(def_id, _)) =
953+
typeck_results.node_type_opt(expr.hir_id).as_ref().map(|ty| ty.kind())
954+
{
955+
Some((*def_id, expr.span, *args))
956+
} else {
957+
None
958+
}
959+
}
960+
hir::ExprKind::MethodCall(_, _, args, span) => {
961+
if let Some(def_id) = typeck_results.type_dependent_def_id(*hir_id) {
962+
Some((def_id, *span, *args))
963+
} else {
964+
None
965+
}
966+
}
967+
_ => None,
968+
}
969+
};
970+
971+
// If we can detect the expression to be an function or method call where the closure was an argument,
972+
// we point at the function or method definition argument...
973+
if let Some((callee_def_id, call_span, call_args)) = get_call_details() {
974+
let arg_pos = call_args
944975
.iter()
945976
.enumerate()
946977
.filter(|(_, arg)| arg.hir_id == closure_id)
947978
.map(|(pos, _)| pos)
948979
.next();
949-
let tables = self.infcx.tcx.typeck(def_id);
950-
if let Some(ty::FnDef(def_id, _)) =
951-
tables.node_type_opt(func.hir_id).as_ref().map(|ty| ty.kind())
952-
{
953-
let arg = match hir.get_if_local(*def_id) {
954-
Some(
955-
hir::Node::Item(hir::Item {
956-
ident, kind: hir::ItemKind::Fn(sig, ..), ..
957-
})
958-
| hir::Node::TraitItem(hir::TraitItem {
959-
ident,
960-
kind: hir::TraitItemKind::Fn(sig, _),
961-
..
980+
981+
let arg = match hir.get_if_local(callee_def_id) {
982+
Some(
983+
hir::Node::Item(hir::Item { ident, kind: hir::ItemKind::Fn(sig, ..), .. })
984+
| hir::Node::TraitItem(hir::TraitItem {
985+
ident,
986+
kind: hir::TraitItemKind::Fn(sig, _),
987+
..
988+
})
989+
| hir::Node::ImplItem(hir::ImplItem {
990+
ident,
991+
kind: hir::ImplItemKind::Fn(sig, _),
992+
..
993+
}),
994+
) => Some(
995+
arg_pos
996+
.and_then(|pos| {
997+
sig.decl.inputs.get(
998+
pos + if sig.decl.implicit_self.has_implicit_self() {
999+
1
1000+
} else {
1001+
0
1002+
},
1003+
)
9621004
})
963-
| hir::Node::ImplItem(hir::ImplItem {
964-
ident,
965-
kind: hir::ImplItemKind::Fn(sig, _),
966-
..
967-
}),
968-
) => Some(
969-
arg_pos
970-
.and_then(|pos| {
971-
sig.decl.inputs.get(
972-
pos + if sig.decl.implicit_self.has_implicit_self() {
973-
1
974-
} else {
975-
0
976-
},
977-
)
978-
})
979-
.map(|arg| arg.span)
980-
.unwrap_or(ident.span),
981-
),
982-
_ => None,
983-
};
984-
if let Some(span) = arg {
985-
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
986-
err.span_label(func.span, "expects `Fn` instead of `FnMut`");
987-
err.span_label(closure_span, "in this closure");
988-
look_at_return = false;
989-
}
1005+
.map(|arg| arg.span)
1006+
.unwrap_or(ident.span),
1007+
),
1008+
_ => None,
1009+
};
1010+
if let Some(span) = arg {
1011+
err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
1012+
err.span_label(call_span, "expects `Fn` instead of `FnMut`");
1013+
err.span_label(closure_span, "in this closure");
1014+
look_at_return = false;
9901015
}
9911016
}
9921017

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

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Regression test for #125325
2+
3+
// Tests that we suggest changing an `impl Fn` param
4+
// to `impl FnMut` when the provided closure arg
5+
// is trying to mutate the closure env.
6+
// Ensures that it works that way for both
7+
// functions and methods
8+
9+
struct S;
10+
11+
impl S {
12+
fn assoc_func(&self, _f: impl Fn()) -> usize {
13+
0
14+
}
15+
}
16+
17+
fn func(_f: impl Fn()) -> usize {
18+
0
19+
}
20+
21+
fn test_func(s: &S) -> usize {
22+
let mut x = ();
23+
s.assoc_func(|| x = ());
24+
//~^ cannot assign to `x`, as it is a captured variable in a `Fn` closure
25+
func(|| x = ())
26+
//~^ cannot assign to `x`, as it is a captured variable in a `Fn` closure
27+
}
28+
29+
fn main() {}

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

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
2+
--> $DIR/wrong-closure-arg-suggestion-125325.rs:23:21
3+
|
4+
LL | fn assoc_func(&self, _f: impl Fn()) -> usize {
5+
| --------- change this to accept `FnMut` instead of `Fn`
6+
...
7+
LL | s.assoc_func(|| x = ());
8+
| --------------^^^^^^-
9+
| | | |
10+
| | | cannot assign
11+
| | in this closure
12+
| expects `Fn` instead of `FnMut`
13+
14+
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
15+
--> $DIR/wrong-closure-arg-suggestion-125325.rs:25:13
16+
|
17+
LL | fn func(_f: impl Fn()) -> usize {
18+
| --------- change this to accept `FnMut` instead of `Fn`
19+
...
20+
LL | func(|| x = ())
21+
| ---- -- ^^^^^^ cannot assign
22+
| | |
23+
| | in this closure
24+
| expects `Fn` instead of `FnMut`
25+
26+
error: aborting due to 2 previous errors
27+
28+
For more information about this error, try `rustc --explain E0594`.

0 commit comments

Comments
 (0)