From c79db9c5e5d7e0bdc230b53b01a4f65d84434082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 23 Dec 2022 09:59:39 -0800 Subject: [PATCH 1/7] Suggest `Pin::as_mut` when encountering borrow error --- .../rustc_borrowck/src/diagnostics/mod.rs | 11 +++++++++ .../ui/moves/move-fn-self-receiver.stderr | 4 ++++ src/test/ui/moves/pin-mut-reborrow.fixed | 15 ++++++++++++ src/test/ui/moves/pin-mut-reborrow.rs | 15 ++++++++++++ src/test/ui/moves/pin-mut-reborrow.stderr | 23 +++++++++++++++++++ 5 files changed, 68 insertions(+) create mode 100644 src/test/ui/moves/pin-mut-reborrow.fixed create mode 100644 src/test/ui/moves/pin-mut-reborrow.rs create mode 100644 src/test/ui/moves/pin-mut-reborrow.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index cbd590052008c..213b8b064f95b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -1133,6 +1133,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place_name, partially_str, loop_message ), ); + if let ty::Adt(def, ..) + = moved_place.ty(self.body, self.infcx.tcx).ty.kind() + && Some(def.did()) == self.infcx.tcx.lang_items().pin_type() + { + err.span_suggestion_verbose( + fn_call_span.shrink_to_lo(), + "consider reborrowing the `Pin` instead of moving it", + "as_mut().".to_string(), + Applicability::MaybeIncorrect, + ); + } } let tcx = self.infcx.tcx; // Avoid pointing to the same function in multiple different diff --git a/src/test/ui/moves/move-fn-self-receiver.stderr b/src/test/ui/moves/move-fn-self-receiver.stderr index b3f95ee192a56..0ed44b7674a5f 100644 --- a/src/test/ui/moves/move-fn-self-receiver.stderr +++ b/src/test/ui/moves/move-fn-self-receiver.stderr @@ -67,6 +67,10 @@ note: `Foo::use_pin_box_self` takes ownership of the receiver `self`, which move | LL | fn use_pin_box_self(self: Pin>) {} | ^^^^ +help: consider reborrowing the `Pin` instead of moving it + | +LL | pin_box_foo.as_mut().use_pin_box_self(); + | +++++++++ error[E0505]: cannot move out of `mut_foo` because it is borrowed --> $DIR/move-fn-self-receiver.rs:50:5 diff --git a/src/test/ui/moves/pin-mut-reborrow.fixed b/src/test/ui/moves/pin-mut-reborrow.fixed new file mode 100644 index 0000000000000..e808186d7d445 --- /dev/null +++ b/src/test/ui/moves/pin-mut-reborrow.fixed @@ -0,0 +1,15 @@ +// run-rustfix +use std::pin::Pin; + +struct Foo; + +impl Foo { + fn foo(self: Pin<&mut Self>) {} +} + +fn main() { + let mut foo = Foo; + let mut foo = Pin::new(&mut foo); + foo.as_mut().foo(); + foo.foo(); //~ ERROR use of moved value +} diff --git a/src/test/ui/moves/pin-mut-reborrow.rs b/src/test/ui/moves/pin-mut-reborrow.rs new file mode 100644 index 0000000000000..fee6236ebb4db --- /dev/null +++ b/src/test/ui/moves/pin-mut-reborrow.rs @@ -0,0 +1,15 @@ +// run-rustfix +use std::pin::Pin; + +struct Foo; + +impl Foo { + fn foo(self: Pin<&mut Self>) {} +} + +fn main() { + let mut foo = Foo; + let mut foo = Pin::new(&mut foo); + foo.foo(); + foo.foo(); //~ ERROR use of moved value +} diff --git a/src/test/ui/moves/pin-mut-reborrow.stderr b/src/test/ui/moves/pin-mut-reborrow.stderr new file mode 100644 index 0000000000000..16fa4bacc2d85 --- /dev/null +++ b/src/test/ui/moves/pin-mut-reborrow.stderr @@ -0,0 +1,23 @@ +error[E0382]: use of moved value: `foo` + --> $DIR/pin-mut-reborrow.rs:14:5 + | +LL | let mut foo = Pin::new(&mut foo); + | ------- move occurs because `foo` has type `Pin<&mut Foo>`, which does not implement the `Copy` trait +LL | foo.foo(); + | ----- `foo` moved due to this method call +LL | foo.foo(); + | ^^^ value used here after move + | +note: `Foo::foo` takes ownership of the receiver `self`, which moves `foo` + --> $DIR/pin-mut-reborrow.rs:7:12 + | +LL | fn foo(self: Pin<&mut Self>) {} + | ^^^^ +help: consider reborrowing the `Pin` instead of moving it + | +LL | foo.as_mut().foo(); + | +++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0382`. From a929316aed324598825e5a74a4082d09a846df32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 23 Dec 2022 12:44:47 -0800 Subject: [PATCH 2/7] Suggest `.clone()` on method call move errors --- Cargo.lock | 1 + compiler/rustc_borrowck/Cargo.toml | 1 + .../rustc_borrowck/src/diagnostics/mod.rs | 43 +++++++++++++++---- src/test/ui/moves/suggest-clone.fixed | 11 +++++ src/test/ui/moves/suggest-clone.rs | 11 +++++ src/test/ui/moves/suggest-clone.stderr | 22 ++++++++++ 6 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/moves/suggest-clone.fixed create mode 100644 src/test/ui/moves/suggest-clone.rs create mode 100644 src/test/ui/moves/suggest-clone.stderr diff --git a/Cargo.lock b/Cargo.lock index 5d05a09f03893..6de8ba0f8e659 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3447,6 +3447,7 @@ dependencies = [ "rustc_errors", "rustc_graphviz", "rustc_hir", + "rustc_hir_analysis", "rustc_index", "rustc_infer", "rustc_lexer", diff --git a/compiler/rustc_borrowck/Cargo.toml b/compiler/rustc_borrowck/Cargo.toml index 87c113f3e30b7..f5001172754c7 100644 --- a/compiler/rustc_borrowck/Cargo.toml +++ b/compiler/rustc_borrowck/Cargo.toml @@ -15,6 +15,7 @@ rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } rustc_graphviz = { path = "../rustc_graphviz" } rustc_hir = { path = "../rustc_hir" } +rustc_hir_analysis = { path = "../rustc_hir_analysis" } rustc_index = { path = "../rustc_index" } rustc_infer = { path = "../rustc_infer" } rustc_lexer = { path = "../rustc_lexer" } diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 213b8b064f95b..5250f39003d27 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -6,6 +6,7 @@ use rustc_errors::{Applicability, Diagnostic}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::GeneratorKind; +use rustc_hir_analysis::hir_ty_to_ty; use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ @@ -1066,18 +1067,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } CallKind::Normal { self_arg, desugaring, method_did } => { let self_arg = self_arg.unwrap(); + let tcx = self.infcx.tcx; 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) { + let ty = moved_place.ty(self.body, tcx).ty; + let suggest = match tcx.get_diagnostic_item(sym::IntoIterator) { Some(def_id) => { let infcx = self.infcx.tcx.infer_ctxt().build(); 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), - ), + tcx.mk_imm_ref(tcx.lifetimes.re_erased, tcx.erase_regions(ty)), def_id, DUMMY_SP, ) @@ -1133,8 +1132,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place_name, partially_str, loop_message ), ); - if let ty::Adt(def, ..) - = moved_place.ty(self.body, self.infcx.tcx).ty.kind() + let ty = moved_place.ty(self.body, self.infcx.tcx).ty; + if let ty::Adt(def, ..) = ty.kind() && Some(def.did()) == self.infcx.tcx.lang_items().pin_type() { err.span_suggestion_verbose( @@ -1144,8 +1143,34 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Applicability::MaybeIncorrect, ); } + if let Some(clone_trait) = tcx.lang_items().clone_trait() { + // We can't use `predicate_may_hold` or `can_eq` without ICEs in + // borrowck because of the inference context, so we do a poor-man's + // version here. + for impl_def_id in tcx.all_impls(clone_trait) { + if let Some(def_id) = impl_def_id.as_local() + && let hir_id = tcx.hir().local_def_id_to_hir_id(def_id) + && let hir::Node::Item(hir::Item { + kind: hir::ItemKind::Impl(hir::Impl { + self_ty, + .. + }), + .. + }) = tcx.hir().get(hir_id) + { + if ty == hir_ty_to_ty(tcx, self_ty) { + err.span_suggestion_verbose( + fn_call_span.shrink_to_lo(), + "you can `clone` the value and consume it, but this \ + might not be your desired behavior", + "clone().".to_string(), + Applicability::MaybeIncorrect, + ); + } + } + } + } } - let tcx = self.infcx.tcx; // Avoid pointing to the same function in multiple different // error messages. if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) { diff --git a/src/test/ui/moves/suggest-clone.fixed b/src/test/ui/moves/suggest-clone.fixed new file mode 100644 index 0000000000000..204bfdb10b0b3 --- /dev/null +++ b/src/test/ui/moves/suggest-clone.fixed @@ -0,0 +1,11 @@ +// run-rustfix + +#[derive(Clone)] +struct Foo; +impl Foo { + fn foo(self) {} +} +fn main() { + let foo = &Foo; + foo.clone().foo(); //~ ERROR cannot move out +} diff --git a/src/test/ui/moves/suggest-clone.rs b/src/test/ui/moves/suggest-clone.rs new file mode 100644 index 0000000000000..25dd9f006f9ea --- /dev/null +++ b/src/test/ui/moves/suggest-clone.rs @@ -0,0 +1,11 @@ +// run-rustfix + +#[derive(Clone)] +struct Foo; +impl Foo { + fn foo(self) {} +} +fn main() { + let foo = &Foo; + foo.foo(); //~ ERROR cannot move out +} diff --git a/src/test/ui/moves/suggest-clone.stderr b/src/test/ui/moves/suggest-clone.stderr new file mode 100644 index 0000000000000..cbb3dfea3ba9e --- /dev/null +++ b/src/test/ui/moves/suggest-clone.stderr @@ -0,0 +1,22 @@ +error[E0507]: cannot move out of `*foo` which is behind a shared reference + --> $DIR/suggest-clone.rs:10:5 + | +LL | foo.foo(); + | ^^^^----- + | | | + | | `*foo` moved due to this method call + | move occurs because `*foo` has type `Foo`, which does not implement the `Copy` trait + | +note: `Foo::foo` takes ownership of the receiver `self`, which moves `*foo` + --> $DIR/suggest-clone.rs:6:12 + | +LL | fn foo(self) {} + | ^^^^ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | foo.clone().foo(); + | ++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0507`. From 12fd9011b82c508420cd19b6b50d8626f07430cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 23 Dec 2022 15:13:04 -0800 Subject: [PATCH 3/7] Verify receiver is of `self: Pin<&mut Self>` --- compiler/rustc_borrowck/src/diagnostics/mod.rs | 8 ++++++-- src/test/ui/moves/move-fn-self-receiver.stderr | 4 ---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 5250f39003d27..762188b73b38e 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -1132,9 +1132,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place_name, partially_str, loop_message ), ); - let ty = moved_place.ty(self.body, self.infcx.tcx).ty; - if let ty::Adt(def, ..) = ty.kind() + let ty = tcx.erase_regions(moved_place.ty(self.body, self.infcx.tcx).ty); + if let ty::Adt(def, substs) = ty.kind() && Some(def.did()) == self.infcx.tcx.lang_items().pin_type() + && let ty::Ref(_, _, hir::Mutability::Mut) = substs.type_at(0).kind() + // FIXME: this is a hack because we can't call `can_eq` + && ty.to_string() == + tcx.fn_sig(method_did).input(0).skip_binder().to_string() { err.span_suggestion_verbose( fn_call_span.shrink_to_lo(), diff --git a/src/test/ui/moves/move-fn-self-receiver.stderr b/src/test/ui/moves/move-fn-self-receiver.stderr index 0ed44b7674a5f..b3f95ee192a56 100644 --- a/src/test/ui/moves/move-fn-self-receiver.stderr +++ b/src/test/ui/moves/move-fn-self-receiver.stderr @@ -67,10 +67,6 @@ note: `Foo::use_pin_box_self` takes ownership of the receiver `self`, which move | LL | fn use_pin_box_self(self: Pin>) {} | ^^^^ -help: consider reborrowing the `Pin` instead of moving it - | -LL | pin_box_foo.as_mut().use_pin_box_self(); - | +++++++++ error[E0505]: cannot move out of `mut_foo` because it is borrowed --> $DIR/move-fn-self-receiver.rs:50:5 From ea7de0d48530b34cf7f8ae18435e68463e57c6b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 23 Dec 2022 16:40:22 -0800 Subject: [PATCH 4/7] Do not use `hir_ty_to_ty` --- Cargo.lock | 1 - compiler/rustc_borrowck/Cargo.toml | 1 - .../rustc_borrowck/src/diagnostics/mod.rs | 25 ++++++++----------- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6de8ba0f8e659..5d05a09f03893 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3447,7 +3447,6 @@ dependencies = [ "rustc_errors", "rustc_graphviz", "rustc_hir", - "rustc_hir_analysis", "rustc_index", "rustc_infer", "rustc_lexer", diff --git a/compiler/rustc_borrowck/Cargo.toml b/compiler/rustc_borrowck/Cargo.toml index f5001172754c7..87c113f3e30b7 100644 --- a/compiler/rustc_borrowck/Cargo.toml +++ b/compiler/rustc_borrowck/Cargo.toml @@ -15,7 +15,6 @@ rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } rustc_graphviz = { path = "../rustc_graphviz" } rustc_hir = { path = "../rustc_hir" } -rustc_hir_analysis = { path = "../rustc_hir_analysis" } rustc_index = { path = "../rustc_index" } rustc_infer = { path = "../rustc_infer" } rustc_lexer = { path = "../rustc_lexer" } diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 762188b73b38e..989fbb066f768 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -6,7 +6,6 @@ use rustc_errors::{Applicability, Diagnostic}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::GeneratorKind; -use rustc_hir_analysis::hir_ty_to_ty; use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ @@ -1137,7 +1136,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { && Some(def.did()) == self.infcx.tcx.lang_items().pin_type() && let ty::Ref(_, _, hir::Mutability::Mut) = substs.type_at(0).kind() // FIXME: this is a hack because we can't call `can_eq` - && ty.to_string() == + && ty.to_string() == tcx.fn_sig(method_did).input(0).skip_binder().to_string() { err.span_suggestion_verbose( @@ -1155,22 +1154,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let Some(def_id) = impl_def_id.as_local() && let hir_id = tcx.hir().local_def_id_to_hir_id(def_id) && let hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { - self_ty, - .. - }), + kind: hir::ItemKind::Impl(_), .. }) = tcx.hir().get(hir_id) + && tcx.type_of(impl_def_id) == ty { - if ty == hir_ty_to_ty(tcx, self_ty) { - err.span_suggestion_verbose( - fn_call_span.shrink_to_lo(), - "you can `clone` the value and consume it, but this \ - might not be your desired behavior", - "clone().".to_string(), - Applicability::MaybeIncorrect, - ); - } + err.span_suggestion_verbose( + fn_call_span.shrink_to_lo(), + "you can `clone` the value and consume it, but this might \ + not be your desired behavior", + "clone().".to_string(), + Applicability::MaybeIncorrect, + ); } } } From 2d6a2ff76ee9443d4fb0f611e66ef2d72954ac8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 25 Dec 2022 16:51:11 -0800 Subject: [PATCH 5/7] Create new inference context --- .../rustc_borrowck/src/diagnostics/mod.rs | 62 ++++++++++--------- ...k-move-out-of-overloaded-auto-deref.stderr | 4 ++ ...move-upvar-from-non-once-ref-closure.fixed | 15 +++++ ...es-move-upvar-from-non-once-ref-closure.rs | 1 + ...ove-upvar-from-non-once-ref-closure.stderr | 6 +- src/test/ui/codemap_tests/tab_3.stderr | 4 ++ .../ui/moves/move-fn-self-receiver.stderr | 12 ++++ ...moves-based-on-type-access-to-field.stderr | 4 ++ .../ui/moves/moves-based-on-type-exprs.stderr | 8 +++ .../ui/suggestions/option-content-move.stderr | 8 +++ 10 files changed, 94 insertions(+), 30 deletions(-) create mode 100644 src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 989fbb066f768..6b5c1d1a20fbc 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -6,7 +6,7 @@ use rustc_errors::{Applicability, Diagnostic}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::GeneratorKind; -use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::infer::{LateBoundRegionConversionTime, TyCtxtInferExt}; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand, @@ -18,7 +18,10 @@ use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult}; use rustc_span::def_id::LocalDefId; use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP}; use rustc_target::abi::VariantIdx; -use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; +use rustc_trait_selection::traits::{ + type_known_to_meet_bound_modulo_regions, Obligation, ObligationCause, +}; use super::borrow_set::BorrowData; use super::MirBorrowckCtxt; @@ -1131,13 +1134,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place_name, partially_str, loop_message ), ); - let ty = tcx.erase_regions(moved_place.ty(self.body, self.infcx.tcx).ty); + let infcx = tcx.infer_ctxt().build(); + let ty = infcx.freshen(moved_place.ty(self.body, tcx).ty); if let ty::Adt(def, substs) = ty.kind() - && Some(def.did()) == self.infcx.tcx.lang_items().pin_type() + && Some(def.did()) == tcx.lang_items().pin_type() && let ty::Ref(_, _, hir::Mutability::Mut) = substs.type_at(0).kind() - // FIXME: this is a hack because we can't call `can_eq` - && ty.to_string() == - tcx.fn_sig(method_did).input(0).skip_binder().to_string() + && let self_ty = infcx.freshen( + infcx.replace_bound_vars_with_fresh_vars( + fn_call_span, + LateBoundRegionConversionTime::FnCall, + tcx.fn_sig(method_did).input(0), + ) + ) + && infcx.can_eq(self.param_env, ty, self_ty).is_ok() { err.span_suggestion_verbose( fn_call_span.shrink_to_lo(), @@ -1146,28 +1155,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Applicability::MaybeIncorrect, ); } - if let Some(clone_trait) = tcx.lang_items().clone_trait() { - // We can't use `predicate_may_hold` or `can_eq` without ICEs in - // borrowck because of the inference context, so we do a poor-man's - // version here. - for impl_def_id in tcx.all_impls(clone_trait) { - if let Some(def_id) = impl_def_id.as_local() - && let hir_id = tcx.hir().local_def_id_to_hir_id(def_id) - && let hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(_), - .. - }) = tcx.hir().get(hir_id) - && tcx.type_of(impl_def_id) == ty - { - err.span_suggestion_verbose( - fn_call_span.shrink_to_lo(), - "you can `clone` the value and consume it, but this might \ - not be your desired behavior", - "clone().".to_string(), - Applicability::MaybeIncorrect, - ); - } - } + if let Some(clone_trait) = tcx.lang_items().clone_trait() + && let trait_ref = tcx.mk_trait_ref(clone_trait, [ty]) + && let o = Obligation::new( + tcx, + ObligationCause::dummy(), + self.param_env, + ty::Binder::dummy(trait_ref), + ) + && infcx.predicate_must_hold_modulo_regions(&o) + { + err.span_suggestion_verbose( + fn_call_span.shrink_to_lo(), + "you can `clone` the value and consume it, but this might not be \ + your desired behavior", + "clone().".to_string(), + Applicability::MaybeIncorrect, + ); } } // Avoid pointing to the same function in multiple different diff --git a/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr b/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr index ecf5382e863e0..87135f0bb438f 100644 --- a/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr +++ b/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr @@ -9,6 +9,10 @@ LL | let _x = Rc::new(vec![1, 2]).into_iter(); | note: `into_iter` takes ownership of the receiver `self`, which moves value --> $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 + | +LL | let _x = Rc::new(vec![1, 2]).clone().into_iter(); + | ++++++++ error: aborting due to previous error diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed new file mode 100644 index 0000000000000..b0c5376105b28 --- /dev/null +++ b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed @@ -0,0 +1,15 @@ +// run-rustfix +// Test that a by-ref `FnMut` closure gets an error when it tries to +// consume a value. + +fn call(f: F) where F : Fn() { + f(); +} + +fn main() { + let y = vec![format!("World")]; + call(|| { + y.clone().into_iter(); + //~^ ERROR cannot move out of `y`, a captured variable in an `Fn` closure + }); +} diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.rs b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.rs index d54b09c5da95a..4666b8a337353 100644 --- a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.rs +++ b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.rs @@ -1,3 +1,4 @@ +// run-rustfix // Test that a by-ref `FnMut` closure gets an error when it tries to // consume a value. diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr index b1367c652188b..f033d53bf8e48 100644 --- a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr +++ b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr @@ -1,5 +1,5 @@ error[E0507]: cannot move out of `y`, a captured variable in an `Fn` closure - --> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:11:9 + --> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:12:9 | LL | let y = vec![format!("World")]; | - captured outer variable @@ -12,6 +12,10 @@ LL | y.into_iter(); | 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 + | +LL | y.clone().into_iter(); + | ++++++++ error: aborting due to previous error diff --git a/src/test/ui/codemap_tests/tab_3.stderr b/src/test/ui/codemap_tests/tab_3.stderr index e0e369124a4be..6f87a2a367e45 100644 --- a/src/test/ui/codemap_tests/tab_3.stderr +++ b/src/test/ui/codemap_tests/tab_3.stderr @@ -16,6 +16,10 @@ help: consider cloning the value if the performance cost is acceptable | LL | some_vec.clone().into_iter(); | ++++++++ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | some_vec.clone().into_iter(); + | ++++++++ error: aborting due to previous error diff --git a/src/test/ui/moves/move-fn-self-receiver.stderr b/src/test/ui/moves/move-fn-self-receiver.stderr index b3f95ee192a56..106fd31a088c0 100644 --- a/src/test/ui/moves/move-fn-self-receiver.stderr +++ b/src/test/ui/moves/move-fn-self-receiver.stderr @@ -9,6 +9,10 @@ LL | val.0; note: `into_iter` takes ownership of the receiver `self`, which moves `val.0` --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL = note: move occurs because `val.0` has type `Vec`, which does not implement the `Copy` trait +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | val.0.clone().into_iter().next(); + | ++++++++ error[E0382]: use of moved value: `foo` --> $DIR/move-fn-self-receiver.rs:34:5 @@ -97,6 +101,10 @@ help: consider cloning the value if the performance cost is acceptable | LL | rc_foo.clone().use_rc_self(); | ++++++++ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | rc_foo.clone().use_rc_self(); + | ++++++++ error[E0382]: use of moved value: `foo_add` --> $DIR/move-fn-self-receiver.rs:59:5 @@ -140,6 +148,10 @@ help: consider cloning the value if the performance cost is acceptable | LL | for _val in explicit_into_iter.clone().into_iter() {} | ++++++++ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | for _val in explicit_into_iter.clone().into_iter() {} + | ++++++++ error[E0382]: use of moved value: `container` --> $DIR/move-fn-self-receiver.rs:71:5 diff --git a/src/test/ui/moves/moves-based-on-type-access-to-field.stderr b/src/test/ui/moves/moves-based-on-type-access-to-field.stderr index 0b1a623a01345..b51ba16cca191 100644 --- a/src/test/ui/moves/moves-based-on-type-access-to-field.stderr +++ b/src/test/ui/moves/moves-based-on-type-access-to-field.stderr @@ -14,6 +14,10 @@ help: consider cloning the value if the performance cost is acceptable | LL | consume(x.clone().into_iter().next().unwrap()); | ++++++++ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | consume(x.clone().into_iter().next().unwrap()); + | ++++++++ error: aborting due to previous error diff --git a/src/test/ui/moves/moves-based-on-type-exprs.stderr b/src/test/ui/moves/moves-based-on-type-exprs.stderr index ae76889f104c8..8458d9cad5fbd 100644 --- a/src/test/ui/moves/moves-based-on-type-exprs.stderr +++ b/src/test/ui/moves/moves-based-on-type-exprs.stderr @@ -166,6 +166,10 @@ help: consider cloning the value if the performance cost is acceptable | LL | let _y = x.clone().into_iter().next().unwrap(); | ++++++++ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | let _y = x.clone().into_iter().next().unwrap(); + | ++++++++ error[E0382]: borrow of moved value: `x` --> $DIR/moves-based-on-type-exprs.rs:83:11 @@ -183,6 +187,10 @@ help: consider cloning the value if the performance cost is acceptable | LL | let _y = [x.clone().into_iter().next().unwrap(); 1]; | ++++++++ +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | let _y = [x.clone().into_iter().next().unwrap(); 1]; + | ++++++++ error: aborting due to 11 previous errors diff --git a/src/test/ui/suggestions/option-content-move.stderr b/src/test/ui/suggestions/option-content-move.stderr index 3e0271d02572b..474a72093c631 100644 --- a/src/test/ui/suggestions/option-content-move.stderr +++ b/src/test/ui/suggestions/option-content-move.stderr @@ -9,6 +9,10 @@ LL | if selection.1.unwrap().contains(selection.0) { | note: `Option::::unwrap` takes ownership of the receiver `self`, which moves `selection.1` --> $SRC_DIR/core/src/option.rs:LL:COL +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | if selection.1.clone().unwrap().contains(selection.0) { + | ++++++++ error[E0507]: cannot move out of `selection.1` which is behind a shared reference --> $DIR/option-content-move.rs:27:20 @@ -21,6 +25,10 @@ LL | if selection.1.unwrap().contains(selection.0) { | note: `Result::::unwrap` takes ownership of the receiver `self`, which moves `selection.1` --> $SRC_DIR/core/src/result.rs:LL:COL +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | if selection.1.clone().unwrap().contains(selection.0) { + | ++++++++ error: aborting due to 2 previous errors From bd890f9cd1e06b5b8c635db2def235373290b09d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 25 Dec 2022 17:16:54 -0800 Subject: [PATCH 6/7] Remove redundant clone suggestion --- .../src/diagnostics/conflict_errors.rs | 14 +++++++++++++- src/test/ui/codemap_tests/tab_3.stderr | 4 ---- src/test/ui/moves/move-fn-self-receiver.stderr | 8 -------- .../moves-based-on-type-access-to-field.stderr | 4 ---- src/test/ui/moves/moves-based-on-type-exprs.stderr | 8 -------- 5 files changed, 13 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 72c0257756ef2..3f6acf5f5d108 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -194,7 +194,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if !seen_spans.contains(&move_span) { if !closure { - self.suggest_ref_or_clone(mpi, move_span, &mut err, &mut in_pattern); + self.suggest_ref_or_clone( + mpi, + move_span, + &mut err, + &mut in_pattern, + move_spans, + ); } self.explain_captures( @@ -312,6 +318,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { move_span: Span, err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>, in_pattern: &mut bool, + move_spans: UseSpans<'_>, ) { struct ExpressionFinder<'hir> { expr_span: Span, @@ -440,6 +447,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) = call_expr.kind { // Do not suggest `.clone()` in a `for` loop, we already suggest borrowing. + } else if let UseSpans::FnSelfUse { + kind: CallKind::Normal { .. }, + .. + } = move_spans { + // We already suggest cloning for these cases in `explain_captures`. } else { self.suggest_cloning(err, ty, move_span); } diff --git a/src/test/ui/codemap_tests/tab_3.stderr b/src/test/ui/codemap_tests/tab_3.stderr index 6f87a2a367e45..17bea2f366fa0 100644 --- a/src/test/ui/codemap_tests/tab_3.stderr +++ b/src/test/ui/codemap_tests/tab_3.stderr @@ -12,10 +12,6 @@ LL | println!("{:?}", some_vec); note: `into_iter` takes ownership of the receiver `self`, which moves `some_vec` --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider cloning the value if the performance cost is acceptable - | -LL | some_vec.clone().into_iter(); - | ++++++++ help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | some_vec.clone().into_iter(); diff --git a/src/test/ui/moves/move-fn-self-receiver.stderr b/src/test/ui/moves/move-fn-self-receiver.stderr index 106fd31a088c0..7f69e5dcfb784 100644 --- a/src/test/ui/moves/move-fn-self-receiver.stderr +++ b/src/test/ui/moves/move-fn-self-receiver.stderr @@ -97,10 +97,6 @@ note: `Foo::use_rc_self` takes ownership of the receiver `self`, which moves `rc | LL | fn use_rc_self(self: Rc) {} | ^^^^ -help: consider cloning the value if the performance cost is acceptable - | -LL | rc_foo.clone().use_rc_self(); - | ++++++++ help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | rc_foo.clone().use_rc_self(); @@ -144,10 +140,6 @@ LL | for _val in explicit_into_iter.into_iter() {} LL | explicit_into_iter; | ^^^^^^^^^^^^^^^^^^ value used here after move | -help: consider cloning the value if the performance cost is acceptable - | -LL | for _val in explicit_into_iter.clone().into_iter() {} - | ++++++++ help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | for _val in explicit_into_iter.clone().into_iter() {} diff --git a/src/test/ui/moves/moves-based-on-type-access-to-field.stderr b/src/test/ui/moves/moves-based-on-type-access-to-field.stderr index b51ba16cca191..a28f324aafac9 100644 --- a/src/test/ui/moves/moves-based-on-type-access-to-field.stderr +++ b/src/test/ui/moves/moves-based-on-type-access-to-field.stderr @@ -10,10 +10,6 @@ LL | touch(&x[0]); | note: `into_iter` takes ownership of the receiver `self`, which moves `x` --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL -help: consider cloning the value if the performance cost is acceptable - | -LL | consume(x.clone().into_iter().next().unwrap()); - | ++++++++ help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | consume(x.clone().into_iter().next().unwrap()); diff --git a/src/test/ui/moves/moves-based-on-type-exprs.stderr b/src/test/ui/moves/moves-based-on-type-exprs.stderr index 8458d9cad5fbd..ab7c27456882f 100644 --- a/src/test/ui/moves/moves-based-on-type-exprs.stderr +++ b/src/test/ui/moves/moves-based-on-type-exprs.stderr @@ -162,10 +162,6 @@ LL | touch(&x); | note: `into_iter` takes ownership of the receiver `self`, which moves `x` --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL -help: consider cloning the value if the performance cost is acceptable - | -LL | let _y = x.clone().into_iter().next().unwrap(); - | ++++++++ help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | let _y = x.clone().into_iter().next().unwrap(); @@ -183,10 +179,6 @@ LL | touch(&x); | note: `into_iter` takes ownership of the receiver `self`, which moves `x` --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL -help: consider cloning the value if the performance cost is acceptable - | -LL | let _y = [x.clone().into_iter().next().unwrap(); 1]; - | ++++++++ help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | let _y = [x.clone().into_iter().next().unwrap(); 1]; From 8a13a7c14874147621e5344e3f31aaed39d390f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 26 Dec 2022 13:35:35 -0800 Subject: [PATCH 7/7] review comments --- compiler/rustc_borrowck/src/diagnostics/mod.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 6b5c1d1a20fbc..63b16aa95a6a5 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -1135,16 +1135,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ), ); let infcx = tcx.infer_ctxt().build(); - let ty = infcx.freshen(moved_place.ty(self.body, tcx).ty); + let ty = tcx.erase_regions(moved_place.ty(self.body, tcx).ty); if let ty::Adt(def, substs) = ty.kind() && Some(def.did()) == tcx.lang_items().pin_type() && let ty::Ref(_, _, hir::Mutability::Mut) = substs.type_at(0).kind() - && let self_ty = infcx.freshen( - infcx.replace_bound_vars_with_fresh_vars( - fn_call_span, - LateBoundRegionConversionTime::FnCall, - tcx.fn_sig(method_did).input(0), - ) + && let self_ty = infcx.replace_bound_vars_with_fresh_vars( + fn_call_span, + LateBoundRegionConversionTime::FnCall, + tcx.fn_sig(method_did).input(0), ) && infcx.can_eq(self.param_env, ty, self_ty).is_ok() {