Suggest borrowing arguments in generic positions when trait bounds are satisfied

dianne · dianne · commit 3c4d3214a05c · 2024-10-25T23:24:05.000-07:00
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.
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Bar> for Bar {
-    fn as_ref(&self) -> &Bar {
-        self
-    }
+pub fn write_stuff<W: Write>(mut writer: W) -> io::Result<()> {
+    writeln!(writer, "stuff")
 }
 
-impl AsMut<Bar> for Bar {
-    fn as_mut(&mut self) -> &mut Bar {
-        self
-    }
+pub fn read_and_discard<R: Read>(mut reader: R) -> io::Result<()> {
+    let mut buf = Vec::new();
+    reader.read_to_end(&mut buf).map(|_| ())
 }
 
-pub fn foo<T: AsRef<Bar>>(_: T) {}
-pub fn qux<T: AsMut<Bar>>(_: T) {}
-pub fn bat<T: Borrow<T>>(_: T) {}
-pub fn baz<T: BorrowMut<T>>(_: T) {}
+pub fn sum_three<I: IntoIterator>(iter: I) -> <I as IntoIterator>::Item
+    where <I as IntoIterator>::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
@@ -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
@@ -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
@@ -24,10 +24,6 @@ LL | fn conspirator<F>(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
@@ -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
@@ -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
@@ -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<u8>`, 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<u8>`, 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<usize, 6>`, 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

Original file line number	Diff line number	Diff line change
@@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
`11`	`11`	`\|`
`12`	`12`	`LL \| if let MultiVariant::Point(ref mut x, _) = point {`
`13`	`13`	`\| ^^^^^`
`14`		-help: consider mutably borrowing `c`
`15`		`- \|`
`16`		`-LL \| let a = &mut c;`
`17`		`- \| ++++`
`18`	`14`
`19`	`15`	`error: aborting due to 1 previous error`
`20`	`16`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		-fn takes_fn(f: impl Fn()) { //~ HELP if `impl Fn()` implemented `Clone`
	`1`	`+fn takes_fn(f: impl Fn()) {`
`2`	`2`	`loop {`
`3`	`3`	`takes_fnonce(f);`
`4`	`4`	`//~^ ERROR use of moved value`