From 2df4f7dd8c51e1c3e65b615d1c44fdc9d0b4b044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 2 May 2024 01:15:40 +0000 Subject: [PATCH] Suggest borrowing on fn argument that is `impl AsRef` When encountering a move conflict, on an expression that is `!Copy` passed as an argument to an `fn` that is `impl AsRef`, suggest borrowing the expression. ``` error[E0382]: use of moved value: `bar` --> f204.rs:14:15 | 12 | let bar = Bar; | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait 13 | foo(bar); | --- value moved here 14 | let baa = bar; | ^^^ value used here after move | help: borrow the value to avoid moving it | 13 | foo(&bar); | + ``` Fix #41708 --- .../src/diagnostics/conflict_errors.rs | 89 +++++++++++++++---- library/core/src/borrow.rs | 1 + .../ui/moves/moved-value-on-as-ref-arg.fixed | 37 ++++++++ tests/ui/moves/moved-value-on-as-ref-arg.rs | 37 ++++++++ .../ui/moves/moved-value-on-as-ref-arg.stderr | 79 ++++++++++++++++ 5 files changed, 224 insertions(+), 19 deletions(-) create mode 100644 tests/ui/moves/moved-value-on-as-ref-arg.fixed create mode 100644 tests/ui/moves/moved-value-on-as-ref-arg.rs create mode 100644 tests/ui/moves/moved-value-on-as-ref-arg.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 6f0bd0543293c..9efb3362e18ee 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -449,6 +449,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } else { (None, &[][..], 0) }; + let mut can_suggest_clone = true; if let Some(def_id) = def_id && let node = self.infcx.tcx.hir_node_by_def_id(def_id) && let Some(fn_sig) = node.fn_sig() @@ -456,24 +457,73 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { && let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id) && let Some(arg) = fn_sig.decl.inputs.get(pos + offset) { - let mut span: MultiSpan = arg.span.into(); - span.push_span_label( - arg.span, - "this parameter takes ownership of the value".to_string(), - ); - let descr = match node.fn_kind() { - Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function", - Some(hir::intravisit::FnKind::Method(..)) => "method", - Some(hir::intravisit::FnKind::Closure) => "closure", - }; - span.push_span_label(ident.span, format!("in this {descr}")); - err.span_note( - span, - format!( - "consider changing this parameter type in {descr} `{ident}` to borrow \ - instead if owning the value isn't necessary", - ), - ); + 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 + && 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())) + && 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 + { + if [ + self.infcx.tcx.get_diagnostic_item(sym::AsMut), + self.infcx.tcx.get_diagnostic_item(sym::BorrowMut), + ] + .contains(&Some(predicate.def_id())) + { + is_mut = true; + } + true + } else { + false + } + }) + { + // The type of the argument corresponding to the expression that got moved + // is a type parameter `T`, which is has a `T: AsRef` obligation. + err.span_suggestion_verbose( + expr.span.shrink_to_lo(), + "borrow the value to avoid moving it", + format!("&{}", if is_mut { "mut " } else { "" }), + Applicability::MachineApplicable, + ); + can_suggest_clone = is_mut; + } else { + let mut span: MultiSpan = arg.span.into(); + span.push_span_label( + arg.span, + "this parameter takes ownership of the value".to_string(), + ); + let descr = match node.fn_kind() { + Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function", + Some(hir::intravisit::FnKind::Method(..)) => "method", + Some(hir::intravisit::FnKind::Closure) => "closure", + }; + span.push_span_label(ident.span, format!("in this {descr}")); + err.span_note( + span, + format!( + "consider changing this parameter type in {descr} `{ident}` to \ + borrow instead if owning the value isn't necessary", + ), + ); + } } let place = &self.move_data.move_paths[mpi].place; let ty = place.ty(self.body, self.infcx.tcx).ty; @@ -491,9 +541,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)), .. } = move_spans + && can_suggest_clone { self.suggest_cloning(err, ty, expr, None, Some(move_spans)); - } else if self.suggest_hoisting_call_outside_loop(err, expr) { + } else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone { // The place where the type moves would be misleading to suggest clone. // #121466 self.suggest_cloning(err, ty, expr, None, Some(move_spans)); diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index bc026d0a44634..ccb1cc4e974d6 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -184,6 +184,7 @@ pub trait Borrow { /// an underlying type by providing a mutable reference. See [`Borrow`] /// for more information on borrowing as another type. #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "BorrowMut"] pub trait BorrowMut: Borrow { /// Mutably borrows from an owned value. /// diff --git a/tests/ui/moves/moved-value-on-as-ref-arg.fixed b/tests/ui/moves/moved-value-on-as-ref-arg.fixed new file mode 100644 index 0000000000000..292fa98a3f71b --- /dev/null +++ b/tests/ui/moves/moved-value-on-as-ref-arg.fixed @@ -0,0 +1,37 @@ +//@ run-rustfix +#![allow(unused_mut)] +use std::borrow::{Borrow, BorrowMut}; +use std::convert::{AsMut, AsRef}; +struct Bar; + +impl AsRef for Bar { + fn as_ref(&self) -> &Bar { + self + } +} + +impl AsMut for Bar { + fn as_mut(&mut self) -> &mut Bar { + self + } +} + +fn foo>(_: T) {} +fn qux>(_: T) {} +fn bat>(_: T) {} +fn baz>(_: T) {} + +pub fn main() { + let bar = Bar; + foo(&bar); + let _baa = bar; //~ ERROR use of moved value + let mut bar = Bar; + qux(&mut bar); + let _baa = bar; //~ ERROR use of moved value + let bar = Bar; + bat(&bar); + let _baa = bar; //~ ERROR use of moved value + let mut bar = Bar; + baz(&mut bar); + let _baa = bar; //~ ERROR use of moved value +} diff --git a/tests/ui/moves/moved-value-on-as-ref-arg.rs b/tests/ui/moves/moved-value-on-as-ref-arg.rs new file mode 100644 index 0000000000000..632af9efcda3e --- /dev/null +++ b/tests/ui/moves/moved-value-on-as-ref-arg.rs @@ -0,0 +1,37 @@ +//@ run-rustfix +#![allow(unused_mut)] +use std::borrow::{Borrow, BorrowMut}; +use std::convert::{AsMut, AsRef}; +struct Bar; + +impl AsRef for Bar { + fn as_ref(&self) -> &Bar { + self + } +} + +impl AsMut for Bar { + fn as_mut(&mut self) -> &mut Bar { + self + } +} + +fn foo>(_: T) {} +fn qux>(_: T) {} +fn bat>(_: T) {} +fn baz>(_: T) {} + +pub fn main() { + let bar = Bar; + foo(bar); + let _baa = bar; //~ ERROR use of moved value + let mut bar = Bar; + qux(bar); + let _baa = bar; //~ ERROR use of moved value + let bar = Bar; + bat(bar); + let _baa = bar; //~ ERROR use of moved value + let mut bar = Bar; + baz(bar); + let _baa = bar; //~ ERROR use of moved value +} diff --git a/tests/ui/moves/moved-value-on-as-ref-arg.stderr b/tests/ui/moves/moved-value-on-as-ref-arg.stderr new file mode 100644 index 0000000000000..4004b7a43bc09 --- /dev/null +++ b/tests/ui/moves/moved-value-on-as-ref-arg.stderr @@ -0,0 +1,79 @@ +error[E0382]: use of moved value: `bar` + --> $DIR/moved-value-on-as-ref-arg.rs:27:16 + | +LL | let bar = Bar; + | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | foo(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +help: borrow the value to avoid moving it + | +LL | foo(&bar); + | + + +error[E0382]: use of moved value: `bar` + --> $DIR/moved-value-on-as-ref-arg.rs:30:16 + | +LL | let mut bar = Bar; + | ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | qux(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +note: if `Bar` implemented `Clone`, you could clone the value + --> $DIR/moved-value-on-as-ref-arg.rs:5:1 + | +LL | struct Bar; + | ^^^^^^^^^^ consider implementing `Clone` for this type +... +LL | qux(bar); + | --- you could clone this value +help: borrow the value to avoid moving it + | +LL | qux(&mut bar); + | ++++ + +error[E0382]: use of moved value: `bar` + --> $DIR/moved-value-on-as-ref-arg.rs:33:16 + | +LL | let bar = Bar; + | --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | bat(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +help: borrow the value to avoid moving it + | +LL | bat(&bar); + | + + +error[E0382]: use of moved value: `bar` + --> $DIR/moved-value-on-as-ref-arg.rs:36:16 + | +LL | let mut bar = Bar; + | ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait +LL | baz(bar); + | --- value moved here +LL | let _baa = bar; + | ^^^ value used here after move + | +note: if `Bar` implemented `Clone`, you could clone the value + --> $DIR/moved-value-on-as-ref-arg.rs:5:1 + | +LL | struct Bar; + | ^^^^^^^^^^ consider implementing `Clone` for this type +... +LL | baz(bar); + | --- you could clone this value +help: borrow the value to avoid moving it + | +LL | baz(&mut bar); + | ++++ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0382`.