Skip to content

Commit 2ab8480

Browse files
committed
Suggest borrowing arguments in generic positions when trait bounds are 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.
1 parent ea37000 commit 2ab8480

13 files changed

+327
-300
lines changed

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 157 additions & 159 deletions
Large diffs are not rendered by default.

tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
1111
|
1212
LL | if let MultiVariant::Point(ref mut x, _) = point {
1313
| ^^^^^
14-
help: consider mutably borrowing `c`
15-
|
16-
LL | let a = &mut c;
17-
| ++++
1814

1915
error: aborting due to 1 previous error
2016

tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
1111
|
1212
LL | let SingleVariant::Point(ref mut x, _) = point;
1313
| ^^^^^
14-
help: consider mutably borrowing `c`
15-
|
16-
LL | let b = &mut c;
17-
| ++++
1814

1915
error: aborting due to 1 previous error
2016

tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
1111
|
1212
LL | x.y.a += 1;
1313
| ^^^^^
14-
help: consider mutably borrowing `hello`
15-
|
16-
LL | let b = &mut hello;
17-
| ++++
1814

1915
error: aborting due to 1 previous error
2016

tests/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
1111
|
1212
LL | x.0 += 1;
1313
| ^^^
14-
help: consider mutably borrowing `hello`
15-
|
16-
LL | let b = &mut hello;
17-
| ++++
1814

1915
error: aborting due to 1 previous error
2016

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,20 @@
11
//! auxiliary definitons for suggest-borrow-for-generic-arg.rs, to ensure the suggestion works on
22
//! functions defined in other crates.
33
4-
use std::borrow::{Borrow, BorrowMut};
5-
use std::convert::{AsMut, AsRef};
6-
pub struct Bar;
4+
use std::io::{self, Read, Write};
5+
use std::iter::Sum;
76

8-
impl AsRef<Bar> for Bar {
9-
fn as_ref(&self) -> &Bar {
10-
self
11-
}
7+
pub fn write_stuff<W: Write>(mut writer: W) -> io::Result<()> {
8+
writeln!(writer, "stuff")
129
}
1310

14-
impl AsMut<Bar> for Bar {
15-
fn as_mut(&mut self) -> &mut Bar {
16-
self
17-
}
11+
pub fn read_and_discard<R: Read>(mut reader: R) -> io::Result<()> {
12+
let mut buf = Vec::new();
13+
reader.read_to_end(&mut buf).map(|_| ())
1814
}
1915

20-
pub fn foo<T: AsRef<Bar>>(_: T) {}
21-
pub fn qux<T: AsMut<Bar>>(_: T) {}
22-
pub fn bat<T: Borrow<T>>(_: T) {}
23-
pub fn baz<T: BorrowMut<T>>(_: T) {}
16+
pub fn sum_three<I: IntoIterator>(iter: I) -> <I as IntoIterator>::Item
17+
where <I as IntoIterator>::Item: Sum
18+
{
19+
iter.into_iter().take(3).sum()
20+
}

tests/ui/moves/borrow-closures-instead-of-move.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff 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()) {
22
loop {
33
takes_fnonce(f);
44
//~^ ERROR use of moved value

tests/ui/moves/borrow-closures-instead-of-move.stderr

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,6 @@ LL | loop {
88
LL | takes_fnonce(f);
99
| ^ value moved here, in previous iteration of loop
1010
|
11-
note: consider changing this parameter type in function `takes_fnonce` to borrow instead if owning the value isn't necessary
12-
--> $DIR/borrow-closures-instead-of-move.rs:34:20
13-
|
14-
LL | fn takes_fnonce(_: impl FnOnce()) {}
15-
| ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value
16-
| |
17-
| in this function
18-
help: if `impl Fn()` implemented `Clone`, you could clone the value
19-
--> $DIR/borrow-closures-instead-of-move.rs:1:16
20-
|
21-
LL | fn takes_fn(f: impl Fn()) {
22-
| ^^^^^^^^^ consider constraining this type parameter with `Clone`
23-
LL | loop {
24-
LL | takes_fnonce(f);
25-
| - you could clone this value
2611
help: consider borrowing `f`
2712
|
2813
LL | takes_fnonce(&f);
@@ -40,13 +25,6 @@ LL | takes_fnonce(m);
4025
LL | takes_fnonce(m);
4126
| ^ value used here after move
4227
|
43-
note: consider changing this parameter type in function `takes_fnonce` to borrow instead if owning the value isn't necessary
44-
--> $DIR/borrow-closures-instead-of-move.rs:34:20
45-
|
46-
LL | fn takes_fnonce(_: impl FnOnce()) {}
47-
| ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value
48-
| |
49-
| in this function
5028
help: if `impl FnMut()` implemented `Clone`, you could clone the value
5129
--> $DIR/borrow-closures-instead-of-move.rs:9:20
5230
|

tests/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ LL | fn conspirator<F>(mut f: F) where F: FnMut(&mut R, bool) {
2424
| ^ consider constraining this type parameter with `Clone`
2525
LL | let mut r = R {c: Box::new(f)};
2626
| - you could clone this value
27-
help: consider mutably borrowing `f`
28-
|
29-
LL | let mut r = R {c: Box::new(&mut f)};
30-
| ++++
3127

3228
error: aborting due to 2 previous errors
3329

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,46 @@
1-
//! Test suggetions to borrow generic arguments instead of moving when all predicates hold after
2-
//! substituting in the reference type.
1+
//! Test suggetions to borrow generic arguments instead of moving. Tests for other instances of this
2+
//! can be found in `moved-value-on-as-ref-arg.rs` and `borrow-closures-instead-of-move.rs`
33
//@ run-rustfix
4-
//@ aux-build:suggest-borrow-for-generic-arg-aux.rs
4+
//@ aux-crate:aux=suggest-borrow-for-generic-arg-aux.rs
5+
//@ edition: 2021
56

67
#![allow(unused_mut)]
7-
extern crate suggest_borrow_for_generic_arg_aux as aux;
8+
use std::io::{self, Write};
89

9-
pub fn main() {
10-
let bar = aux::Bar;
11-
aux::foo(&bar); //~ HELP consider borrowing `bar`
12-
let _baa = bar; //~ ERROR use of moved value
13-
let mut bar = aux::Bar;
14-
aux::qux(&mut bar); //~ HELP consider mutably borrowing `bar`
15-
let _baa = bar; //~ ERROR use of moved value
16-
let bar = aux::Bar;
17-
aux::bat(&bar); //~ HELP consider borrowing `bar`
18-
let _baa = bar; //~ ERROR use of moved value
19-
let mut bar = aux::Bar;
20-
aux::baz(&mut bar); //~ HELP consider mutably borrowing `bar`
21-
let _baa = bar; //~ ERROR use of moved value
10+
// test for `std::io::Write` (#131413)
11+
fn test_write() -> io::Result<()> {
12+
let mut stdout = io::stdout();
13+
aux::write_stuff(&stdout)?; //~ HELP consider borrowing `stdout`
14+
writeln!(stdout, "second line")?; //~ ERROR borrow of moved value: `stdout`
15+
16+
let mut buf = Vec::new();
17+
aux::write_stuff(&mut buf.clone())?; //~ HELP consider mutably borrowing `buf`
18+
//~^ HELP consider cloning the value
19+
writeln!(buf, "second_line") //~ ERROR borrow of moved value: `buf`
20+
}
21+
22+
/// test for `std::io::Read` (#131413)
23+
fn test_read() -> io::Result<()> {
24+
let stdin = io::stdin();
25+
aux::read_and_discard(&stdin)?; //~ HELP consider borrowing `stdin`
26+
aux::read_and_discard(stdin)?; //~ ERROR use of moved value: `stdin`
27+
28+
let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]);
29+
aux::read_and_discard(&mut bytes.clone())?; //~ HELP consider mutably borrowing `bytes`
30+
//~^ HELP consider cloning the value
31+
aux::read_and_discard(bytes) //~ ERROR use of moved value: `bytes`
32+
}
33+
34+
/// test that suggestions work with projection types in the callee's signature
35+
fn test_projections() {
36+
let mut iter = [1, 2, 3, 4, 5, 6].into_iter();
37+
let _six: usize = aux::sum_three(&mut iter.clone()); //~ HELP consider mutably borrowing `iter`
38+
//~^ HELP consider cloning the value
39+
let _fifteen: usize = aux::sum_three(iter); //~ ERROR use of moved value: `iter`
40+
}
41+
42+
fn main() {
43+
test_write().unwrap();
44+
test_read().unwrap();
45+
test_projections();
2246
}

0 commit comments

Comments
 (0)