about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2024-05-02 01:15:40 +0000
committerEsteban Küber <esteban@kuber.com.ar>2024-05-09 23:25:31 +0000
commit2df4f7dd8c51e1c3e65b615d1c44fdc9d0b4b044 (patch)
tree3fbc86ced21974cfa630dc4ebc185dee45064c45
parente6e262f1250e6978eea528ee8c391786c9b4f44e (diff)
downloadrust-2df4f7dd8c51e1c3e65b615d1c44fdc9d0b4b044.tar.gz
rust-2df4f7dd8c51e1c3e65b615d1c44fdc9d0b4b044.zip
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
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs89
-rw-r--r--library/core/src/borrow.rs1
-rw-r--r--tests/ui/moves/moved-value-on-as-ref-arg.fixed37
-rw-r--r--tests/ui/moves/moved-value-on-as-ref-arg.rs37
-rw-r--r--tests/ui/moves/moved-value-on-as-ref-arg.stderr79
5 files changed, 224 insertions, 19 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index 6f0bd054329..9efb3362e18 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 bc026d0a446..ccb1cc4e974 100644
--- a/library/core/src/borrow.rs
+++ b/library/core/src/borrow.rs
@@ -184,6 +184,7 @@ pub trait Borrow<Borrowed: ?Sized> {
 /// an underlying type by providing a mutable reference. See [`Borrow<T>`]
 /// for more information on borrowing as another type.
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_diagnostic_item = "BorrowMut"]
 pub trait BorrowMut<Borrowed: ?Sized>: Borrow<Borrowed> {
     /// 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 00000000000..292fa98a3f7
--- /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<Bar> for Bar {
+    fn as_ref(&self) -> &Bar {
+        self
+    }
+}
+
+impl AsMut<Bar> for Bar {
+    fn as_mut(&mut self) -> &mut Bar {
+        self
+    }
+}
+
+fn foo<T: AsRef<Bar>>(_: T) {}
+fn qux<T: AsMut<Bar>>(_: T) {}
+fn bat<T: Borrow<T>>(_: T) {}
+fn baz<T: BorrowMut<T>>(_: 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 00000000000..632af9efcda
--- /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<Bar> for Bar {
+    fn as_ref(&self) -> &Bar {
+        self
+    }
+}
+
+impl AsMut<Bar> for Bar {
+    fn as_mut(&mut self) -> &mut Bar {
+        self
+    }
+}
+
+fn foo<T: AsRef<Bar>>(_: T) {}
+fn qux<T: AsMut<Bar>>(_: T) {}
+fn bat<T: Borrow<T>>(_: T) {}
+fn baz<T: BorrowMut<T>>(_: 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 00000000000..4004b7a43bc
--- /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`.