From 2280d8ea0028bf163b1d9ac25514614823843140 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 20 Oct 2024 04:22:07 -0700 Subject: [PATCH] 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 e5b28289faa..ede1e815f4d 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 00000000000..62e8510cf4d --- /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 00000000000..3f711924d1e --- /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 00000000000..eb6abcdf0d5 --- /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 00000000000..79602c4af4a --- /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`.