Auto merge of #94515 - estebank:tweak-move-error, r=davidtwco

Tweak move error

Point at method definition that causes type to be consumed.

Fix #94056.

3 files changed, 195 insertions, 183 deletions

diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index 2a74e1ce8b1..1cbca58ca25 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -1,5 +1,5 @@
 use either::Either;
-use rustc_const_eval::util::{CallDesugaringKind, CallKind};
+use rustc_const_eval::util::CallKind;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
 use rustc_hir as hir;
@@ -17,7 +17,7 @@ use rustc_middle::ty::{
 };
 use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex};
 use rustc_span::symbol::sym;
-use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP};
+use rustc_span::{BytePos, MultiSpan, Span};
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::TraitEngineExt as _;
 
@@ -195,144 +195,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     is_loop_move = true;
                 }
 
-                if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } = move_spans {
-                    let place_name = self
-                        .describe_place(moved_place.as_ref())
-                        .map(|n| format!("`{}`", n))
-                        .unwrap_or_else(|| "value".to_owned());
-                    match kind {
-                        CallKind::FnCall { fn_trait_id, .. }
-                            if Some(fn_trait_id) == self.infcx.tcx.lang_items().fn_once_trait() =>
-                        {
-                            err.span_label(
-                                fn_call_span,
-                                &format!(
-                                    "{} {}moved due to this call{}",
-                                    place_name, partially_str, loop_message
-                                ),
-                            );
-                            err.span_note(
-                                var_span,
-                                "this value implements `FnOnce`, which causes it to be moved when called",
-                            );
-                        }
-                        CallKind::Operator { self_arg, .. } => {
-                            let self_arg = self_arg.unwrap();
-                            err.span_label(
-                                fn_call_span,
-                                &format!(
-                                    "{} {}moved due to usage in operator{}",
-                                    place_name, partially_str, loop_message
-                                ),
-                            );
-                            if self.fn_self_span_reported.insert(fn_span) {
-                                err.span_note(
-                                    // Check whether the source is accessible
-                                    if self
-                                        .infcx
-                                        .tcx
-                                        .sess
-                                        .source_map()
-                                        .span_to_snippet(self_arg.span)
-                                        .is_ok()
-                                    {
-                                        self_arg.span
-                                    } else {
-                                        fn_call_span
-                                    },
-                                    "calling this operator moves the left-hand side",
-                                );
-                            }
-                        }
-                        CallKind::Normal { self_arg, desugaring, is_option_or_result } => {
-                            let self_arg = self_arg.unwrap();
-                            if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
-                                err.span_label(
-                                    fn_call_span,
-                                    &format!(
-                                        "{} {}moved due to this implicit call to `.into_iter()`{}",
-                                        place_name, partially_str, loop_message
-                                    ),
-                                );
-                                let sess = self.infcx.tcx.sess;
-                                let ty = used_place.ty(self.body, self.infcx.tcx).ty;
-                                // If we have a `&mut` ref, we need to reborrow.
-                                if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() {
-                                    // If we are in a loop this will be suggested later.
-                                    if !is_loop_move {
-                                        err.span_suggestion_verbose(
-                                            move_span.shrink_to_lo(),
-                                            &format!(
-                                                "consider creating a fresh reborrow of {} here",
-                                                self.describe_place(moved_place.as_ref())
-                                                    .map(|n| format!("`{}`", n))
-                                                    .unwrap_or_else(
-                                                        || "the mutable reference".to_string()
-                                                    ),
-                                            ),
-                                            "&mut *".to_string(),
-                                            Applicability::MachineApplicable,
-                                        );
-                                    }
-                                } else if let Ok(snippet) =
-                                    sess.source_map().span_to_snippet(move_span)
-                                {
-                                    err.span_suggestion(
-                                        move_span,
-                                        "consider borrowing to avoid moving into the for loop",
-                                        format!("&{}", snippet),
-                                        Applicability::MaybeIncorrect,
-                                    );
-                                }
-                            } else {
-                                err.span_label(
-                                    fn_call_span,
-                                    &format!(
-                                        "{} {}moved due to this method call{}",
-                                        place_name, partially_str, loop_message
-                                    ),
-                                );
-                            }
-                            if is_option_or_result && maybe_reinitialized_locations.is_empty() {
-                                err.span_suggestion_verbose(
-                                    fn_call_span.shrink_to_lo(),
-                                    "consider calling `.as_ref()` to borrow the type's contents",
-                                    "as_ref().".to_string(),
-                                    Applicability::MachineApplicable,
-                                );
-                            }
-                            // Avoid pointing to the same function in multiple different
-                            // error messages.
-                            if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span)
-                            {
-                                err.span_note(
-                                        self_arg.span,
-                                        &format!("this function takes ownership of the receiver `self`, which moves {}", place_name)
-                                    );
-                            }
-                        }
-                        // Other desugarings takes &self, which cannot cause a move
-                        _ => unreachable!(),
-                    }
-                } else {
-                    err.span_label(
-                        move_span,
-                        format!("value {}moved{} here{}", partially_str, move_msg, loop_message),
-                    );
-                    // If the move error occurs due to a loop, don't show
-                    // another message for the same span
-                    if loop_message.is_empty() {
-                        move_spans.var_span_label(
-                            &mut err,
-                            format!(
-                                "variable {}moved due to use{}",
-                                partially_str,
-                                move_spans.describe()
-                            ),
-                            "moved",
-                        );
-                    }
-                }
+                self.explain_captures(
+                    &mut err,
+                    span,
+                    move_span,
+                    move_spans,
+                    *moved_place,
+                    Some(used_place),
+                    partially_str,
+                    loop_message,
+                    move_msg,
+                    is_loop_move,
+                    maybe_reinitialized_locations.is_empty(),
+                );
 
                 if let (UseSpans::PatUse(span), []) =
                     (move_spans, &maybe_reinitialized_locations[..])
diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs
index 754856043b3..164ebfed0f7 100644
--- a/compiler/rustc_borrowck/src/diagnostics/mod.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs
@@ -1,11 +1,12 @@
 //! Borrow checker diagnostics.
 
-use rustc_const_eval::util::call_kind;
-use rustc_errors::Diagnostic;
+use rustc_const_eval::util::{call_kind, CallDesugaringKind};
+use rustc_errors::{Applicability, Diagnostic};
 use rustc_hir as hir;
 use rustc_hir::def::Namespace;
 use rustc_hir::def_id::DefId;
 use rustc_hir::GeneratorKind;
+use rustc_infer::infer::TyCtxtInferExt;
 use rustc_middle::mir::{
     AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand,
     Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
@@ -13,8 +14,9 @@ use rustc_middle::mir::{
 use rustc_middle::ty::print::Print;
 use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
 use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
-use rustc_span::{symbol::sym, Span};
+use rustc_span::{symbol::sym, Span, DUMMY_SP};
 use rustc_target::abi::VariantIdx;
+use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;
 
 use super::borrow_set::BorrowData;
 use super::MirBorrowckCtxt;
@@ -482,9 +484,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             BorrowedContentSource::DerefSharedRef
         }
     }
-}
 
-impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     /// Return the name of the provided `Ty` (that must be a reference) with a synthesized lifetime
     /// name where required.
     pub(super) fn get_name_for_ty(&self, ty: Ty<'tcx>, counter: usize) -> String {
@@ -995,4 +995,173 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         let span = self.body.source_info(borrow.reserve_location).span;
         self.borrow_spans(span, borrow.reserve_location)
     }
+
+    fn explain_captures(
+        &mut self,
+        err: &mut Diagnostic,
+        span: Span,
+        move_span: Span,
+        move_spans: UseSpans<'tcx>,
+        moved_place: Place<'tcx>,
+        used_place: Option<PlaceRef<'tcx>>,
+        partially_str: &str,
+        loop_message: &str,
+        move_msg: &str,
+        is_loop_move: bool,
+        maybe_reinitialized_locations_is_empty: bool,
+    ) {
+        if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } = move_spans {
+            let place_name = self
+                .describe_place(moved_place.as_ref())
+                .map(|n| format!("`{}`", n))
+                .unwrap_or_else(|| "value".to_owned());
+            match kind {
+                CallKind::FnCall { fn_trait_id, .. }
+                    if Some(fn_trait_id) == self.infcx.tcx.lang_items().fn_once_trait() =>
+                {
+                    err.span_label(
+                        fn_call_span,
+                        &format!(
+                            "{} {}moved due to this call{}",
+                            place_name, partially_str, loop_message
+                        ),
+                    );
+                    err.span_note(
+                        var_span,
+                        "this value implements `FnOnce`, which causes it to be moved when called",
+                    );
+                }
+                CallKind::Operator { self_arg, .. } => {
+                    let self_arg = self_arg.unwrap();
+                    err.span_label(
+                        fn_call_span,
+                        &format!(
+                            "{} {}moved due to usage in operator{}",
+                            place_name, partially_str, loop_message
+                        ),
+                    );
+                    if self.fn_self_span_reported.insert(fn_span) {
+                        err.span_note(
+                            // Check whether the source is accessible
+                            if self
+                                .infcx
+                                .tcx
+                                .sess
+                                .source_map()
+                                .span_to_snippet(self_arg.span)
+                                .is_ok()
+                            {
+                                self_arg.span
+                            } else {
+                                fn_call_span
+                            },
+                            "calling this operator moves the left-hand side",
+                        );
+                    }
+                }
+                CallKind::Normal { self_arg, desugaring, is_option_or_result } => {
+                    let self_arg = self_arg.unwrap();
+                    if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
+                        let ty = moved_place.ty(self.body, self.infcx.tcx).ty;
+                        let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) {
+                            Some(def_id) => self.infcx.tcx.infer_ctxt().enter(|infcx| {
+                                type_known_to_meet_bound_modulo_regions(
+                                    &infcx,
+                                    self.param_env,
+                                    infcx.tcx.mk_imm_ref(
+                                        infcx.tcx.lifetimes.re_erased,
+                                        infcx.tcx.erase_regions(ty),
+                                    ),
+                                    def_id,
+                                    DUMMY_SP,
+                                )
+                            }),
+                            _ => false,
+                        };
+                        if suggest {
+                            err.span_suggestion_verbose(
+                                move_span.shrink_to_lo(),
+                                &format!(
+                                    "consider iterating over a slice of the `{}`'s content to \
+                                     avoid moving into the `for` loop",
+                                    ty,
+                                ),
+                                "&".to_string(),
+                                Applicability::MaybeIncorrect,
+                            );
+                        }
+
+                        err.span_label(
+                            fn_call_span,
+                            &format!(
+                                "{} {}moved due to this implicit call to `.into_iter()`{}",
+                                place_name, partially_str, loop_message
+                            ),
+                        );
+                        // If we have a `&mut` ref, we need to reborrow.
+                        if let Some(ty::Ref(_, _, hir::Mutability::Mut)) = used_place
+                            .map(|used_place| used_place.ty(self.body, self.infcx.tcx).ty.kind())
+                        {
+                            // If we are in a loop this will be suggested later.
+                            if !is_loop_move {
+                                err.span_suggestion_verbose(
+                                    move_span.shrink_to_lo(),
+                                    &format!(
+                                        "consider creating a fresh reborrow of {} here",
+                                        self.describe_place(moved_place.as_ref())
+                                            .map(|n| format!("`{}`", n))
+                                            .unwrap_or_else(|| "the mutable reference".to_string()),
+                                    ),
+                                    "&mut *".to_string(),
+                                    Applicability::MachineApplicable,
+                                );
+                            }
+                        }
+                    } else {
+                        err.span_label(
+                            fn_call_span,
+                            &format!(
+                                "{} {}moved due to this method call{}",
+                                place_name, partially_str, loop_message
+                            ),
+                        );
+                    }
+                    if is_option_or_result && maybe_reinitialized_locations_is_empty {
+                        err.span_suggestion_verbose(
+                            fn_call_span.shrink_to_lo(),
+                            "consider calling `.as_ref()` to borrow the type's contents",
+                            "as_ref().".to_string(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                    // Avoid pointing to the same function in multiple different
+                    // error messages.
+                    if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
+                        err.span_note(
+                            self_arg.span,
+                            &format!("this function takes ownership of the receiver `self`, which moves {}", place_name)
+                        );
+                    }
+                }
+                // Other desugarings takes &self, which cannot cause a move
+                _ => {}
+            }
+        } else {
+            if move_span != span || !loop_message.is_empty() {
+                err.span_label(
+                    move_span,
+                    format!("value {}moved{} here{}", partially_str, move_msg, loop_message),
+                );
+            }
+            // If the move error occurs due to a loop, don't show
+            // another message for the same span
+            if loop_message.is_empty() {
+                move_spans.var_span_label(
+                    err,
+                    format!("variable {}moved due to use{}", partially_str, move_spans.describe()),
+                    "moved",
+                );
+            }
+        }
+    }
 }
diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
index 19091569b4d..f76ec031aa9 100644
--- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
@@ -1,15 +1,12 @@
-use rustc_const_eval::util::CallDesugaringKind;
 use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
-use rustc_infer::infer::TyCtxtInferExt;
 use rustc_middle::mir::*;
 use rustc_middle::ty;
 use rustc_mir_dataflow::move_paths::{
     IllegalMoveOrigin, IllegalMoveOriginKind, LookupResult, MoveError, MovePathIndex,
 };
-use rustc_span::{sym, Span, DUMMY_SP};
-use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;
+use rustc_span::{sym, Span};
 
-use crate::diagnostics::{CallKind, UseSpans};
+use crate::diagnostics::UseSpans;
 use crate::prefixes::PrefixSet;
 use crate::MirBorrowckCtxt;
 
@@ -409,34 +406,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 ".as_ref()".to_string(),
                 Applicability::MaybeIncorrect,
             );
-        } else if let Some(UseSpans::FnSelfUse {
-            kind:
-                CallKind::Normal { desugaring: Some((CallDesugaringKind::ForLoopIntoIter, _)), .. },
-            ..
-        }) = use_spans
-        {
-            let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) {
-                Some(def_id) => self.infcx.tcx.infer_ctxt().enter(|infcx| {
-                    type_known_to_meet_bound_modulo_regions(
-                        &infcx,
-                        self.param_env,
-                        infcx
-                            .tcx
-                            .mk_imm_ref(infcx.tcx.lifetimes.re_erased, infcx.tcx.erase_regions(ty)),
-                        def_id,
-                        DUMMY_SP,
-                    )
-                }),
-                _ => false,
-            };
-            if suggest {
-                err.span_suggestion_verbose(
-                    span.shrink_to_lo(),
-                    &format!("consider iterating over a slice of the `{}`'s content", ty),
-                    "&".to_string(),
-                    Applicability::MaybeIncorrect,
-                );
-            }
+        } else if let Some(use_spans) = use_spans {
+            self.explain_captures(
+                &mut err, span, span, use_spans, move_place, None, "", "", "", false, true,
+            );
         }
         err
     }
@@ -491,11 +464,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 self.note_type_does_not_implement_copy(err, &place_desc, place_ty, Some(span), "");
 
                 use_spans.args_span_label(err, format!("move out of {} occurs here", place_desc));
-                use_spans.var_span_label(
-                    err,
-                    format!("move occurs due to use{}", use_spans.describe()),
-                    "moved",
-                );
             }
         }
     }