Skip to content

Commit

Permalink
Suggest borrowing arguments in generic positions when trait bounds ar…
Browse files Browse the repository at this point in the history
…e 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.
  • Loading branch information
dianne committed Nov 14, 2024
1 parent ea37000 commit 2ab8480
Show file tree
Hide file tree
Showing 13 changed files with 327 additions and 300 deletions.
316 changes: 157 additions & 159 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 12 additions & 15 deletions tests/ui/moves/auxiliary/suggest-borrow-for-generic-arg-aux.rs
Original file line number Diff line number Diff line change
@@ -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()
}
2 changes: 1 addition & 1 deletion tests/ui/moves/borrow-closures-instead-of-move.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
22 changes: 0 additions & 22 deletions tests/ui/moves/borrow-closures-instead-of-move.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
58 changes: 41 additions & 17 deletions tests/ui/moves/suggest-borrow-for-generic-arg.fixed
Original file line number Diff line number Diff line change
@@ -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();
}
58 changes: 41 additions & 17 deletions tests/ui/moves/suggest-borrow-for-generic-arg.rs
Original file line number Diff line number Diff line change
@@ -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();
}
120 changes: 75 additions & 45 deletions tests/ui/moves/suggest-borrow-for-generic-arg.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading

0 comments on commit 2ab8480

Please sign in to comment.