Suggest `.clone()` or `ref binding` on E0382

9 files changed, 187 insertions, 29 deletions

diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
index 54bd25d6471..28072f153a4 100644
--- a/compiler/rustc_ast/src/ast.rs
+++ b/compiler/rustc_ast/src/ast.rs
@@ -1376,7 +1376,7 @@ pub enum ExprKind {
     /// Conditionless loop (can be exited with `break`, `continue`, or `return`).
     ///
     /// `'label: loop { block }`
-    Loop(P<Block>, Option<Label>),
+    Loop(P<Block>, Option<Label>, Span),
     /// A `match` block.
     Match(P<Expr>, Vec<Arm>),
     /// A closure (e.g., `move |a, b, c| a + b + c`).
diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
index 4e1dcb2842f..a5b24c403dd 100644
--- a/compiler/rustc_ast/src/mut_visit.rs
+++ b/compiler/rustc_ast/src/mut_visit.rs
@@ -1355,9 +1355,10 @@ pub fn noop_visit_expr<T: MutVisitor>(
             vis.visit_block(body);
             visit_opt(label, |label| vis.visit_label(label));
         }
-        ExprKind::Loop(body, label) => {
+        ExprKind::Loop(body, label, span) => {
             vis.visit_block(body);
             visit_opt(label, |label| vis.visit_label(label));
+            vis.visit_span(span);
         }
         ExprKind::Match(expr, arms) => {
             vis.visit_expr(expr);
diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs
index 5c69e535212..c528118be08 100644
--- a/compiler/rustc_ast/src/visit.rs
+++ b/compiler/rustc_ast/src/visit.rs
@@ -824,7 +824,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
             visitor.visit_expr(subexpression);
             visitor.visit_block(block);
         }
-        ExprKind::Loop(block, opt_label) => {
+        ExprKind::Loop(block, opt_label, _) => {
             walk_list!(visitor, visit_label, opt_label);
             visitor.visit_block(block);
         }
diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
index a00100ee0a8..6215d9af370 100644
--- a/compiler/rustc_ast_lowering/src/expr.rs
+++ b/compiler/rustc_ast_lowering/src/expr.rs
@@ -131,12 +131,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     let span = this.mark_span_with_reason(DesugaringKind::WhileLoop, e.span, None);
                     this.lower_expr_while_in_loop_scope(span, cond, body, *opt_label)
                 }),
-                ExprKind::Loop(body, opt_label) => self.with_loop_scope(e.id, |this| {
+                ExprKind::Loop(body, opt_label, span) => self.with_loop_scope(e.id, |this| {
                     hir::ExprKind::Loop(
                         this.lower_block(body, false),
                         this.lower_label(*opt_label),
                         hir::LoopSource::Loop,
-                        DUMMY_SP,
+                        this.lower_span(span),
                     )
                 }),
                 ExprKind::TryBlock(body) => self.lower_expr_try_block(body),
diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
index 1da40d2302e..4b37fa027f5 100644
--- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
+++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
@@ -377,7 +377,7 @@ impl<'a> State<'a> {
                 self.space();
                 self.print_block_with_attrs(blk, attrs);
             }
-            ast::ExprKind::Loop(ref blk, opt_label) => {
+            ast::ExprKind::Loop(ref blk, opt_label, _) => {
                 if let Some(label) = opt_label {
                     self.print_ident(label.ident);
                     self.word_space(":");
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index 5f99d86b4ea..9b6836039a1 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -191,6 +191,146 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     is_loop_move = true;
                 }
 
+                struct ExpressionFinder<'hir> {
+                    expr_span: Span,
+                    expr: Option<&'hir hir::Expr<'hir>>,
+                    pat: Option<&'hir hir::Pat<'hir>>,
+                }
+                impl<'hir> Visitor<'hir> for ExpressionFinder<'hir> {
+                    fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) {
+                        if e.span == self.expr_span {
+                            self.expr = Some(e);
+                        }
+                        hir::intravisit::walk_expr(self, e);
+                    }
+                    fn visit_pat(&mut self, p: &'hir hir::Pat<'hir>) {
+                        if p.span == self.expr_span {
+                            self.pat = Some(p);
+                        }
+                        if let hir::PatKind::Binding(hir::BindingAnnotation::NONE, _, i, _) = p.kind
+                            && i.span == self.expr_span
+                        {
+                            self.pat = Some(p);
+                        }
+                        hir::intravisit::walk_pat(self, p);
+                    }
+                }
+
+                let hir = self.infcx.tcx.hir();
+                if let Some(hir::Node::Item(hir::Item {
+                    kind: hir::ItemKind::Fn(_, _, body_id),
+                    ..
+                })) = hir.find(hir.local_def_id_to_hir_id(self.mir_def_id()))
+                    && let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id)
+                {
+                    let place = &self.move_data.move_paths[mpi].place;
+                    let span = place.as_local()
+                        .map(|local| self.body.local_decls[local].source_info.span);
+                    let mut finder = ExpressionFinder {
+                        expr_span: move_span,
+                        expr: None,
+                        pat: None,
+                    };
+                    finder.visit_expr(expr);
+                    if let Some(span) = span && let Some(expr) = finder.expr {
+                        for (_, expr) in hir.parent_iter(expr.hir_id) {
+                            if let hir::Node::Expr(expr) = expr {
+                                if expr.span.contains(span) {
+                                    // If the let binding occurs within the same loop, then that
+                                    // loop isn't relevant, like in the following, the outermost `loop`
+                                    // doesn't play into `x` being moved.
+                                    // ```
+                                    // loop {
+                                    //     let x = String::new();
+                                    //     loop {
+                                    //         foo(x);
+                                    //     }
+                                    // }
+                                    // ```
+                                    break;
+                                }
+                                if let hir::ExprKind::Loop(.., loop_span) = expr.kind {
+                                    err.span_label(loop_span, "inside of this loop");
+                                }
+                            }
+                        }
+                        let typeck = self.infcx.tcx.typeck(self.mir_def_id());
+                        let hir_id = hir.get_parent_node(expr.hir_id);
+                        if let Some(parent) = hir.find(hir_id) {
+                            if let hir::Node::Expr(parent_expr) = parent
+                                && let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind
+                                && let Some(def_id) = typeck.type_dependent_def_id(parent_expr.hir_id)
+                                && let Some(def_id) = def_id.as_local()
+                                && let Some(node) = hir.find(hir.local_def_id_to_hir_id(def_id))
+                                && let Some(fn_sig) = node.fn_sig()
+                                && let Some(ident) = node.ident()
+                                && let Some(pos) = args.iter()
+                                    .position(|arg| arg.hir_id == expr.hir_id)
+                                && let Some(arg) = fn_sig.decl.inputs.get(pos + 1)
+                            {
+                                let mut span: MultiSpan = arg.span.into();
+                                span.push_span_label(
+                                    arg.span,
+                                    "this type parameter takes ownership of the value".to_string(),
+                                );
+                                span.push_span_label(
+                                    ident.span,
+                                    "in this method".to_string(),
+                                );
+                                err.span_note(
+                                    span,
+                                    format!(
+                                        "consider changing this parameter type in `{}` to borrow \
+                                         instead if ownering the value isn't necessary",
+                                        ident,
+                                    ),
+                                );
+                            }
+                            if let hir::Node::Expr(parent_expr) = parent
+                                && let hir::ExprKind::Call(call, args) = parent_expr.kind
+                                && let ty::FnDef(def_id, _) = typeck.node_type(call.hir_id).kind()
+                                && let Some(def_id) = def_id.as_local()
+                                && let Some(node) = hir.find(hir.local_def_id_to_hir_id(def_id))
+                                && let Some(fn_sig) = node.fn_sig()
+                                && let Some(ident) = node.ident()
+                                && let Some(pos) = args.iter()
+                                    .position(|arg| arg.hir_id == expr.hir_id)
+                                && let Some(arg) = fn_sig.decl.inputs.get(pos)
+                            {
+                                let mut span: MultiSpan = arg.span.into();
+                                span.push_span_label(
+                                    arg.span,
+                                    "this type parameter takes ownership of the value".to_string(),
+                                );
+                                span.push_span_label(
+                                    ident.span,
+                                    "in this function".to_string(),
+                                );
+                                err.span_note(
+                                    span,
+                                    format!(
+                                        "consider changing this parameter type in `{}` to borrow \
+                                         instead if ownering the value isn't necessary",
+                                        ident,
+                                    ),
+                                );
+                            }
+                            let place = &self.move_data.move_paths[mpi].place;
+                            let ty = place.ty(self.body, self.infcx.tcx).ty;
+                            self.suggest_cloning(&mut err, ty, move_span);
+                        }
+                    }
+                    if let Some(pat) = finder.pat {
+                        in_pattern = true;
+                        err.span_suggestion_verbose(
+                            pat.span.shrink_to_lo(),
+                            "borrow this binding in the pattern to avoid moving the value",
+                            "ref ".to_string(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                }
+
                 self.explain_captures(
                     &mut err,
                     span,
@@ -203,25 +343,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     is_loop_move,
                     maybe_reinitialized_locations.is_empty(),
                 );
-
-                if let (UseSpans::PatUse(span), []) =
-                    (move_spans, &maybe_reinitialized_locations[..])
-                {
-                    if maybe_reinitialized_locations.is_empty() {
-                        err.span_suggestion_verbose(
-                            span.shrink_to_lo(),
-                            &format!(
-                                "borrow this field in the pattern to avoid moving {}",
-                                self.describe_place(moved_place.as_ref())
-                                    .map(|n| format!("`{}`", n))
-                                    .unwrap_or_else(|| "the value".to_string())
-                            ),
-                            "ref ",
-                            Applicability::MachineApplicable,
-                        );
-                        in_pattern = true;
-                    }
-                }
             }
 
             use_spans.var_path_only_subdiag(&mut err, desired_action);
@@ -590,6 +711,39 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         true
     }
 
+    fn suggest_cloning(&self, err: &mut Diagnostic, ty: Ty<'tcx>, span: Span) {
+        let tcx = self.infcx.tcx;
+
+        // Try to find predicates on *generic params* that would allow copying `ty`
+        let infcx = tcx.infer_ctxt().build();
+        let mut fulfill_cx = <dyn rustc_infer::traits::TraitEngine<'_>>::new(infcx.tcx);
+
+        let clone_did = infcx.tcx.lang_items().clone_trait().unwrap();
+        let cause = ObligationCause::new(
+            span,
+            self.mir_hir_id(),
+            rustc_infer::traits::ObligationCauseCode::MiscObligation,
+        );
+        fulfill_cx.register_bound(
+            &infcx,
+            self.param_env,
+            // Erase any region vids from the type, which may not be resolved
+            infcx.tcx.erase_regions(ty),
+            clone_did,
+            cause,
+        );
+        // Select all, including ambiguous predicates
+        let errors = fulfill_cx.select_all_or_error(&infcx);
+        if errors.is_empty() {
+            err.span_suggestion_verbose(
+                span.shrink_to_hi(),
+                "consider cloning the value if the performance cost is acceptable",
+                ".clone()".to_string(),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+
     fn suggest_adding_copy_bounds(&self, err: &mut Diagnostic, ty: Ty<'tcx>, span: Span) {
         let tcx = self.infcx.tcx;
         let generics = tcx.generics_of(self.mir_def_id());
diff --git a/compiler/rustc_builtin_macros/src/assert/context.rs b/compiler/rustc_builtin_macros/src/assert/context.rs
index 220b7a8ad0f..9f42a0c2d58 100644
--- a/compiler/rustc_builtin_macros/src/assert/context.rs
+++ b/compiler/rustc_builtin_macros/src/assert/context.rs
@@ -307,7 +307,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
             | ExprKind::InlineAsm(_)
             | ExprKind::Let(_, _, _)
             | ExprKind::Lit(_)
-            | ExprKind::Loop(_, _)
+            | ExprKind::Loop(_, _, _)
             | ExprKind::MacCall(_)
             | ExprKind::Match(_, _)
             | ExprKind::Path(_, _)
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index dc914f5ea64..9f2267efb82 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -1734,7 +1734,7 @@ impl<'a> Parser<'a> {
                         expr.kind,
                         ExprKind::While(_, _, None)
                             | ExprKind::ForLoop(_, _, _, None)
-                            | ExprKind::Loop(_, None)
+                            | ExprKind::Loop(_, None, _)
                             | ExprKind::Block(_, None)
                     )
                 {
@@ -2444,10 +2444,11 @@ impl<'a> Parser<'a> {
 
     /// Parses `loop { ... }` (`loop` token already eaten).
     fn parse_loop_expr(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
+        let loop_span = self.prev_token.span;
         let (attrs, body) = self.parse_inner_attrs_and_block()?;
         Ok(self.mk_expr_with_attrs(
             lo.to(self.prev_token.span),
-            ExprKind::Loop(body, opt_label),
+            ExprKind::Loop(body, opt_label, loop_span),
             attrs,
         ))
     }
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index 5072d2aad16..93b0f5814de 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -3841,7 +3841,9 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                 }
             }
 
-            ExprKind::Loop(ref block, label) => self.resolve_labeled_block(label, expr.id, &block),
+            ExprKind::Loop(ref block, label, _) => {
+                self.resolve_labeled_block(label, expr.id, &block)
+            }
 
             ExprKind::While(ref cond, ref block, label) => {
                 self.with_resolved_label(label, expr.id, |this| {