Commit Graph

1604 Commits

Author SHA1 Message Date
Esteban Küber
64a4cdcfd4 review comment: rename method 2024-04-25 18:26:36 +00:00
Esteban Küber
abdb64d4ea Check equivalence of indices in more cases 2024-04-25 16:55:33 +00:00
Esteban Küber
ad6ae61246 Don't suggest split_at_mut when the multiple borrows have the same index 2024-04-25 16:55:33 +00:00
Esteban Küber
9f9f0aa534 Mention split_at_mut when mixing mutability in indexing ops
Emit suggestion when encountering

```rust
let a = &mut foo[0];
let b = &foo[1];
a.use_mut();
```
2024-04-25 16:55:33 +00:00
Esteban Küber
dbaa4e2148 Only suggest split_at_mut on indexing borrowck errors for std types 2024-04-25 16:55:32 +00:00
Esteban Küber
386236f289 Detect borrow error involving sub-slices and suggest split_at_mut
```
error[E0499]: cannot borrow `foo` as mutable more than once at a time
  --> $DIR/suggest-split-at-mut.rs:13:18
   |
LL |     let a = &mut foo[..2];
   |                  --- first mutable borrow occurs here
LL |     let b = &mut foo[2..];
   |                  ^^^ second mutable borrow occurs here
LL |     a[0] = 5;
   |     ---- first borrow later used here
   |
   = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices
```

Address most of #58792.

For follow up work, we should emit a structured suggestion for cases where we can identify the exact `let (a, b) = foo.split_at_mut(2);` call that is needed.
2024-04-25 16:55:32 +00:00
bors
31e6e8c6c5 Auto merge of #119650 - chenyukang:yukang-fix-118596-ref-mut, r=wesleywiser
Suggest ref mut for pattern matching assignment

Fixes #118596
2024-04-25 08:52:19 +00:00
Esteban Küber
ad9a5a5f9f Suggest cloning captured binding in move closure
```
error[E0507]: cannot move out of `bar`, a captured variable in an `FnMut` closure
  --> $DIR/borrowck-move-by-capture.rs:9:29
   |
LL |     let bar: Box<_> = Box::new(3);
   |         --- captured outer variable
LL |     let _g = to_fn_mut(|| {
   |                        -- captured by this `FnMut` closure
LL |         let _h = to_fn_once(move || -> isize { *bar });
   |                             ^^^^^^^^^^^^^^^^   ----
   |                             |                  |
   |                             |                  variable moved due to use in closure
   |                             |                  move occurs because `bar` has type `Box<isize>`, which does not implement the `Copy` trait
   |                             `bar` is moved here
   |
help: clone the value before moving it into the closure
   |
LL ~         let value = bar.clone();
LL ~         let _h = to_fn_once(move || -> isize { value });
   |
```
2024-04-24 22:21:16 +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
Esteban Küber
4aba2c55e6 Modify find_expr from Span to better account for closures
Start pointing to where bindings were declared when they are captured in closures:

```
error[E0597]: `x` does not live long enough
  --> $DIR/suggest-return-closure.rs:23:9
   |
LL |     let x = String::new();
   |         - binding `x` declared here
...
LL |     |c| {
   |     --- value captured here
LL |         x.push(c);
   |         ^ borrowed value does not live long enough
...
LL | }
   | -- borrow later used here
   | |
   | `x` dropped here while still borrowed
```

Suggest cloning in more cases involving closures:

```
error[E0507]: cannot move out of `foo` in pattern guard
  --> $DIR/issue-27282-move-ref-mut-into-guard.rs:11:19
   |
LL |             if { (|| { let mut bar = foo; bar.take() })(); false } => {},
   |                   ^^                 --- move occurs because `foo` has type `&mut Option<&i32>`, which does not implement the `Copy` trait
   |                   |
   |                   `foo` is moved here
   |
   = note: variables bound in patterns cannot be moved from until after the end of the pattern guard
help: consider cloning the value if the performance cost is acceptable
   |
LL |             if { (|| { let mut bar = foo.clone(); bar.take() })(); false } => {},
   |                                         ++++++++
```
2024-04-24 22:21:13 +00:00
yukang
be9dbe9102 Suggest ref mut for pattern matching assignment 2024-04-25 04:54:25 +08:00
León Orell Valerian Liehr
332cac2c6d
Rollup merge of #122598 - Nadrieril:full-derefpats, r=matthewjasper
deref patterns: lower deref patterns to MIR

This lowers deref patterns to MIR. This is a bit tricky because this is the first kind of pattern that requires storing a value in a temporary. Thanks to https://github.com/rust-lang/rust/pull/123324 false edges are no longer a problem.

The thing I'm not confident about is the handling of fake borrows. This PR ignores any fake borrows inside a deref pattern. We are guaranteed to at least fake borrow the place of the first pointer value, which could be enough, but I'm not certain.
2024-04-23 17:25:15 +02:00
Guillaume Gomez
e984447405
Rollup merge of #124183 - compiler-errors:unnecessary-by-ref, r=oli-obk
Stop taking `ParamTy`/`ParamConst`/`EarlyParamRegion`/`AliasTy` by ref

It's unnecessary and is annoying when we have it by value.
2024-04-22 20:25:59 +02:00
Scott McMurray
e6b2b764ec Add AggregateKind::RawPtr and enough support to compile 2024-04-21 11:08:37 -07:00
Nadrieril
50531806ee Add a non-shallow fake borrow 2024-04-20 16:01:35 +02:00
Michael Goulet
86756c1804 Stop taking ParamTy/ParamConst/EarlyParamRegion/AliasTy by ref 2024-04-19 21:09:51 -04:00
Jules Bertholet
2a4624ddd1
Rename BindingAnnotation to BindingMode 2024-04-17 09:34:39 -04:00
bors
3fba278231 Auto merge of #123537 - compiler-errors:shallow, r=lcnr
Simplify shallow resolver to just fold ty/consts

Probably faster than using a whole folder?
2024-04-16 21:59:36 +00:00
Guillaume Gomez
24cdb7ed56
Rollup merge of #123990 - compiler-errors:suggest-closure-ret-deref, r=oli-obk
Make `suggest_deref_closure_return` more idiomatic/easier to understand

The only functional change here really is just making it not use a fresh type variable for upvars. I'll point that out in the code.

The rest of the changes are just stylistic, because reading this code was really confusing me (variable names were vague, ways of accessing types were unidiomatic, order of operations was kind of strange, etc).

This is stacked on #123989.

r? oli-obk since you approved #122213
2024-04-16 15:19:14 +02:00
Guillaume Gomez
9da8f04da3
Rollup merge of #123797 - amandasystems:better-graphviz, r=oli-obk
Better graphviz output for SCCs and NLL constraints

This PR modifies the output for `-Z dump-mir-graphviz=yes`. Specifically, it changes the output of the files `.-------.nll.0.regioncx.all.dot` and `nll.0.regioncx.scc.dot` to be easier to read and contain some information that helped me during debugging. In particular:

- SCC indices are contracted to `SCC(n)` instead of `ConstraintSccIndex(n)` to compress the nodes
- SCC regions are in `{}` rather than `[]` (controversial since they are technically ordered by index, but I figured they're more sets than arrays conceptually since they're equivalence classes).
- For regions in other universes than the root, also show the region universe (as ?8/U1)
- For regions with external names, show the external name in parenthesis
- For the region graph where edges are locations, render the All variant of the enum without the file since it's extremely long and often destroys the rendering
- For region graph edge annotations for single locations, remove the wrapping around the Location variant and just add its contents since this can be unambiguously done

Example output (from the function `foo()` of `tests/ui/error-codes/E0582.rs`) for an SCC graph:
![a graph showing SCCs](https://github.com/rust-lang/rust/assets/102855/0b998338-0379-4829-b99e-d8105c094897)

...and for the constraints:
![a graph showing regions and their constraints](https://github.com/rust-lang/rust/assets/102855/e984c4ca-7aa2-4db2-9878-bf38fe8208d5)

This PR also gives `UniverseIndex`es the `is_root()` method since this is now an operation that happens three times in the borrowck crate.
2024-04-16 15:19:14 +02:00
Michael Goulet
00838b3140 Make suggest_deref_closure_return more idiomatic/easier to understand 2024-04-15 23:04:16 -04:00
León Orell Valerian Liehr
daa2ebc70c
Rollup merge of #123989 - compiler-errors:type-dependent-def-id, r=oli-obk
Just use `type_dependent_def_id` to figure out what the method is for an expr

The calls to `lookup_method_for_diagnostic` are overkill.

r? oli-obk
2024-04-16 01:12:39 +02:00
León Orell Valerian Liehr
9cc26b598a
Rollup merge of #123016 - compiler-errors:no-type-var-origin, r=lcnr
Remove `TypeVariableOriginKind` and `ConstVariableOriginKind`

It's annoying to have to import `TypeVariableOriginKind` just to fill it with `MiscVariable` for almost every use. Every other usage other than `TypeParameterDefinition` wasn't even used -- I can see how it may have been useful once for debugging, but I do quite a lot of typeck debugging and I've never really needed it.

So let's just remove it, and keep around the only useful thing which is the `DefId` of the param for `var_for_def`.

This is based on #123006, which removed the special use of `TypeVariableOriginKind::OpaqueInference`, which I'm pretty sure I was the one that added.

r? lcnr or re-roll to types
2024-04-16 01:12:36 +02:00
Michael Goulet
ecef296a03 Simplify shallow resolver to just fold ty/consts 2024-04-15 18:09:16 -04:00
Michael Goulet
98890be9c4 Rebase fallout 2024-04-15 17:37:56 -04:00
Michael Goulet
05bb3d2683 Just use type_dependent_def_id to figure out what the method is for an expr 2024-04-15 17:14:59 -04:00
Michael Goulet
34bce07e8e Remove TypeVariableOriginKind 2024-04-15 16:51:50 -04:00
Michael Goulet
20a5fb3b4a
Rollup merge of #123924 - compiler-errors:tuple-sugg, r=estebank
Fix various bugs in `ty_kind_suggestion`

Consolidates two implementations of `ty_kind_suggestion`
Fixes some misuse of the empty param-env
Fixes a problem where we suggested `(42)` instead of `(42,)` for tuple suggestions
Suggest a value when `return;`, making it consistent with `break;`
Fixes #123906
2024-04-15 15:18:06 -04:00
许杰友 Jieyou Xu (Joe)
5580ae9795
Rollup merge of #123934 - WaffleLapkin:graph-mini-refactor, r=fmease
`rustc_data_structures::graph` mini refactor

Who doesn't love to breathe dust from the ancient times?
2024-04-15 16:56:18 +01:00
Maybe Waffle
435db9b9bd Use RPITIT for Successors and Predecessors traits
Now with RPITIT instead of GAT!
2024-04-15 13:34:08 +00:00
Maybe Waffle
e8d2221e3b Make depth_first_search into a standalone function
Does not necessarily change much, but we never overwrite it, so I see no reason
for it to be in the `Successors` trait. (+we already have a similar `is_cyclic`)
2024-04-14 16:03:08 +00:00
Maybe Waffle
0d5fc9bf58 Merge {With,Graph}{Successors,Predecessors} into {Successors,Predecessors}
Now with GAT!
2024-04-14 15:48:53 +00:00
Maybe Waffle
398da593a5 Merge WithNumNodes into DirectedGraph 2024-04-14 15:46:40 +00:00
Michael Goulet
d6ac50e547 Consolidate two copies of ty_kind_suggestion 2024-04-14 09:42:53 -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
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