From 2280d8ea0028bf163b1d9ac25514614823843140 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 20 Oct 2024 04:22:07 -0700 Subject: [PATCH 1/3] Provide borrow-instead-of-move suggestions for calls of fn-like items from other crates This also downgrades its applicability to MaybeIncorrect. Its suggestion can result in ill-typed code when the type parameter it suggests providing a different generic argument for appears elsewhere in the callee's signature or predicates. --- .../src/diagnostics/conflict_errors.rs | 79 ++++++++++--------- .../suggest-borrow-for-generic-arg-aux.rs | 23 ++++++ .../suggest-borrow-for-generic-arg.fixed | 22 ++++++ .../moves/suggest-borrow-for-generic-arg.rs | 22 ++++++ .../suggest-borrow-for-generic-arg.stderr | 63 +++++++++++++++ 5 files changed, 170 insertions(+), 39 deletions(-) create mode 100644 tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs create mode 100644 tests/ui/moves/suggest-borrow-for-generic-arg.fixed create mode 100644 tests/ui/moves/suggest-borrow-for-generic-arg.rs create mode 100644 tests/ui/moves/suggest-borrow-for-generic-arg.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index e5b28289faa4e..ede1e815f4df0 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -440,47 +440,46 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } else { (None, &[][..], 0) }; + let ty = place.ty(self.body, self.infcx.tcx).ty; - // If the moved value is a mut reference, it is used in a - // generic function and it's type is a generic param, it can be - // reborrowed to avoid moving. - // for example: - // struct Y(u32); - // x's type is '& mut Y' and it is used in `fn generic(x: T) {}`. + let mut can_suggest_clone = true; if let Some(def_id) = def_id - && self.infcx.tcx.def_kind(def_id).is_fn_like() && let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id) - && let Some(arg) = self - .infcx - .tcx - .fn_sig(def_id) - .skip_binder() - .skip_binder() - .inputs() - .get(pos + offset) - && let ty::Param(_) = arg.kind() { - let place = &self.move_data.move_paths[mpi].place; - let ty = place.ty(self.body, self.infcx.tcx).ty; - if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() { + // The move occurred as one of the arguments to a function call. Is that + // argument generic? `def_id` can't be a closure here, so using `fn_sig` is fine + let arg_param = if self.infcx.tcx.def_kind(def_id).is_fn_like() + && let Some(arg_ty) = self + .infcx + .tcx + .fn_sig(def_id) + .instantiate_identity() + .skip_binder() + .inputs() + .get(pos + offset) + && let ty::Param(arg_param) = arg_ty.kind() + { + Some(arg_param) + } else { + None + }; + + // If the moved value is a mut reference, it is used in a + // generic function and it's type is a generic param, it can be + // reborrowed to avoid moving. + // for example: + // struct Y(u32); + // x's type is '& mut Y' and it is used in `fn generic(x: T) {}`. + if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() + && arg_param.is_some() + { *has_suggest_reborrow = true; self.suggest_reborrow(err, expr.span, moved_place); return; } - } - let mut can_suggest_clone = true; - if let Some(def_id) = def_id - && let Some(local_def_id) = def_id.as_local() - && let node = self.infcx.tcx.hir_node_by_def_id(local_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 + offset) - { let mut is_mut = false; - if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = arg.kind - && let Res::Def(DefKind::TyParam, param_def_id) = path.res + if let Some(param) = arg_param && self .infcx .tcx @@ -497,10 +496,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { self.infcx.tcx.get_diagnostic_item(sym::BorrowMut), ] .contains(&Some(predicate.def_id())) - && let ty::Param(param) = predicate.self_ty().kind() - && let generics = self.infcx.tcx.generics_of(def_id) - && let param = generics.type_param(*param, self.infcx.tcx) - && param.def_id == param_def_id + && *predicate.self_ty().kind() == ty::Param(*param) { if [ self.infcx.tcx.get_diagnostic_item(sym::AsMut), @@ -522,10 +518,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { expr.span.shrink_to_lo(), "borrow the value to avoid moving it", format!("&{}", if is_mut { "mut " } else { "" }), - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); can_suggest_clone = is_mut; - } else { + } else if let Some(local_def_id) = def_id.as_local() + && let node = self.infcx.tcx.hir_node_by_def_id(local_def_id) + && let Some(fn_decl) = node.fn_decl() + && let Some(ident) = node.ident() + && let Some(arg) = fn_decl.inputs.get(pos + offset) + { + // If we can't suggest borrowing in the call, but the function definition + // is local, instead offer changing the function to borrow that argument. let mut span: MultiSpan = arg.span.into(); span.push_span_label( arg.span, @@ -546,8 +549,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ); } } - let place = &self.move_data.move_paths[mpi].place; - let ty = place.ty(self.body, self.infcx.tcx).ty; 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, _)) = diff --git a/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs b/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs new file mode 100644 index 0000000000000..62e8510cf4dea --- /dev/null +++ b/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs @@ -0,0 +1,23 @@ +//! auxiliary definitons for suggest-borrow-for-generic-arg.rs, to ensure the suggestion works on +//! functions defined in other crates. + +use std::borrow::{Borrow, BorrowMut}; +use std::convert::{AsMut, AsRef}; +pub struct Bar; + +impl AsRef for Bar { + fn as_ref(&self) -> &Bar { + self + } +} + +impl AsMut for Bar { + fn as_mut(&mut self) -> &mut Bar { + self + } +} + +pub fn foo>(_: T) {} +pub fn qux>(_: T) {} +pub fn bat>(_: T) {} +pub fn baz>(_: T) {} diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.fixed b/tests/ui/moves/suggest-borrow-for-generic-arg.fixed new file mode 100644 index 0000000000000..3f711924d1e9c --- /dev/null +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.fixed @@ -0,0 +1,22 @@ +//! Test suggetions to borrow generic arguments instead of moving when all predicates hold after +//! substituting in the reference type. +//@ run-rustfix +//@ aux-build:suggest-borrow-for-generic-arg-aux.rs + +#![allow(unused_mut)] +extern crate suggest_borrow_for_generic_arg_aux as aux; + +pub fn main() { + let bar = aux::Bar; + aux::foo(&bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value + let mut bar = aux::Bar; + aux::qux(&mut bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value + let bar = aux::Bar; + aux::bat(&bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value + let mut bar = aux::Bar; + aux::baz(&mut bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value +} diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.rs b/tests/ui/moves/suggest-borrow-for-generic-arg.rs new file mode 100644 index 0000000000000..eb6abcdf0d570 --- /dev/null +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.rs @@ -0,0 +1,22 @@ +//! Test suggetions to borrow generic arguments instead of moving when all predicates hold after +//! substituting in the reference type. +//@ run-rustfix +//@ aux-build:suggest-borrow-for-generic-arg-aux.rs + +#![allow(unused_mut)] +extern crate suggest_borrow_for_generic_arg_aux as aux; + +pub fn main() { + let bar = aux::Bar; + aux::foo(bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value + let mut bar = aux::Bar; + aux::qux(bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value + let bar = aux::Bar; + aux::bat(bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value + let mut bar = aux::Bar; + aux::baz(bar); //~ HELP borrow the value + let _baa = bar; //~ ERROR use of moved value +} diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.stderr b/tests/ui/moves/suggest-borrow-for-generic-arg.stderr new file mode 100644 index 0000000000000..79602c4af4aa4 --- /dev/null +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.stderr @@ -0,0 +1,63 @@ +error[E0382]: use of moved value: `bar` + --> $DIR/suggest-borrow-for-generic-arg.rs:12:16 + | +LL | let bar = aux::Bar; + | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | aux::foo(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +help: borrow the value to avoid moving it + | +LL | aux::foo(&bar); + | + + +error[E0382]: use of moved value: `bar` + --> $DIR/suggest-borrow-for-generic-arg.rs:15:16 + | +LL | let mut bar = aux::Bar; + | ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | aux::qux(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +help: borrow the value to avoid moving it + | +LL | aux::qux(&mut bar); + | ++++ + +error[E0382]: use of moved value: `bar` + --> $DIR/suggest-borrow-for-generic-arg.rs:18:16 + | +LL | let bar = aux::Bar; + | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | aux::bat(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +help: borrow the value to avoid moving it + | +LL | aux::bat(&bar); + | + + +error[E0382]: use of moved value: `bar` + --> $DIR/suggest-borrow-for-generic-arg.rs:21:16 + | +LL | let mut bar = aux::Bar; + | ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | aux::baz(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +help: borrow the value to avoid moving it + | +LL | aux::baz(&mut bar); + | ++++ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0382`. From ea37000b565021086c560f3fd881508bfed6f531 Mon Sep 17 00:00:00 2001 From: dianne Date: Wed, 23 Oct 2024 10:31:58 -0700 Subject: [PATCH 2/3] Use a common subdiagnostic format for generic borrows This is setup for unifing the logic for suggestions to borrow arguments in generic positions. As of this commit, it's still special cases for `AsRef`/`Borrow`-like traits and `Fn`-like traits. --- .../src/diagnostics/conflict_errors.rs | 15 ++++++++++----- tests/ui/moves/moved-value-on-as-ref-arg.stderr | 8 ++++---- .../ui/moves/suggest-borrow-for-generic-arg.fixed | 8 ++++---- tests/ui/moves/suggest-borrow-for-generic-arg.rs | 8 ++++---- .../moves/suggest-borrow-for-generic-arg.stderr | 8 ++++---- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index ede1e815f4df0..5ffd635f1ccda 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -478,7 +478,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { return; } - let mut is_mut = false; + let mut mutbl = ty::Mutability::Not; if let Some(param) = arg_param && self .infcx @@ -504,7 +504,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ] .contains(&Some(predicate.def_id())) { - is_mut = true; + mutbl = ty::Mutability::Mut; } true } else { @@ -514,13 +514,18 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { { // The type of the argument corresponding to the expression that got moved // is a type parameter `T`, which is has a `T: AsRef` obligation. + let place_desc = if let Some(desc) = self.describe_place(moved_place) { + format!("`{desc}`") + } else { + "here".to_owned() + }; err.span_suggestion_verbose( expr.span.shrink_to_lo(), - "borrow the value to avoid moving it", - format!("&{}", if is_mut { "mut " } else { "" }), + format!("consider {}borrowing {place_desc}", mutbl.mutably_str()), + mutbl.ref_prefix_str(), Applicability::MaybeIncorrect, ); - can_suggest_clone = is_mut; + can_suggest_clone = mutbl.is_mut(); } else if let Some(local_def_id) = def_id.as_local() && let node = self.infcx.tcx.hir_node_by_def_id(local_def_id) && let Some(fn_decl) = node.fn_decl() diff --git a/tests/ui/moves/moved-value-on-as-ref-arg.stderr b/tests/ui/moves/moved-value-on-as-ref-arg.stderr index 4004b7a43bc09..a99bdb4fe9d41 100644 --- a/tests/ui/moves/moved-value-on-as-ref-arg.stderr +++ b/tests/ui/moves/moved-value-on-as-ref-arg.stderr @@ -8,7 +8,7 @@ LL | foo(bar); LL | let _baa = bar; | ^^^ value used here after move | -help: borrow the value to avoid moving it +help: consider borrowing `bar` | LL | foo(&bar); | + @@ -31,7 +31,7 @@ LL | struct Bar; ... LL | qux(bar); | --- you could clone this value -help: borrow the value to avoid moving it +help: consider mutably borrowing `bar` | LL | qux(&mut bar); | ++++ @@ -46,7 +46,7 @@ LL | bat(bar); LL | let _baa = bar; | ^^^ value used here after move | -help: borrow the value to avoid moving it +help: consider borrowing `bar` | LL | bat(&bar); | + @@ -69,7 +69,7 @@ LL | struct Bar; ... LL | baz(bar); | --- you could clone this value -help: borrow the value to avoid moving it +help: consider mutably borrowing `bar` | LL | baz(&mut bar); | ++++ diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.fixed b/tests/ui/moves/suggest-borrow-for-generic-arg.fixed index 3f711924d1e9c..372a469dcd63d 100644 --- a/tests/ui/moves/suggest-borrow-for-generic-arg.fixed +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.fixed @@ -8,15 +8,15 @@ extern crate suggest_borrow_for_generic_arg_aux as aux; pub fn main() { let bar = aux::Bar; - aux::foo(&bar); //~ HELP borrow the value + aux::foo(&bar); //~ HELP consider borrowing `bar` let _baa = bar; //~ ERROR use of moved value let mut bar = aux::Bar; - aux::qux(&mut bar); //~ HELP borrow the value + aux::qux(&mut bar); //~ HELP consider mutably borrowing `bar` let _baa = bar; //~ ERROR use of moved value let bar = aux::Bar; - aux::bat(&bar); //~ HELP borrow the value + aux::bat(&bar); //~ HELP consider borrowing `bar` let _baa = bar; //~ ERROR use of moved value let mut bar = aux::Bar; - aux::baz(&mut bar); //~ HELP borrow the value + aux::baz(&mut bar); //~ HELP consider mutably borrowing `bar` let _baa = bar; //~ ERROR use of moved value } diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.rs b/tests/ui/moves/suggest-borrow-for-generic-arg.rs index eb6abcdf0d570..c8b421fc03c52 100644 --- a/tests/ui/moves/suggest-borrow-for-generic-arg.rs +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.rs @@ -8,15 +8,15 @@ extern crate suggest_borrow_for_generic_arg_aux as aux; pub fn main() { let bar = aux::Bar; - aux::foo(bar); //~ HELP borrow the value + aux::foo(bar); //~ HELP consider borrowing `bar` let _baa = bar; //~ ERROR use of moved value let mut bar = aux::Bar; - aux::qux(bar); //~ HELP borrow the value + aux::qux(bar); //~ HELP consider mutably borrowing `bar` let _baa = bar; //~ ERROR use of moved value let bar = aux::Bar; - aux::bat(bar); //~ HELP borrow the value + aux::bat(bar); //~ HELP consider borrowing `bar` let _baa = bar; //~ ERROR use of moved value let mut bar = aux::Bar; - aux::baz(bar); //~ HELP borrow the value + aux::baz(bar); //~ HELP consider mutably borrowing `bar` let _baa = bar; //~ ERROR use of moved value } diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.stderr b/tests/ui/moves/suggest-borrow-for-generic-arg.stderr index 79602c4af4aa4..1a5106e6cd9f2 100644 --- a/tests/ui/moves/suggest-borrow-for-generic-arg.stderr +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.stderr @@ -8,7 +8,7 @@ LL | aux::foo(bar); LL | let _baa = bar; | ^^^ value used here after move | -help: borrow the value to avoid moving it +help: consider borrowing `bar` | LL | aux::foo(&bar); | + @@ -23,7 +23,7 @@ LL | aux::qux(bar); LL | let _baa = bar; | ^^^ value used here after move | -help: borrow the value to avoid moving it +help: consider mutably borrowing `bar` | LL | aux::qux(&mut bar); | ++++ @@ -38,7 +38,7 @@ LL | aux::bat(bar); LL | let _baa = bar; | ^^^ value used here after move | -help: borrow the value to avoid moving it +help: consider borrowing `bar` | LL | aux::bat(&bar); | + @@ -53,7 +53,7 @@ LL | aux::baz(bar); LL | let _baa = bar; | ^^^ value used here after move | -help: borrow the value to avoid moving it +help: consider mutably borrowing `bar` | LL | aux::baz(&mut bar); | ++++ From 2ab848060527f50a02cda91b9b8d98df6eb7ceab Mon Sep 17 00:00:00 2001 From: dianne Date: Wed, 23 Oct 2024 01:32:12 -0700 Subject: [PATCH 3/3] Suggest borrowing arguments in generic positions when trait bounds are satisfied This subsumes the suggestions to borrow arguments with `AsRef`/`Borrow` bounds and those to borrow arguments with `Fn` and `FnMut` bounds. It works for other traits implemented on references as well, such as `std::io::Read`, `std::io::Write`, and `core::fmt::Write`. Incidentally, by making the logic for suggesting borrowing closures general, this removes some spurious suggestions to mutably borrow `FnMut` closures in assignments, as well as an unhelpful suggestion to add a `Clone` constraint to an `impl Fn` argument. --- .../src/diagnostics/conflict_errors.rs | 316 +++++++++--------- ...re-origin-multi-variant-diagnostics.stderr | 4 - ...e-origin-single-variant-diagnostics.stderr | 4 - .../closure-origin-struct-diagnostics.stderr | 4 - .../closure-origin-tuple-diagnostics-1.stderr | 4 - .../suggest-borrow-for-generic-arg-aux.rs | 27 +- .../moves/borrow-closures-instead-of-move.rs | 2 +- .../borrow-closures-instead-of-move.stderr | 22 -- ...-on-type-no-recursive-stack-closure.stderr | 4 - .../suggest-borrow-for-generic-arg.fixed | 58 +++- .../moves/suggest-borrow-for-generic-arg.rs | 58 +++- .../suggest-borrow-for-generic-arg.stderr | 120 ++++--- tests/ui/not-copy-closure.stderr | 4 - 13 files changed, 327 insertions(+), 300 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 5ffd635f1ccda..454fd14ea7480 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -27,7 +27,7 @@ use rustc_middle::mir::{ }; use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{ - self, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast, + self, ClauseKind, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast, suggest_constraining_type_params, }; use rustc_middle::util::CallKind; @@ -39,6 +39,7 @@ use rustc_span::{BytePos, Span, Symbol}; use rustc_trait_selection::error_reporting::InferCtxtErrorExt; use rustc_trait_selection::error_reporting::traits::FindExprBySpan; use rustc_trait_selection::infer::InferCtxtExt; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; use rustc_trait_selection::traits::{Obligation, ObligationCause, ObligationCtxt}; use tracing::{debug, instrument}; @@ -201,16 +202,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { let mut has_suggest_reborrow = false; if !seen_spans.contains(&move_span) { - if !closure { - self.suggest_ref_or_clone( - mpi, - &mut err, - &mut in_pattern, - move_spans, - moved_place.as_ref(), - &mut has_suggest_reborrow, - ); - } + self.suggest_ref_or_clone( + mpi, + &mut err, + &mut in_pattern, + move_spans, + moved_place.as_ref(), + &mut has_suggest_reborrow, + closure, + ); let msg_opt = CapturedMessageOpt { is_partial_move, @@ -266,27 +266,11 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } } - let opt_name = self.describe_place_with_options(place.as_ref(), DescribePlaceOpt { - including_downcast: true, - including_tuple_field: true, - }); - let note_msg = match opt_name { - Some(name) => format!("`{name}`"), - None => "value".to_owned(), - }; - if self.suggest_borrow_fn_like(&mut err, ty, &move_site_vec, ¬e_msg) - || if let UseSpans::FnSelfUse { kind, .. } = use_spans - && let CallKind::FnCall { fn_trait_id, self_ty } = kind - && let ty::Param(_) = self_ty.kind() - && ty == self_ty - && self.infcx.tcx.is_lang_item(fn_trait_id, LangItem::FnOnce) - { - // this is a type parameter `T: FnOnce()`, don't suggest `T: FnOnce() + Clone`. - true - } else { - false - } - { + if self.param_env.caller_bounds().iter().any(|c| { + c.as_trait_clause().is_some_and(|pred| { + pred.skip_binder().self_ty() == ty && self.infcx.tcx.is_fn_trait(pred.def_id()) + }) + }) { // Suppress the next suggestion since we don't want to put more bounds onto // something that already has `Fn`-like bounds (or is a closure), so we can't // restrict anyways. @@ -295,6 +279,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { self.suggest_adding_bounds(&mut err, ty, copy_did, span); } + let opt_name = self.describe_place_with_options(place.as_ref(), DescribePlaceOpt { + including_downcast: true, + including_tuple_field: true, + }); + let note_msg = match opt_name { + Some(name) => format!("`{name}`"), + None => "value".to_owned(), + }; if needs_note { if let Some(local) = place.as_local() { let span = self.body.local_decls[local].source_info.span; @@ -341,6 +333,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { move_spans: UseSpans<'tcx>, moved_place: PlaceRef<'tcx>, has_suggest_reborrow: &mut bool, + moved_or_invoked_closure: bool, ) { let move_span = match move_spans { UseSpans::ClosureUse { capture_kind_span, .. } => capture_kind_span, @@ -428,17 +421,18 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } let typeck = self.infcx.tcx.typeck(self.mir_def_id()); let parent = self.infcx.tcx.parent_hir_node(expr.hir_id); - let (def_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent + let (def_id, call_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent && let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind { - (typeck.type_dependent_def_id(parent_expr.hir_id), args, 1) + let def_id = typeck.type_dependent_def_id(parent_expr.hir_id); + (def_id, Some(parent_expr.hir_id), args, 1) } else 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() { - (Some(*def_id), args, 0) + (Some(*def_id), Some(call.hir_id), args, 0) } else { - (None, &[][..], 0) + (None, None, &[][..], 0) }; let ty = place.ty(self.body, self.infcx.tcx).ty; @@ -449,14 +443,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // The move occurred as one of the arguments to a function call. Is that // argument generic? `def_id` can't be a closure here, so using `fn_sig` is fine let arg_param = if self.infcx.tcx.def_kind(def_id).is_fn_like() - && let Some(arg_ty) = self - .infcx - .tcx - .fn_sig(def_id) - .instantiate_identity() - .skip_binder() - .inputs() - .get(pos + offset) + && let sig = + self.infcx.tcx.fn_sig(def_id).instantiate_identity().skip_binder() + && let Some(arg_ty) = sig.inputs().get(pos + offset) && let ty::Param(arg_param) = arg_ty.kind() { Some(arg_param) @@ -478,54 +467,22 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { return; } - let mut mutbl = ty::Mutability::Not; - if let Some(param) = arg_param - && self - .infcx - .tcx - .predicates_of(def_id) - .instantiate_identity(self.infcx.tcx) - .predicates - .into_iter() - .any(|pred| { - if let ty::ClauseKind::Trait(predicate) = pred.kind().skip_binder() - && [ - self.infcx.tcx.get_diagnostic_item(sym::AsRef), - self.infcx.tcx.get_diagnostic_item(sym::AsMut), - self.infcx.tcx.get_diagnostic_item(sym::Borrow), - self.infcx.tcx.get_diagnostic_item(sym::BorrowMut), - ] - .contains(&Some(predicate.def_id())) - && *predicate.self_ty().kind() == ty::Param(*param) - { - if [ - self.infcx.tcx.get_diagnostic_item(sym::AsMut), - self.infcx.tcx.get_diagnostic_item(sym::BorrowMut), - ] - .contains(&Some(predicate.def_id())) - { - mutbl = ty::Mutability::Mut; - } - true - } else { - false - } - }) + // If the moved place is used generically by the callee and a reference to it + // would still satisfy any bounds on its type, suggest borrowing. + if let Some(¶m) = arg_param + && let Some(generic_args) = call_id.and_then(|id| typeck.node_args_opt(id)) + && let Some(ref_mutability) = self.suggest_borrow_generic_arg( + err, + def_id, + generic_args, + param, + moved_place, + pos + offset, + ty, + expr.span, + ) { - // The type of the argument corresponding to the expression that got moved - // is a type parameter `T`, which is has a `T: AsRef` obligation. - let place_desc = if let Some(desc) = self.describe_place(moved_place) { - format!("`{desc}`") - } else { - "here".to_owned() - }; - err.span_suggestion_verbose( - expr.span.shrink_to_lo(), - format!("consider {}borrowing {place_desc}", mutbl.mutably_str()), - mutbl.ref_prefix_str(), - Applicability::MaybeIncorrect, - ); - can_suggest_clone = mutbl.is_mut(); + can_suggest_clone = ref_mutability.is_mut(); } else if let Some(local_def_id) = def_id.as_local() && let node = self.infcx.tcx.hir_node_by_def_id(local_def_id) && let Some(fn_decl) = node.fn_decl() @@ -563,6 +520,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } else if let UseSpans::FnSelfUse { kind: CallKind::Normal { .. }, .. } = move_spans { // We already suggest cloning for these cases in `explain_captures`. + } else if moved_or_invoked_closure { + // Do not suggest `closure.clone()()`. } else if let UseSpans::ClosureUse { closure_kind: ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)), @@ -671,6 +630,113 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ); } + /// If a place is used after being moved as an argument to a function, the function is generic + /// in that argument, and a reference to the argument's type would still satisfy the function's + /// bounds, suggest borrowing. This covers, e.g., borrowing an `impl Fn()` argument being passed + /// in an `impl FnOnce()` position. + /// Returns `Some(mutability)` when suggesting to borrow with mutability `mutability`, or `None` + /// if no suggestion is made. + fn suggest_borrow_generic_arg( + &self, + err: &mut Diag<'_>, + callee_did: DefId, + generic_args: ty::GenericArgsRef<'tcx>, + param: ty::ParamTy, + moved_place: PlaceRef<'tcx>, + moved_arg_pos: usize, + moved_arg_ty: Ty<'tcx>, + place_span: Span, + ) -> Option { + let tcx = self.infcx.tcx; + let sig = tcx.fn_sig(callee_did).instantiate_identity().skip_binder(); + let clauses = tcx.predicates_of(callee_did).instantiate_identity(self.infcx.tcx).predicates; + + // First, is there at least one method on one of `param`'s trait bounds? + // This keeps us from suggesting borrowing the argument to `mem::drop`, e.g. + if !clauses.iter().any(|clause| { + clause.as_trait_clause().is_some_and(|tc| { + tc.self_ty().skip_binder().is_param(param.index) + && tc.polarity() == ty::PredicatePolarity::Positive + && tcx + .supertrait_def_ids(tc.def_id()) + .flat_map(|trait_did| tcx.associated_items(trait_did).in_definition_order()) + .any(|item| item.fn_has_self_parameter) + }) + }) { + return None; + } + + // Try borrowing a shared reference first, then mutably. + if let Some(mutbl) = [ty::Mutability::Not, ty::Mutability::Mut].into_iter().find(|&mutbl| { + let re = self.infcx.tcx.lifetimes.re_erased; + let ref_ty = Ty::new_ref(self.infcx.tcx, re, moved_arg_ty, mutbl); + + // Ensure that substituting `ref_ty` in the callee's signature doesn't break + // other inputs or the return type. + let new_args = tcx.mk_args_from_iter(generic_args.iter().enumerate().map( + |(i, arg)| { + if i == param.index as usize { ref_ty.into() } else { arg } + }, + )); + let can_subst = |ty: Ty<'tcx>| { + // Normalize before comparing to see through type aliases and projections. + let old_ty = ty::EarlyBinder::bind(ty).instantiate(tcx, generic_args); + let new_ty = ty::EarlyBinder::bind(ty).instantiate(tcx, new_args); + if let Ok(old_ty) = tcx.try_normalize_erasing_regions(self.param_env, old_ty) + && let Ok(new_ty) = tcx.try_normalize_erasing_regions(self.param_env, new_ty) + { + old_ty == new_ty + } else { + false + } + }; + if !can_subst(sig.output()) + || sig + .inputs() + .iter() + .enumerate() + .any(|(i, &input_ty)| i != moved_arg_pos && !can_subst(input_ty)) + { + return false; + } + + // Test the callee's predicates, substituting a reference in for the self ty + // in bounds on `param`. + clauses.iter().all(|&clause| { + let clause_for_ref = clause.kind().map_bound(|kind| match kind { + ClauseKind::Trait(c) if c.self_ty().is_param(param.index) => { + ClauseKind::Trait(c.with_self_ty(tcx, ref_ty)) + } + ClauseKind::Projection(c) if c.self_ty().is_param(param.index) => { + ClauseKind::Projection(c.with_self_ty(tcx, ref_ty)) + } + _ => kind, + }); + self.infcx.predicate_must_hold_modulo_regions(&Obligation::new( + tcx, + ObligationCause::dummy(), + self.param_env, + ty::EarlyBinder::bind(clause_for_ref).instantiate(tcx, generic_args), + )) + }) + }) { + let place_desc = if let Some(desc) = self.describe_place(moved_place) { + format!("`{desc}`") + } else { + "here".to_owned() + }; + err.span_suggestion_verbose( + place_span.shrink_to_lo(), + format!("consider {}borrowing {place_desc}", mutbl.mutably_str()), + mutbl.ref_prefix_str(), + Applicability::MaybeIncorrect, + ); + Some(mutbl) + } else { + None + } + } + fn report_use_of_uninitialized( &self, mpi: MovePathIndex, @@ -851,74 +917,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ); } - fn suggest_borrow_fn_like( - &self, - err: &mut Diag<'_>, - ty: Ty<'tcx>, - move_sites: &[MoveSite], - value_name: &str, - ) -> bool { - let tcx = self.infcx.tcx; - - // Find out if the predicates show that the type is a Fn or FnMut - let find_fn_kind_from_did = |(pred, _): (ty::Clause<'tcx>, _)| { - if let ty::ClauseKind::Trait(pred) = pred.kind().skip_binder() - && pred.self_ty() == ty - { - if tcx.is_lang_item(pred.def_id(), LangItem::Fn) { - return Some(hir::Mutability::Not); - } else if tcx.is_lang_item(pred.def_id(), LangItem::FnMut) { - return Some(hir::Mutability::Mut); - } - } - None - }; - - // If the type is opaque/param/closure, and it is Fn or FnMut, let's suggest (mutably) - // borrowing the type, since `&mut F: FnMut` iff `F: FnMut` and similarly for `Fn`. - // These types seem reasonably opaque enough that they could be instantiated with their - // borrowed variants in a function body when we see a move error. - let borrow_level = match *ty.kind() { - ty::Param(_) => tcx - .explicit_predicates_of(self.mir_def_id().to_def_id()) - .predicates - .iter() - .copied() - .find_map(find_fn_kind_from_did), - ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => tcx - .explicit_item_super_predicates(def_id) - .iter_instantiated_copied(tcx, args) - .find_map(|(clause, span)| find_fn_kind_from_did((clause, span))), - ty::Closure(_, args) => match args.as_closure().kind() { - ty::ClosureKind::Fn => Some(hir::Mutability::Not), - ty::ClosureKind::FnMut => Some(hir::Mutability::Mut), - _ => None, - }, - _ => None, - }; - - let Some(borrow_level) = borrow_level else { - return false; - }; - let sugg = move_sites - .iter() - .map(|move_site| { - let move_out = self.move_data.moves[(*move_site).moi]; - let moved_place = &self.move_data.move_paths[move_out.path].place; - let move_spans = self.move_spans(moved_place.as_ref(), move_out.source); - let move_span = move_spans.args_or_use(); - let suggestion = borrow_level.ref_prefix_str().to_owned(); - (move_span.shrink_to_lo(), suggestion) - }) - .collect(); - err.multipart_suggestion_verbose( - format!("consider {}borrowing {value_name}", borrow_level.mutably_str()), - sugg, - Applicability::MaybeIncorrect, - ); - true - } - /// In a move error that occurs on a call within a loop, we try to identify cases where cloning /// the value would lead to a logic error. We infer these cases by seeing if the moved value is /// part of the logic to break the loop, either through an explicit `break` or if the expression diff --git a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr index 347fa3fa8920d..4cfe51eccac09 100644 --- a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr +++ b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr @@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | if let MultiVariant::Point(ref mut x, _) = point { | ^^^^^ -help: consider mutably borrowing `c` - | -LL | let a = &mut c; - | ++++ error: aborting due to 1 previous error diff --git a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr index c9b27e7687959..2ed611d6b5238 100644 --- a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr +++ b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr @@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | let SingleVariant::Point(ref mut x, _) = point; | ^^^^^ -help: consider mutably borrowing `c` - | -LL | let b = &mut c; - | ++++ error: aborting due to 1 previous error diff --git a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr index 079a9abedf976..47db2c76a2f1d 100644 --- a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr +++ b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr @@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | x.y.a += 1; | ^^^^^ -help: consider mutably borrowing `hello` - | -LL | let b = &mut hello; - | ++++ error: aborting due to 1 previous error diff --git a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr index 0bf717404ceb7..9db64ec04b9db 100644 --- a/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr +++ b/tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr @@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | x.0 += 1; | ^^^ -help: consider mutably borrowing `hello` - | -LL | let b = &mut hello; - | ++++ error: aborting due to 1 previous error diff --git a/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs b/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs index 62e8510cf4dea..c71238ba07296 100644 --- a/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs +++ b/tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs @@ -1,23 +1,20 @@ //! auxiliary definitons for suggest-borrow-for-generic-arg.rs, to ensure the suggestion works on //! functions defined in other crates. -use std::borrow::{Borrow, BorrowMut}; -use std::convert::{AsMut, AsRef}; -pub struct Bar; +use std::io::{self, Read, Write}; +use std::iter::Sum; -impl AsRef for Bar { - fn as_ref(&self) -> &Bar { - self - } +pub fn write_stuff(mut writer: W) -> io::Result<()> { + writeln!(writer, "stuff") } -impl AsMut for Bar { - fn as_mut(&mut self) -> &mut Bar { - self - } +pub fn read_and_discard(mut reader: R) -> io::Result<()> { + let mut buf = Vec::new(); + reader.read_to_end(&mut buf).map(|_| ()) } -pub fn foo>(_: T) {} -pub fn qux>(_: T) {} -pub fn bat>(_: T) {} -pub fn baz>(_: T) {} +pub fn sum_three(iter: I) -> ::Item + where ::Item: Sum +{ + iter.into_iter().take(3).sum() +} diff --git a/tests/ui/moves/borrow-closures-instead-of-move.rs b/tests/ui/moves/borrow-closures-instead-of-move.rs index e4bca54e995fa..869aa654ef727 100644 --- a/tests/ui/moves/borrow-closures-instead-of-move.rs +++ b/tests/ui/moves/borrow-closures-instead-of-move.rs @@ -1,4 +1,4 @@ -fn takes_fn(f: impl Fn()) { //~ HELP if `impl Fn()` implemented `Clone` +fn takes_fn(f: impl Fn()) { loop { takes_fnonce(f); //~^ ERROR use of moved value diff --git a/tests/ui/moves/borrow-closures-instead-of-move.stderr b/tests/ui/moves/borrow-closures-instead-of-move.stderr index ab6ff417efb60..ea145f365c2da 100644 --- a/tests/ui/moves/borrow-closures-instead-of-move.stderr +++ b/tests/ui/moves/borrow-closures-instead-of-move.stderr @@ -8,21 +8,6 @@ LL | loop { LL | takes_fnonce(f); | ^ value moved here, in previous iteration of loop | -note: consider changing this parameter type in function `takes_fnonce` to borrow instead if owning the value isn't necessary - --> $DIR/borrow-closures-instead-of-move.rs:34:20 - | -LL | fn takes_fnonce(_: impl FnOnce()) {} - | ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value - | | - | in this function -help: if `impl Fn()` implemented `Clone`, you could clone the value - --> $DIR/borrow-closures-instead-of-move.rs:1:16 - | -LL | fn takes_fn(f: impl Fn()) { - | ^^^^^^^^^ consider constraining this type parameter with `Clone` -LL | loop { -LL | takes_fnonce(f); - | - you could clone this value help: consider borrowing `f` | LL | takes_fnonce(&f); @@ -40,13 +25,6 @@ LL | takes_fnonce(m); LL | takes_fnonce(m); | ^ value used here after move | -note: consider changing this parameter type in function `takes_fnonce` to borrow instead if owning the value isn't necessary - --> $DIR/borrow-closures-instead-of-move.rs:34:20 - | -LL | fn takes_fnonce(_: impl FnOnce()) {} - | ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value - | | - | in this function help: if `impl FnMut()` implemented `Clone`, you could clone the value --> $DIR/borrow-closures-instead-of-move.rs:9:20 | diff --git a/tests/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr b/tests/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr index a8473bb81983d..a4c8401ce5756 100644 --- a/tests/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr +++ b/tests/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr @@ -24,10 +24,6 @@ LL | fn conspirator(mut f: F) where F: FnMut(&mut R, bool) { | ^ consider constraining this type parameter with `Clone` LL | let mut r = R {c: Box::new(f)}; | - you could clone this value -help: consider mutably borrowing `f` - | -LL | let mut r = R {c: Box::new(&mut f)}; - | ++++ error: aborting due to 2 previous errors diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.fixed b/tests/ui/moves/suggest-borrow-for-generic-arg.fixed index 372a469dcd63d..b5e0b468aa60a 100644 --- a/tests/ui/moves/suggest-borrow-for-generic-arg.fixed +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.fixed @@ -1,22 +1,46 @@ -//! Test suggetions to borrow generic arguments instead of moving when all predicates hold after -//! substituting in the reference type. +//! Test suggetions to borrow generic arguments instead of moving. Tests for other instances of this +//! can be found in `moved-value-on-as-ref-arg.rs` and `borrow-closures-instead-of-move.rs` //@ run-rustfix -//@ aux-build:suggest-borrow-for-generic-arg-aux.rs +//@ aux-crate:aux=suggest-borrow-for-generic-arg-aux.rs +//@ edition: 2021 #![allow(unused_mut)] -extern crate suggest_borrow_for_generic_arg_aux as aux; +use std::io::{self, Write}; -pub fn main() { - let bar = aux::Bar; - aux::foo(&bar); //~ HELP consider borrowing `bar` - let _baa = bar; //~ ERROR use of moved value - let mut bar = aux::Bar; - aux::qux(&mut bar); //~ HELP consider mutably borrowing `bar` - let _baa = bar; //~ ERROR use of moved value - let bar = aux::Bar; - aux::bat(&bar); //~ HELP consider borrowing `bar` - let _baa = bar; //~ ERROR use of moved value - let mut bar = aux::Bar; - aux::baz(&mut bar); //~ HELP consider mutably borrowing `bar` - let _baa = bar; //~ ERROR use of moved value +// test for `std::io::Write` (#131413) +fn test_write() -> io::Result<()> { + let mut stdout = io::stdout(); + aux::write_stuff(&stdout)?; //~ HELP consider borrowing `stdout` + writeln!(stdout, "second line")?; //~ ERROR borrow of moved value: `stdout` + + let mut buf = Vec::new(); + aux::write_stuff(&mut buf.clone())?; //~ HELP consider mutably borrowing `buf` + //~^ HELP consider cloning the value + writeln!(buf, "second_line") //~ ERROR borrow of moved value: `buf` +} + +/// test for `std::io::Read` (#131413) +fn test_read() -> io::Result<()> { + let stdin = io::stdin(); + aux::read_and_discard(&stdin)?; //~ HELP consider borrowing `stdin` + aux::read_and_discard(stdin)?; //~ ERROR use of moved value: `stdin` + + let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]); + aux::read_and_discard(&mut bytes.clone())?; //~ HELP consider mutably borrowing `bytes` + //~^ HELP consider cloning the value + aux::read_and_discard(bytes) //~ ERROR use of moved value: `bytes` +} + +/// test that suggestions work with projection types in the callee's signature +fn test_projections() { + let mut iter = [1, 2, 3, 4, 5, 6].into_iter(); + let _six: usize = aux::sum_three(&mut iter.clone()); //~ HELP consider mutably borrowing `iter` + //~^ HELP consider cloning the value + let _fifteen: usize = aux::sum_three(iter); //~ ERROR use of moved value: `iter` +} + +fn main() { + test_write().unwrap(); + test_read().unwrap(); + test_projections(); } diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.rs b/tests/ui/moves/suggest-borrow-for-generic-arg.rs index c8b421fc03c52..e08978db63aeb 100644 --- a/tests/ui/moves/suggest-borrow-for-generic-arg.rs +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.rs @@ -1,22 +1,46 @@ -//! Test suggetions to borrow generic arguments instead of moving when all predicates hold after -//! substituting in the reference type. +//! Test suggetions to borrow generic arguments instead of moving. Tests for other instances of this +//! can be found in `moved-value-on-as-ref-arg.rs` and `borrow-closures-instead-of-move.rs` //@ run-rustfix -//@ aux-build:suggest-borrow-for-generic-arg-aux.rs +//@ aux-crate:aux=suggest-borrow-for-generic-arg-aux.rs +//@ edition: 2021 #![allow(unused_mut)] -extern crate suggest_borrow_for_generic_arg_aux as aux; +use std::io::{self, Write}; -pub fn main() { - let bar = aux::Bar; - aux::foo(bar); //~ HELP consider borrowing `bar` - let _baa = bar; //~ ERROR use of moved value - let mut bar = aux::Bar; - aux::qux(bar); //~ HELP consider mutably borrowing `bar` - let _baa = bar; //~ ERROR use of moved value - let bar = aux::Bar; - aux::bat(bar); //~ HELP consider borrowing `bar` - let _baa = bar; //~ ERROR use of moved value - let mut bar = aux::Bar; - aux::baz(bar); //~ HELP consider mutably borrowing `bar` - let _baa = bar; //~ ERROR use of moved value +// test for `std::io::Write` (#131413) +fn test_write() -> io::Result<()> { + let mut stdout = io::stdout(); + aux::write_stuff(stdout)?; //~ HELP consider borrowing `stdout` + writeln!(stdout, "second line")?; //~ ERROR borrow of moved value: `stdout` + + let mut buf = Vec::new(); + aux::write_stuff(buf)?; //~ HELP consider mutably borrowing `buf` + //~^ HELP consider cloning the value + writeln!(buf, "second_line") //~ ERROR borrow of moved value: `buf` +} + +/// test for `std::io::Read` (#131413) +fn test_read() -> io::Result<()> { + let stdin = io::stdin(); + aux::read_and_discard(stdin)?; //~ HELP consider borrowing `stdin` + aux::read_and_discard(stdin)?; //~ ERROR use of moved value: `stdin` + + let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]); + aux::read_and_discard(bytes)?; //~ HELP consider mutably borrowing `bytes` + //~^ HELP consider cloning the value + aux::read_and_discard(bytes) //~ ERROR use of moved value: `bytes` +} + +/// test that suggestions work with projection types in the callee's signature +fn test_projections() { + let mut iter = [1, 2, 3, 4, 5, 6].into_iter(); + let _six: usize = aux::sum_three(iter); //~ HELP consider mutably borrowing `iter` + //~^ HELP consider cloning the value + let _fifteen: usize = aux::sum_three(iter); //~ ERROR use of moved value: `iter` +} + +fn main() { + test_write().unwrap(); + test_read().unwrap(); + test_projections(); } diff --git a/tests/ui/moves/suggest-borrow-for-generic-arg.stderr b/tests/ui/moves/suggest-borrow-for-generic-arg.stderr index 1a5106e6cd9f2..07e24f566cb9b 100644 --- a/tests/ui/moves/suggest-borrow-for-generic-arg.stderr +++ b/tests/ui/moves/suggest-borrow-for-generic-arg.stderr @@ -1,63 +1,93 @@ -error[E0382]: use of moved value: `bar` - --> $DIR/suggest-borrow-for-generic-arg.rs:12:16 +error[E0382]: borrow of moved value: `stdout` + --> $DIR/suggest-borrow-for-generic-arg.rs:14:14 | -LL | let bar = aux::Bar; - | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait -LL | aux::foo(bar); - | --- value moved here -LL | let _baa = bar; - | ^^^ value used here after move +LL | let mut stdout = io::stdout(); + | ---------- move occurs because `stdout` has type `Stdout`, which does not implement the `Copy` trait +LL | aux::write_stuff(stdout)?; + | ------ value moved here +LL | writeln!(stdout, "second line")?; + | ^^^^^^ value borrowed here after move | -help: consider borrowing `bar` +help: consider borrowing `stdout` | -LL | aux::foo(&bar); - | + +LL | aux::write_stuff(&stdout)?; + | + -error[E0382]: use of moved value: `bar` - --> $DIR/suggest-borrow-for-generic-arg.rs:15:16 +error[E0382]: borrow of moved value: `buf` + --> $DIR/suggest-borrow-for-generic-arg.rs:19:14 | -LL | let mut bar = aux::Bar; - | ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait -LL | aux::qux(bar); - | --- value moved here -LL | let _baa = bar; - | ^^^ value used here after move +LL | let mut buf = Vec::new(); + | ------- move occurs because `buf` has type `Vec`, which does not implement the `Copy` trait +LL | aux::write_stuff(buf)?; + | --- value moved here +LL | +LL | writeln!(buf, "second_line") + | ^^^ value borrowed here after move | -help: consider mutably borrowing `bar` +help: consider mutably borrowing `buf` | -LL | aux::qux(&mut bar); - | ++++ +LL | aux::write_stuff(&mut buf)?; + | ++++ +help: consider cloning the value if the performance cost is acceptable + | +LL | aux::write_stuff(buf.clone())?; + | ++++++++ -error[E0382]: use of moved value: `bar` - --> $DIR/suggest-borrow-for-generic-arg.rs:18:16 +error[E0382]: use of moved value: `stdin` + --> $DIR/suggest-borrow-for-generic-arg.rs:26:27 | -LL | let bar = aux::Bar; - | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait -LL | aux::bat(bar); - | --- value moved here -LL | let _baa = bar; - | ^^^ value used here after move +LL | let stdin = io::stdin(); + | ----- move occurs because `stdin` has type `Stdin`, which does not implement the `Copy` trait +LL | aux::read_and_discard(stdin)?; + | ----- value moved here +LL | aux::read_and_discard(stdin)?; + | ^^^^^ value used here after move | -help: consider borrowing `bar` +help: consider borrowing `stdin` | -LL | aux::bat(&bar); - | + +LL | aux::read_and_discard(&stdin)?; + | + -error[E0382]: use of moved value: `bar` - --> $DIR/suggest-borrow-for-generic-arg.rs:21:16 +error[E0382]: use of moved value: `bytes` + --> $DIR/suggest-borrow-for-generic-arg.rs:31:27 + | +LL | let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]); + | --------- move occurs because `bytes` has type `VecDeque`, which does not implement the `Copy` trait +LL | aux::read_and_discard(bytes)?; + | ----- value moved here +LL | +LL | aux::read_and_discard(bytes) + | ^^^^^ value used here after move + | +help: consider mutably borrowing `bytes` + | +LL | aux::read_and_discard(&mut bytes)?; + | ++++ +help: consider cloning the value if the performance cost is acceptable + | +LL | aux::read_and_discard(bytes.clone())?; + | ++++++++ + +error[E0382]: use of moved value: `iter` + --> $DIR/suggest-borrow-for-generic-arg.rs:39:42 + | +LL | let mut iter = [1, 2, 3, 4, 5, 6].into_iter(); + | -------- move occurs because `iter` has type `std::array::IntoIter`, which does not implement the `Copy` trait +LL | let _six: usize = aux::sum_three(iter); + | ---- value moved here +LL | +LL | let _fifteen: usize = aux::sum_three(iter); + | ^^^^ value used here after move | -LL | let mut bar = aux::Bar; - | ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait -LL | aux::baz(bar); - | --- value moved here -LL | let _baa = bar; - | ^^^ value used here after move +help: consider mutably borrowing `iter` | -help: consider mutably borrowing `bar` +LL | let _six: usize = aux::sum_three(&mut iter); + | ++++ +help: consider cloning the value if the performance cost is acceptable | -LL | aux::baz(&mut bar); - | ++++ +LL | let _six: usize = aux::sum_three(iter.clone()); + | ++++++++ -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0382`. diff --git a/tests/ui/not-copy-closure.stderr b/tests/ui/not-copy-closure.stderr index 50e25a24d811f..60cb135231306 100644 --- a/tests/ui/not-copy-closure.stderr +++ b/tests/ui/not-copy-closure.stderr @@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | a += 1; | ^ -help: consider mutably borrowing `hello` - | -LL | let b = &mut hello; - | ++++ error: aborting due to 1 previous error