Commit Graph

1569 Commits

Author SHA1 Message Date
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
4c7213c888 review comments
Added comments and reworded messages
2024-04-12 20:57:07 +00:00
Esteban Küber
dea9b5031c Better account for more cases involving closures 2024-04-12 04:46:31 +00:00
Esteban Küber
3cdc6897c5 Fix rebase 2024-04-12 04:45:50 +00:00
Matthias Krüger
6f78bf2322
Rollup merge of #123599 - matthiaskrgr:rm, r=cjgillot
remove some things that do not need to be
2024-04-12 04:38:21 +02:00
Matthias Krüger
ec91d71a38
Rollup merge of #123523 - estebank:issue-123414, r=BoxyUwU
Account for trait/impl difference when suggesting changing argument from ref to mut ref

Do not ICE when encountering a lifetime error involving an argument with an immutable reference of a method that differs from the trait definition.

Fix #123414.
2024-04-11 22:38:54 +02:00
Matthias Krüger
f9ca213510 remove some things that do not need to be 2024-04-11 21:09:52 +02: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
d578ac9e47 Account for move error in the spread operator on struct literals
We attempt to suggest an appropriate clone for move errors on expressions
like `S { ..s }` where a field isn't `Copy`. If we can't suggest, we still don't
emit the incorrect suggestion of `S { ..s }.clone()`.

```
error[E0509]: cannot move out of type `S<K>`, which implements the `Drop` trait
  --> $DIR/borrowck-struct-update-with-dtor.rs:28:19
   |
LL |         let _s2 = S { a: 2, ..s0 };
   |                   ^^^^^^^^^^^^^^^^
   |                   |
   |                   cannot move out of here
   |                   move occurs because `s0.c` has type `K`, which does not implement the `Copy` trait
   |
help: clone the value from the field instead of using the spread operator syntax
   |
LL |         let _s2 = S { a: 2, c: s0.c.clone(), ..s0 };
   |                           +++++++++++++++++
```
```
error[E0509]: cannot move out of type `S<()>`, which implements the `Drop` trait
  --> $DIR/borrowck-struct-update-with-dtor.rs:20:19
   |
LL |         let _s2 = S { a: 2, ..s0 };
   |                   ^^^^^^^^^^^^^^^^
   |                   |
   |                   cannot move out of here
   |                   move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait
   |
note: `B` doesn't implement `Copy` or `Clone`
  --> $DIR/borrowck-struct-update-with-dtor.rs:4:1
   |
LL | struct B;
   | ^^^^^^^^
help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax
   |
LL |         let _s2 = S { a: 2, b: s0.b.clone(), ..s0 };
   |                           +++++++++++++++++
```
2024-04-11 16:41:42 +00:00
Esteban Küber
4ca876b7a4 Better account for FnOnce in move errors
```
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
```
2024-04-11 16:41:42 +00:00
Esteban Küber
dfe28debb9 Account for assign binops in clone suggestions
Explicitly look for `expr += other_expr;` and avoid suggesting
`expr.clone() += other_expr;`, instead suggesting `expr += other_expr.clone();`.
2024-04-11 16:41:41 +00:00
Esteban Küber
a1a3abb08f When possible, suggest cloning the result of a call instead of an argument
```
error[E0505]: cannot move out of `a` because it is borrowed
  --> $DIR/variance-issue-20533.rs:28:14
   |
LL |         let a = AffineU32(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).clone();
   |                        ++++++++
```
2024-04-11 16:41:41 +00:00
Esteban Küber
7f7f6792f1 Do not recomment cloning explicit &mut expressions 2024-04-11 16:41:41 +00:00
Esteban Küber
5a7caa3174 Fix accuracy of T: Clone check in suggestion 2024-04-11 16:41:41 +00:00
Esteban Küber
259348cf7e Remove unnecessary argument from suggest_cloning 2024-04-11 16:41:41 +00:00
Esteban Küber
01b810e052 Silence redundant clone suggestion 2024-04-11 16:41:41 +00:00
Esteban Küber
065454dd1d More move error suggestions to clone
```
error[E0507]: cannot move out of `val`, a captured variable in an `FnMut` closure
  --> $DIR/issue-87456-point-to-closure.rs:10:28
   |
LL |     let val = String::new();
   |         --- captured outer variable
LL |
LL |     take_mut(|| {
   |              -- captured by this `FnMut` closure
LL |
LL |         let _foo: String = val;
   |                            ^^^ move occurs because `val` has type `String`, which does not implement the `Copy` trait
   |
help: consider borrowing here
   |
LL |         let _foo: String = &val;
   |                            +
help: consider cloning the value if the performance cost is acceptable
   |
LL |         let _foo: String = val.clone();
   |                               ++++++++
```
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
bce78102c3 Account for unops when suggesting cloning 2024-04-11 16:41:41 +00:00
Esteban Küber
fa2fc3ab96 Suggest .clone() when moved while borrowed 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
Matthias Krüger
93645f481d
Rollup merge of #123704 - estebank:diag-changes, r=compiler-errors
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-11 09:31:50 +02:00
bors
08273780d8 Auto merge of #122213 - estebank:issue-50195, r=oli-obk,estebank
Provide suggestion to dereference closure tail if appropriate

When encoutnering a case like

```rust
use std::collections::HashMap;

fn main() {
    let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3];

    let mut counts = HashMap::new();
    for num in vs {
        let count = counts.entry(num).or_insert(0);
        *count += 1;
    }

    let _ = counts.iter().max_by_key(|(_, v)| v);
```
produce the following suggestion
```
error: lifetime may not live long enough
  --> $DIR/return-value-lifetime-error.rs:13:47
   |
LL |     let _ = counts.iter().max_by_key(|(_, v)| v);
   |                                       ------- ^ returning this value requires that `'1` must outlive `'2`
   |                                       |     |
   |                                       |     return type of closure is &'2 &i32
   |                                       has type `&'1 (&i32, &i32)`
   |
help: dereference the return value
   |
LL |     let _ = counts.iter().max_by_key(|(_, v)| **v);
   |                                               ++
```

Fix #50195.
2024-04-11 02:37:35 +00:00
Esteban Küber
1eda0565fa Handle more cases of "values to suggest" given a type
Add handling for `String`, `Box`, `Option` and `Result`.
2024-04-10 23:58:36 +00:00
Esteban Küber
e17388b809 Handle more cases of value suggestions 2024-04-10 20:36:14 +00:00
Oli Scherer
f2465f8f20 Use trait solver to answer questions instead of manually writing a trait solver 2024-04-10 05:22:43 +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
bors
b234e44944 Auto merge of #122077 - oli-obk:eager_opaque_checks4, r=lcnr
Pass list of defineable opaque types into canonical queries

This eliminates `DefiningAnchor::Bubble` for good and brings the old solver closer to the new one wrt cycles and nested obligations. At that point the difference between `DefiningAnchor::Bind([])` and `DefiningAnchor::Error` was academic. We only used the difference for some sanity checks, which actually had to be worked around in places, so I just removed `DefiningAnchor` entirely and just stored the list of opaques that may be defined.

fixes #108498
fixes https://github.com/rust-lang/rust/issues/116877

* [x] run crater
  - https://github.com/rust-lang/rust/pull/122077#issuecomment-2013293931
2024-04-08 23:01:50 +00:00
Oli Scherer
2f2350e577 Eliminate DefiningAnchor now that is just a single-variant enum 2024-04-08 15:00:27 +00:00
Oli Scherer
b2b5fa825d Eagerly check for accidentally registered region constraints instead of waiting until borrowck is done 2024-04-08 15:00:26 +00:00
Oli Scherer
19bd91d128 Pass list of defineable opaque types into canonical queries 2024-04-08 15:00:26 +00:00
Oli Scherer
84acfe86de Actually create ranged int types in the type system. 2024-04-08 12:02:19 +00:00
Esteban Küber
731c0e59a4 Account for trait/impl difference when suggesting changing argument from ref to mut ref
Do not ICE when encountering a lifetime error involving an argument with
an immutable reference of a method that differs from the trait definition.

Fix #123414.
2024-04-06 16:23:10 +00:00
Esteban Küber
9de6b70bb6 Provide suggestion to dereference closure tail if appropriate
When encoutnering a case like

```rust
//@ run-rustfix
use std::collections::HashMap;

fn main() {
    let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3];

    let mut counts = HashMap::new();
    for num in vs {
        let count = counts.entry(num).or_insert(0);
        *count += 1;
    }

    let _ = counts.iter().max_by_key(|(_, v)| v);
```
produce the following suggestion
```
error: lifetime may not live long enough
  --> $DIR/return-value-lifetime-error.rs:13:47
   |
LL |     let _ = counts.iter().max_by_key(|(_, v)| v);
   |                                       ------- ^ returning this value requires that `'1` must outlive `'2`
   |                                       |     |
   |                                       |     return type of closure is &'2 &i32
   |                                       has type `&'1 (&i32, &i32)`
   |
help: dereference the return value
   |
LL |     let _ = counts.iter().max_by_key(|(_, v)| **v);
   |                                               ++
```

Fix #50195.
2024-04-05 19:42:55 +00:00
Matthias Krüger
25b0e84170
Rollup merge of #123419 - petrochenkov:zeroindex, r=compiler-errors
rustc_index: Add a `ZERO` constant to index types

It is commonly used.
2024-04-03 22:11:02 +02:00
Vadim Petrochenkov
b40ea03f8a rustc_index: Add a ZERO constant to index types
It is commonly used.
2024-04-03 19:06:22 +03:00
joboet
989660c3e6
rename expose_addr to expose_provenance 2024-04-03 16:00:38 +02:00
Jacob Pratt
e9ef8e1efa
Rollup merge of #122935 - RalfJung:with-exposed-provenance, r=Amanieu
rename ptr::from_exposed_addr -> ptr::with_exposed_provenance

As discussed on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/To.20expose.20or.20not.20to.20expose/near/427757066).

The old name, `from_exposed_addr`, makes little sense as it's not the address that is exposed, it's the provenance. (`ptr.expose_addr()` stays unchanged as we haven't found a better option yet. The intended interpretation is "expose the provenance and return the address".)

The new name nicely matches `ptr::without_provenance`.
2024-04-02 20:37:39 -04:00
Michael Goulet
bda301ead8 Stop calling visitors V 2024-03-30 11:13:33 -04:00
Matthias Krüger
8d820c0c47
Rollup merge of #123188 - klensy:clippy-me2, r=Nilstrieb
compiler: fix few unused_peekable and needless_pass_by_ref_mut clippy lints

This fixes few instances of `unused_peekable` and `needless_pass_by_ref_mut`. While i expected to fix more warnings, `needless_pass_by_ref_mut` produced too much for one PR, so i stopped here.

Better reviewed commit by commit, as fixes splitted by chunks.
2024-03-29 15:17:11 +01:00
bors
45796d1c24 Auto merge of #123080 - Jules-Bertholet:mut-ref-mut, r=Nadrieril
Match ergonomics 2024: implement mutable by-reference bindings

Implements the mutable by-reference bindings portion of match ergonomics 2024 (#123076), with the `mut ref`/`mut ref mut` syntax, under feature gate `mut_ref`.

r? `@Nadrieril`

`@rustbot` label A-patterns A-edition-2024
2024-03-29 11:08:11 +00:00
klensy
9a6b3dfc06 and few more
maybe bug here?

warning: this argument is a mutable reference, but not used mutably
    --> compiler\rustc_borrowck\src\diagnostics\conflict_errors.rs:3857:35
     |
3857 |     pub(crate) fn emit(&self, cx: &mut MirBorrowckCtxt<'_, 'tcx>, diag: &mut Diag<'_>) -> String {
     |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&MirBorrowckCtxt<'_, 'tcx>`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut

warning: this argument is a mutable reference, but not used mutably
   --> compiler\rustc_borrowck\src\type_check\liveness\trace.rs:601:17
    |
601 |         typeck: &mut TypeChecker<'_, 'tcx>,
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&TypeChecker<'_, 'tcx>`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
2024-03-28 16:57:32 +03:00
bors
551abd65be Auto merge of #116891 - aliemjay:opaque-region-infer-rework-2, r=compiler-errors,oli-obk
rework opaque type region inference

User-facing changes are documented in [this comment](https://github.com/rust-lang/rust/pull/116891#issuecomment-1973774412).

The design document is in [this comment](https://github.com/rust-lang/rust/pull/116891#issuecomment-1836900102).

---

\- Fix Ice in check_unique; ICE -> Error; fixes #122782.
\- Ignore uncaptured lifetime args; ICE -> Pass; fixes #111906, fixes #110623, fixes #109059, fixes #122307
\- Except equal parameters from the uniqueness check; Pass -> Error; fixes #113916.
\- Check RPITs for invalid args; Pass -> Error; fixes #111935; ICE -> Error; fixes #110726.
\- Rework opaque types region inference; Pass -> Error; fixes #113971, fixes #112841.
\- Reject external lifetimes as invalid args; Pass -> Error; fixes #105498.

r? `@ghost`
2024-03-28 09:51:39 +00:00
Ali MJ Al-Nasrawy
7c6876f9a9 simplify check_unique 2024-03-28 06:00:26 +00:00
Ali MJ Al-Nasrawy
6b6ed2ea28 reject external lifetimes as invalid arguments 2024-03-28 06:00:26 +00:00
Ali MJ Al-Nasrawy
f4940e4d22 rework opaque types region inference 2024-03-28 06:00:26 +00:00
Ali MJ Al-Nasrawy
08c8caa524 convert all named regions in opaque types to nll vars
Do it in typeck before entering region inference routines
particularly because we will no longer be able to convert placeholders.
2024-03-28 06:00:26 +00:00
Ali MJ Al-Nasrawy
15c7e59c05 favor placeholders over existentials when choosing SCC representatives
... even when the existential has the least RegionVid.

universal regions (of root universe) > placeholders > existentials

The previous behavior, that chooses the minimal RegionVid index, naturally prefers universal regions over others
because they always have the least RegionVids, but there was no guranteed ordering between placeholders and existentials.
2024-03-28 06:00:26 +00:00
Ali MJ Al-Nasrawy
ce91e46a1e check RPITs for invalid args 2024-03-28 06:00:26 +00:00
Ali MJ Al-Nasrawy
c337825d6d ignore error params 2024-03-28 06:00:25 +00:00