Point at the Fn() or FnMut() bound that coerced a closure, which caused a move error

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -2,14 +2,14 @@
 #![allow(rustc::untranslatable_diagnostic)]
 
 use rustc_data_structures::fx::FxHashSet;
-use rustc_errors::{Applicability, Diag};
+use rustc_errors::{Applicability, Diag, MultiSpan};
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::{self as hir, CaptureBy, ExprKind, HirId, Node};
 use rustc_middle::bug;
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_mir_dataflow::move_paths::{LookupResult, MovePathIndex};
-use rustc_span::{BytePos, ExpnKind, MacroKind, Span};
+use rustc_span::{BytePos, DUMMY_SP, ExpnKind, MacroKind, Span};
 use rustc_trait_selection::error_reporting::traits::FindExprBySpan;
 use rustc_trait_selection::infer::InferCtxtExt;
 use tracing::debug;
@@ -508,12 +508,54 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 );
 
                 let closure_span = tcx.def_span(def_id);
+                let mut clause_span: MultiSpan = DUMMY_SP.into();
+                let typck_result = self.infcx.tcx.typeck(self.mir_def_id());
+                if let Some(closure_def_id) = def_id.as_local()
+                    && let hir::Node::Expr(expr) = tcx.hir_node_by_def_id(closure_def_id)
+                    && let hir::Node::Expr(parent) = tcx.parent_hir_node(expr.hir_id)
+                    && let hir::ExprKind::Call(callee, _) = parent.kind
+                    && let Some(ty) = typck_result.node_type_opt(callee.hir_id)
+                    && let ty::FnDef(fn_def_id, args) = ty.kind()
+                    && let predicates = tcx.predicates_of(fn_def_id).instantiate(tcx, args)
+                    && let Some((_, span)) =
+                        predicates.predicates.iter().zip(predicates.spans.iter()).find(
+                            |(pred, _)| match pred.as_trait_clause() {
+                                Some(clause)
+                                    if let ty::Closure(clause_closure_def_id, _) =
+                                        clause.self_ty().skip_binder().kind()
+                                        && clause_closure_def_id == def_id
+                                        && (tcx.lang_items().fn_mut_trait()
+                                            == Some(clause.def_id())
+                                            || tcx.lang_items().fn_trait()
+                                                == Some(clause.def_id())) =>
+                                {
+                                    // Found `<TyOfCapturingClosure as FnMut>`
+                                    true
+                                }
+                                _ => false,
+                            },
+                        )
+                {
+                    // We point at the `Fn()` or `FnMut()` bound that coerced the closure, which
+                    // could be changed to `FnOnce()` to avoid the move error.
+                    clause_span = (*span).into();
+                    if fn_def_id.is_local() {
+                        clause_span
+                            .push_span_label(*span, "consider changing this bound to be `FnOnce`");
+                    }
+                }
+
                 self.cannot_move_out_of(span, &place_description)
                     .with_span_label(upvar_span, "captured outer variable")
                     .with_span_label(
                         closure_span,
                         format!("captured by this `{closure_kind}` closure"),
                     )
+                    .with_span_help(
+                        clause_span,
+                        "`Fn` and `FnMut` closures require captured values to be able to be \
+                         consumed multiple times, but an `FnOnce` consume them only once",
+                    )
             }
             _ => {
                 let source = self.borrowed_content_source(deref_base);

compiler/rustc_errors/src/diagnostic.rs

@@ -847,17 +847,18 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
         self
     }
 
+    with_fn! { with_span_help,
     /// Prints the span with some help above it.
     /// This is like [`Diag::help()`], but it gets its own span.
     #[rustc_lint_diagnostics]
-    pub fn span_help<S: Into<MultiSpan>>(
+    pub fn span_help(
         &mut self,
-        sp: S,
+        sp: impl Into<MultiSpan>,
         msg: impl Into<SubdiagMessage>,
     ) -> &mut Self {
         self.sub(Level::Help, msg, sp.into());
         self
-    }
+    } }
 
     /// Disallow attaching suggestions to this diagnostic.
     /// Any suggestions attached e.g. with the `span_suggestion_*` methods

tests/ui/borrowck/borrowck-in-static.stderr

@@ -10,6 +10,7 @@ LL |     Box::new(|| x)
    |              |
    |              captured by this `Fn` closure
    |
+   = help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
 help: consider cloning the value if the performance cost is acceptable
    |
 LL |     Box::new(|| x.clone())

tests/ui/borrowck/borrowck-move-by-capture.stderr

@@ -12,6 +12,11 @@ LL |         let _h = to_fn_once(move || -> isize { *bar });
    |                             |
    |                             `bar` is moved here
    |
+help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
+  --> $DIR/borrowck-move-by-capture.rs:3:37
+   |
+LL | fn to_fn_mut<A:std::marker::Tuple,F:FnMut<A>>(f: F) -> F { f }
+   |                                     ^^^^^^^^ consider changing this bound to be `FnOnce`
 help: consider cloning the value before moving it into the closure
    |
 LL ~         let value = bar.clone();

tests/ui/borrowck/issue-103624.stderr

@@ -13,6 +13,11 @@ LL |
 LL |             self.b;
    |             ^^^^^^ `self.b` is moved here
    |
+help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
+  --> $DIR/issue-103624.rs:7:36
+   |
+LL | async fn spawn_blocking<T>(f: impl (Fn() -> T) + Send + Sync + 'static) -> T {
+   |                                    ^^^^^^^^^^^ consider changing this bound to be `FnOnce`
 note: if `StructB` implemented `Clone`, you could clone the value
   --> $DIR/issue-103624.rs:23:1
    |

tests/ui/borrowck/issue-87456-point-to-closure.rs

@@ -1,6 +1,6 @@
 // Regression test for #87456.
 
-fn take_mut(_val: impl FnMut()) {}
+fn take_mut(_val: impl FnMut()) {} //~ NOTE: consider changing this bound to be `FnOnce`
 
 fn main() {
     let val = String::new();

tests/ui/borrowck/issue-87456-point-to-closure.stderr

@@ -10,6 +10,11 @@ LL |
 LL |         let _foo: String = val;
    |                            ^^^ move occurs because `val` has type `String`, which does not implement the `Copy` trait
    |
+help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
+  --> $DIR/issue-87456-point-to-closure.rs:3:24
+   |
+LL | fn take_mut(_val: impl FnMut()) {}
+   |                        ^^^^^^^ consider changing this bound to be `FnOnce`
 help: consider borrowing here
    |
 LL |         let _foo: String = &val;

tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr

@@ -10,6 +10,11 @@ LL |         y.into_iter();
    |         |
    |         move occurs because `y` has type `Vec<String>`, which does not implement the `Copy` trait
    |
+help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
+  --> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:5:28
+   |
+LL | fn call<F>(f: F) where F : Fn() {
+   |                            ^^^^ consider changing this bound to be `FnOnce`
 note: `into_iter` takes ownership of the receiver `self`, which moves `y`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
 help: you can `clone` the value and consume it, but this might not be your desired behavior

tests/ui/issues/issue-4335.stderr

@@ -10,6 +10,7 @@ LL |     id(Box::new(|| *v))
    |                 |
    |                 captured by this `FnMut` closure
    |
+   = help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
 help: if `T` implemented `Clone`, you could clone the value
   --> $DIR/issue-4335.rs:5:10
    |

tests/ui/moves/moves-based-on-type-move-out-of-closure-env-issue-1965.stderr

@@ -10,6 +10,11 @@ LL |     let _f = to_fn(|| test(i));
    |                    |
    |                    captured by this `Fn` closure
    |
+help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
+  --> $DIR/moves-based-on-type-move-out-of-closure-env-issue-1965.rs:3:33
+   |
+LL | fn to_fn<A:std::marker::Tuple,F:Fn<A>>(f: F) -> F { f }
+   |                                 ^^^^^ consider changing this bound to be `FnOnce`
 help: consider cloning the value if the performance cost is acceptable
    |
 LL |     let _f = to_fn(|| test(i.clone()));

tests/ui/nll/issue-52663-span-decl-captured-variable.stderr

@@ -10,6 +10,11 @@ LL |        expect_fn(|| drop(x.0));
    |                  |
    |                  captured by this `Fn` closure
    |
+help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
+  --> $DIR/issue-52663-span-decl-captured-variable.rs:1:33
+   |
+LL | fn expect_fn<F>(f: F) where F : Fn() {
+   |                                 ^^^^ consider changing this bound to be `FnOnce`
 help: consider cloning the value if the performance cost is acceptable
    |
 LL |        expect_fn(|| drop(x.0.clone()));

tests/ui/span/borrowck-call-is-borrow-issue-12224.stderr

@@ -44,6 +44,7 @@ LL |
 LL |         foo(f);
    |             ^ move occurs because `f` has type `{closure@$DIR/borrowck-call-is-borrow-issue-12224.rs:52:17: 52:58}`, which does not implement the `Copy` trait
    |
+   = help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
 help: consider cloning the value if the performance cost is acceptable
    |
 LL |         foo(f.clone());

tests/ui/suggestions/dont-suggest-ref/move-into-closure.rs

@@ -11,8 +11,29 @@ struct X(Y);
 struct Y;
 
 fn consume_fn<F: Fn()>(_f: F) { }
+//~^ HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
 
 fn consume_fnmut<F: FnMut()>(_f: F) { }
+//~^ HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
+//~| HELP `Fn` and `FnMut` closures
 
 pub fn main() { }