From d687d46f68295c5e8aa318492500d22450d89b20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar> Date: Thu, 3 Nov 2022 13:51:44 -0700 Subject: [PATCH] Tweak output in for loops Do not suggest `.clone()` as we already suggest borrowing the iterated value. --- .../src/diagnostics/conflict_errors.rs | 14 +++++++++++--- src/test/ui/issues/issue-61108.stderr | 4 ---- src/test/ui/issues/issue-64559.stderr | 4 ---- src/test/ui/moves/move-fn-self-receiver.stderr | 4 ---- .../ui/suggestions/borrow-for-loop-head.stderr | 4 ---- 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 18d8bbf9047..e917d527eb3 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -397,8 +397,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { && 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(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id) && let Some(arg) = fn_sig.decl.inputs.get(pos + offset) { let mut span: MultiSpan = arg.span.into(); @@ -425,7 +424,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } let place = &self.move_data.move_paths[mpi].place; let ty = place.ty(self.body, self.infcx.tcx).ty; - self.suggest_cloning(err, ty, move_span); + if let hir::Node::Expr(parent_expr) = parent + && let hir::ExprKind::Call(call_expr, _) = parent_expr.kind + && let hir::ExprKind::Path( + hir::QPath::LangItem(LangItem::IntoIterIntoIter, _, _) + ) = call_expr.kind + { + // Do not suggest `.clone()` in a `for` loop, we already suggest borrowing. + } else { + self.suggest_cloning(err, ty, move_span); + } } } if let Some(pat) = finder.pat && !seen_spans.contains(&pat.span) { diff --git a/src/test/ui/issues/issue-61108.stderr b/src/test/ui/issues/issue-61108.stderr index 0f6f30531b3..e5b671d7b7a 100644 --- a/src/test/ui/issues/issue-61108.stderr +++ b/src/test/ui/issues/issue-61108.stderr @@ -14,10 +14,6 @@ note: this function takes ownership of the receiver `self`, which moves `bad_let | LL | fn into_iter(self) -> Self::IntoIter; | ^^^^ -help: consider cloning the value if the performance cost is acceptable - | -LL | for l in bad_letters.clone() { - | ++++++++ help: consider iterating over a slice of the `Vec<char>`'s content to avoid moving into the `for` loop | LL | for l in &bad_letters { diff --git a/src/test/ui/issues/issue-64559.stderr b/src/test/ui/issues/issue-64559.stderr index 5c04cefa530..ef178bbd155 100644 --- a/src/test/ui/issues/issue-64559.stderr +++ b/src/test/ui/issues/issue-64559.stderr @@ -15,10 +15,6 @@ note: this function takes ownership of the receiver `self`, which moves `orig` | LL | fn into_iter(self) -> Self::IntoIter; | ^^^^ -help: consider cloning the value if the performance cost is acceptable - | -LL | for _val in orig.clone() {} - | ++++++++ help: consider iterating over a slice of the `Vec<bool>`'s content to avoid moving into the `for` loop | LL | for _val in &orig {} diff --git a/src/test/ui/moves/move-fn-self-receiver.stderr b/src/test/ui/moves/move-fn-self-receiver.stderr index b331b10a3b7..c13dc58826e 100644 --- a/src/test/ui/moves/move-fn-self-receiver.stderr +++ b/src/test/ui/moves/move-fn-self-receiver.stderr @@ -127,10 +127,6 @@ LL | for _val in implicit_into_iter {} LL | implicit_into_iter; | ^^^^^^^^^^^^^^^^^^ value used here after move | -help: consider cloning the value if the performance cost is acceptable - | -LL | for _val in implicit_into_iter.clone() {} - | ++++++++ help: consider iterating over a slice of the `Vec<bool>`'s content to avoid moving into the `for` loop | LL | for _val in &implicit_into_iter {} diff --git a/src/test/ui/suggestions/borrow-for-loop-head.stderr b/src/test/ui/suggestions/borrow-for-loop-head.stderr index 7cd411dde6c..0cc8994fe1f 100644 --- a/src/test/ui/suggestions/borrow-for-loop-head.stderr +++ b/src/test/ui/suggestions/borrow-for-loop-head.stderr @@ -21,10 +21,6 @@ note: this function takes ownership of the receiver `self`, which moves `a` | LL | fn into_iter(self) -> Self::IntoIter; | ^^^^ -help: consider cloning the value if the performance cost is acceptable - | -LL | for j in a.clone() { - | ++++++++ help: consider iterating over a slice of the `Vec<i32>`'s content to avoid moving into the `for` loop | LL | for j in &a {