Commit Graph

39 Commits

Author SHA1 Message Date
Michael Goulet
91acacf85b Peel off explicit (or implicit) deref before suggesting clone on move error in borrowck 2024-07-26 14:41:56 -04:00
Matthias Krüger
8de487fdbd
Rollup merge of #124599 - estebank:issue-41708, r=wesleywiser
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
2024-07-11 17:01:36 +02:00
Esteban Küber
9fd7784b97 Fix ... in multline code-skips in suggestions
When we have long code skips, we write `...` in the line number gutter.

For suggestions, we were "centering" the `...` with the line, but that was consistent with what we do in every other case.
2024-06-20 04:25:17 +00:00
Esteban Küber
2df4f7dd8c 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
2024-05-09 23:25:31 +00:00
Esteban Küber
d68f2a6b71 Mention when type parameter could be Clone
```
error[E0382]: use of moved value: `t`
  --> $DIR/use_of_moved_value_copy_suggestions.rs:7:9
   |
LL | fn duplicate_t<T>(t: T) -> (T, T) {
   |                   - move occurs because `t` has type `T`, which does not implement the `Copy` trait
...
LL |     (t, t)
   |      -  ^ value used here after move
   |      |
   |      value moved here
   |
help: if `T` implemented `Clone`, you could clone the value
  --> $DIR/use_of_moved_value_copy_suggestions.rs:4:16
   |
LL | fn duplicate_t<T>(t: T) -> (T, T) {
   |                ^ consider constraining this type parameter with `Clone`
...
LL |     (t, t)
   |      - you could clone this value
help: consider restricting type parameter `T`
   |
LL | fn duplicate_t<T: Copy>(t: T) -> (T, T) {
   |                 ++++++
```

The `help` is new. On ADTs, we also extend the output with span labels:

```
error[E0507]: cannot move out of static item `FOO`
  --> $DIR/issue-17718-static-move.rs:6:14
   |
LL |     let _a = FOO;
   |              ^^^ move occurs because `FOO` has type `Foo`, which does not implement the `Copy` trait
   |
note: if `Foo` implemented `Clone`, you could clone the value
  --> $DIR/issue-17718-static-move.rs:1:1
   |
LL | struct Foo;
   | ^^^^^^^^^^ consider implementing `Clone` for this type
...
LL |     let _a = FOO;
   |              --- you could clone this value
help: consider borrowing here
   |
LL |     let _a = &FOO;
   |              +
```
2024-04-24 22:21:15 +00:00
Michael Goulet
c95761385e Make array suggestions slightly more accurate 2024-04-15 21:45:47 -04:00
Michael Goulet
8a981b6fee Use /* value */ as a placeholder 2024-04-15 21:36:52 -04:00
bors
6eaa7fb576 Auto merge of #122603 - estebank:clone-o-rama, r=lcnr
Detect borrow checker errors where `.clone()` would be an appropriate user action

When a value is moved twice, suggest cloning the earlier move:

```
error[E0509]: cannot move out of type `U2`, which implements the `Drop` trait
  --> $DIR/union-move.rs:49:18
   |
LL |         move_out(x.f1_nocopy);
   |                  ^^^^^^^^^^^
   |                  |
   |                  cannot move out of here
   |                  move occurs because `x.f1_nocopy` has type `ManuallyDrop<RefCell<i32>>`, which does not implement the `Copy` trait
   |
help: consider cloning the value if the performance cost is acceptable
   |
LL |         move_out(x.f1_nocopy.clone());
   |                             ++++++++
```

When a value is borrowed by an `fn` call, consider if cloning the result of the call would be reasonable, and suggest cloning that, instead of the argument:

```
error[E0505]: cannot move out of `a` because it is borrowed
  --> $DIR/variance-issue-20533.rs:53:14
   |
LL |         let a = AffineU32(1);
   |             - binding `a` declared here
LL |         let x = bat(&a);
   |                     -- borrow of `a` occurs here
LL |         drop(a);
   |              ^ move out of `a` occurs here
LL |         drop(x);
   |              - borrow later used here
   |
help: consider cloning the value if the performance cost is acceptable
   |
LL |         let x = bat(&a).clone();
   |                        ++++++++
```

otherwise, suggest cloning the argument:

```
error[E0505]: cannot move out of `a` because it is borrowed
  --> $DIR/variance-issue-20533.rs:59:14
   |
LL |         let a = ClonableAffineU32(1);
   |             - binding `a` declared here
LL |         let x = foo(&a);
   |                     -- borrow of `a` occurs here
LL |         drop(a);
   |              ^ move out of `a` occurs here
LL |         drop(x);
   |              - borrow later used here
   |
help: consider cloning the value if the performance cost is acceptable
   |
LL -         let x = foo(&a);
LL +         let x = foo(a.clone());
   |
```

This suggestion doesn't attempt to square out the types between what's cloned and what the `fn` expects, to allow the user to make a determination on whether to change the `fn` call or `fn` definition themselves.

Special case move errors caused by `FnOnce`:

```
error[E0382]: use of moved value: `blk`
  --> $DIR/once-cant-call-twice-on-heap.rs:8:5
   |
LL | fn foo<F:FnOnce()>(blk: F) {
   |                    --- move occurs because `blk` has type `F`, which does not implement the `Copy` trait
LL |     blk();
   |     ----- `blk` moved due to this call
LL |     blk();
   |     ^^^ value used here after move
   |
note: `FnOnce` closures can only be called once
  --> $DIR/once-cant-call-twice-on-heap.rs:6:10
   |
LL | fn foo<F:FnOnce()>(blk: F) {
   |          ^^^^^^^^ `F` is made to be an `FnOnce` closure here
LL |     blk();
   |     ----- this value implements `FnOnce`, which causes it to be moved when called
```

Account for redundant `.clone()` calls in resulting suggestions:

```
error[E0507]: cannot move out of dereference of `S`
  --> $DIR/needs-clone-through-deref.rs:15:18
   |
LL |         for _ in self.clone().into_iter() {}
   |                  ^^^^^^^^^^^^ ----------- value moved due to this method call
   |                  |
   |                  move occurs because value has type `Vec<usize>`, which does not implement the `Copy` trait
   |
note: `into_iter` takes ownership of the receiver `self`, which moves value
  --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
help: you can `clone` the value and consume it, but this might not be your desired behavior
   |
LL |         for _ in <Vec<usize> as Clone>::clone(&self).into_iter() {}
   |                  ++++++++++++++++++++++++++++++    ~
```

We use the presence of `&mut` values in a move error as a proxy for the user caring about side effects, so we don't emit a clone suggestion in that case:

```
error[E0505]: cannot move out of `s` because it is borrowed
  --> $DIR/borrowck-overloaded-index-move-index.rs:53:7
   |
LL |     let mut s = "hello".to_string();
   |         ----- binding `s` declared here
LL |     let rs = &mut s;
   |              ------ borrow of `s` occurs here
...
LL |     f[s] = 10;
   |       ^ move out of `s` occurs here
...
LL |     use_mut(rs);
   |             -- borrow later used here
```

We properly account for `foo += foo;` errors where we *don't* suggest `foo.clone() += foo;`, instead suggesting `foo += foo.clone();`.

---

Each commit can be reviewed in isolation. There are some "cleanup" commits, but kept them separate in order to show *why* specific changes were being made, and their effect on tests' output.

Fix #49693, CC #64167.
2024-04-13 09:07:26 +00:00
Esteban Küber
dea9b5031c Better account for more cases involving closures 2024-04-12 04:46:31 +00:00
Esteban Küber
d97d2fe744 Mention when the type of the moved value doesn't implement Clone 2024-04-11 16:41:42 +00:00
Esteban Küber
01b810e052 Silence redundant clone suggestion 2024-04-11 16:41:41 +00:00
Esteban Küber
10c2fbec24 Suggest .clone() in some move errors
```
error[E0507]: cannot move out of `*x` which is behind a shared reference
  --> $DIR/borrowck-fn-in-const-a.rs:6:16
   |
LL |         return *x
   |                ^^ move occurs because `*x` has type `String`, which does not implement the `Copy` trait
   |
help: consider cloning the value if the performance cost is acceptable
   |
LL -         return *x
LL +         return x.clone()
   |
```
2024-04-11 16:41:41 +00:00
Esteban Küber
5eb573a343 Account for .clone() when suggesting <T as Clone>::clone 2024-04-11 16:41:40 +00:00
Esteban Küber
e17388b809 Handle more cases of value suggestions 2024-04-10 20:36:14 +00:00
Esteban Küber
a983dd8563 Tweak value suggestions in borrowck and hir_analysis
Unify the output of `suggest_assign_value` and `ty_kind_suggestion`.

Ideally we'd make these a single function, but doing so would likely require modify the crate dependency tree.
2024-04-09 23:37:13 +00:00
Esteban Küber
f216bac861 Add HELP to test 2024-03-17 21:45:03 +00:00
Esteban Küber
78d29ad8d6 Point at continue and break that might be in the wrong place
Sometimes move errors are because of a misplaced `continue`, but we didn't
surface that anywhere. Now when there are more than one set of nested loops
we show them out and point at the `continue` and `break` expressions within
that might need to go elsewhere.

```
error[E0382]: use of moved value: `foo`
  --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18
   |
LL |     for foo in foos {
   |         ---
   |         |
   |         this reinitialization might get skipped
   |         move occurs because `foo` has type `String`, which does not implement the `Copy` trait
...
LL |         for bar in &bars {
   |         ---------------- inside of this loop
...
LL |                 baz.push(foo);
   |                          --- value moved here, in previous iteration of loop
...
LL |         qux.push(foo);
   |                  ^^^ value used here after move
   |
note: verify that your loop breaking logic is correct
  --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17
   |
LL |     for foo in foos {
   |     ---------------
...
LL |         for bar in &bars {
   |         ----------------
...
LL |                 continue;
   |                 ^^^^^^^^ this `continue` advances the loop at line 33
help: consider moving the expression out of the loop so it is only moved once
   |
LL ~         let mut value = baz.push(foo);
LL ~         for bar in &bars {
LL |
 ...
LL |             if foo == *bar {
LL ~                 value;
   |
help: consider cloning the value if the performance cost is acceptable
   |
LL |                 baz.push(foo.clone());
   |                             ++++++++
```

Fix #92531.
2024-03-17 21:32:26 +00:00
Esteban Küber
14473adf42 Detect when move of !Copy value occurs within loop and should likely not be cloned
When encountering a move error on a value within a loop of any kind,
identify if the moved value belongs to a call expression that should not
be cloned and avoid the semantically incorrect suggestion. Also try to
suggest moving the call expression outside of the loop instead.

```
error[E0382]: use of moved value: `vec`
  --> $DIR/recreating-value-in-loop-condition.rs:6:33
   |
LL |     let vec = vec!["one", "two", "three"];
   |         --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
LL |     while let Some(item) = iter(vec).next() {
   |     ----------------------------^^^--------
   |     |                           |
   |     |                           value moved here, in previous iteration of loop
   |     inside of this loop
   |
note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary
  --> $DIR/recreating-value-in-loop-condition.rs:1:17
   |
LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> {
   |    ----         ^^^^^^ this parameter takes ownership of the value
   |    |
   |    in this function
help: consider moving the expression out of the loop so it is only moved once
   |
LL ~     let mut value = iter(vec);
LL ~     while let Some(item) = value.next() {
   |
```

We use the presence of a `break` in the loop that would be affected by
the moved value as a heuristic for "shouldn't be cloned".

Fix #121466.
2024-03-17 21:32:26 +00:00
Alex Crichton
75fa9f6dec compiletest: Add a //@ needs-threads directive
This commit is extracted from #122036 and adds a new directive to the
`compiletest` test runner, `//@ needs-threads`. This is intended to
capture the need that a target must implement threading to execute a
specific test, typically one that uses `std::thread`. This is primarily
done for WebAssembly targets which currently do not have threads by
default. This enables transitioning a lot of `//@ ignore-wasm*`-style
ignores into a more self-documenting `//@ needs-threads` directive.
Additionally the `wasm32-wasi-preview1-threads` target, for example,
does actually have threads, but isn't tested in CI at this time. This
change enables running these tests for that target, but not other wasm
targets.
2024-03-06 12:35:07 -08:00
许杰友 Jieyou Xu (Joe)
ec2cc761bc
[AUTO-GENERATED] Migrate ui tests from // to //@ directives 2024-02-16 20:02:50 +00:00
r0cky
c7519d42c2 Update tests 2024-02-07 10:42:01 +08:00
Michael Goulet
04a6fd241b Make InferCtxtExt::could_impl_trait less messed up 2024-01-13 22:00:34 +00:00
surechen
40ae34194c remove redundant imports
detects redundant imports that can be eliminated.

for #117772 :

In order to facilitate review and modification, split the checking code and
removing redundant imports code into two PR.
2023-12-10 10:56:22 +08:00
jyn
eb53721a34 recurse into refs when comparing tys for diagnostics 2023-12-07 23:00:46 -05:00
Esteban Küber
a8faa8288c Fix rebase 2023-12-04 22:00:34 +00:00
Esteban Küber
a754fcaa43 On "this .clone() is on the reference", provide more info
When encountering a case where `let x: T = (val: &T).clone();` and
`T: !Clone`, already mention that the reference is being cloned. We now
also suggest `#[derive(Clone)]` not only on `T` but also on type
parameters to satisfy blanket implementations.

```
error[E0308]: mismatched types
  --> $DIR/assignment-of-clone-call-on-ref-due-to-missing-bound.rs:17:39
   |
LL |             let mut x: HashSet<Day> = v.clone();
   |                        ------------   ^^^^^^^^^ expected `HashSet<Day>`, found `&HashSet<Day>`
   |                        |
   |                        expected due to this
   |
   = note: expected struct `HashSet<Day>`
           found reference `&HashSet<Day>`
note: `HashSet<Day>` does not implement `Clone`, so `&HashSet<Day>` was cloned instead
  --> $DIR/assignment-of-clone-call-on-ref-due-to-missing-bound.rs:17:39
   |
LL |             let mut x: HashSet<Day> = v.clone();
   |                                       ^
   = help: `Clone` is not implemented because the trait bound `Day: Clone` is not satisfied
help: consider annotating `Day` with `#[derive(Clone)]`
   |
LL + #[derive(Clone)]
LL | enum Day {
   |
```

Case taken from # #41825.
2023-12-04 21:54:33 +00:00
Esteban Küber
90db536741 Tweak output on specific case 2023-12-04 21:54:32 +00:00
Esteban Küber
98cfed7b97 Suggest cloning and point out obligation errors on move error
When encountering a move error, look for implementations of `Clone` for
the moved type. If there is one, check if all its obligations are met.
If they are, we suggest cloning without caveats. If they aren't, we
suggest cloning while mentioning the unmet obligations, potentially
suggesting `#[derive(Clone)]` when appropriate.

```
error[E0507]: cannot move out of a shared reference
  --> $DIR/suggest-clone-when-some-obligation-is-unmet.rs:20:28
   |
LL |     let mut copy: Vec<U> = map.clone().into_values().collect();
   |                            ^^^^^^^^^^^ ------------- value moved due to this method call
   |                            |
   |                            move occurs because value has type `HashMap<T, U, Hash128_1>`, which does not implement the `Copy` trait
   |
note: `HashMap::<K, V, S>::into_values` takes ownership of the receiver `self`, which moves value
  --> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL
help: you could `clone` the value and consume it, if the `Hash128_1: Clone` trait bound could be satisfied
   |
LL |     let mut copy: Vec<U> = <HashMap<T, U, Hash128_1> as Clone>::clone(&map.clone()).into_values().collect();
   |                            ++++++++++++++++++++++++++++++++++++++++++++           +
help: consider annotating `Hash128_1` with `#[derive(Clone)]`
   |
LL + #[derive(Clone)]
LL | pub struct Hash128_1;
   |
```

Fix #109429.
2023-12-04 21:54:32 +00:00
Esteban Küber
03c88aaf21 Tweak .clone() suggestion to work in more cases
When going through auto-deref, the `<T as Clone>` impl sometimes needs
to be specified for rustc to actually clone the value and not the
reference.

```
error[E0507]: cannot move out of dereference of `S`
  --> $DIR/needs-clone-through-deref.rs:15:18
   |
LL |         for _ in self.clone().into_iter() {}
   |                  ^^^^^^^^^^^^ ----------- value moved due to this method call
   |                  |
   |                  move occurs because value has type `Vec<usize>`, which does not implement the `Copy` trait
   |
note: `into_iter` takes ownership of the receiver `self`, which moves value
  --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
help: you can `clone` the value and consume it, but this might not be your desired behavior
   |
LL |         for _ in <Vec<usize> as Clone>::clone(&self.clone()).into_iter() {}
   |                  ++++++++++++++++++++++++++++++            +
```

CC #109429.
2023-12-04 21:54:32 +00:00
Nilstrieb
41e8d152dc Show number in error message even for one error
Co-authored-by: Adrian <adrian.iosdev@gmail.com>
2023-11-24 19:15:52 +01:00
Caio
5a69151d7d Move tests 2023-08-28 17:47:37 -03:00
Esteban Küber
edc3e26773 Account for Rc and Arc when suggesting to clone
When suggesting to clone a reference-counted value, be less uncertain.
2023-08-04 17:50:12 +00:00
Michael Goulet
a74db1abb3 Fix another strange suggestion span 2023-07-10 20:09:28 +00:00
Michael Goulet
3a3f4a2144 Don't use method span on clone suggestion 2023-07-10 20:09:28 +00:00
Michael Goulet
fe870424a7 Do not set up wrong span for adjustments 2023-07-10 20:09:26 +00:00
Caio
d75e43c371 Move tests 2023-02-16 11:42:35 -03:00
Nilstrieb
832751fe1d Also erase substs for new infcx in pin move error
The code originally correctly erased the regions of the type it passed
to the newly created infcx. But after the `fn_sig` query was made to
return an `EarlyBinder<T>`, some substs that were around were
substituted there without erasing their regions. They were then passed
into the newly cerated infcx, which caused the ICE.
2023-01-28 20:43:16 +01:00
Esteban Küber
7b8251e188 Account for method call and indexing when looking for inner-most path in expression 2023-01-17 02:52:43 +00:00
Albert Larsan
cf2dff2b1e
Move /src/test to /tests 2023-01-11 09:32:08 +00:00