From f4e6d28b0f7137118ef0c062180d6a0f87b99ddc Mon Sep 17 00:00:00 2001 From: dianne Date: Wed, 23 Oct 2024 01:32:12 -0700 Subject: [PATCH] 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 | 324 +++++++++--------- ...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, 336 insertions(+), 299 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index a00e2bf126056..b2049fb22af5c 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,21 @@ 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(), + let is_fn_like = match ty.kind() { + // a type parameter is fn-like if it has a fn-like trait bound + ty::Param(_) => 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()) + }) + }), + // an opaque type is fn-like if its trait is fn-like + ty::Alias(ty::Opaque, opaque) => self.infcx.tcx.is_fn_trait(opaque.def_id), + // closures are always fn-like + ty::Closure(_, _) => true, + _ => false, }; - 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 is_fn_like { // 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 +289,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 +343,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 +431,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 +453,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 +477,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 +530,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 +640,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 +927,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