Tweak `.clone()` suggestion to work in more cases #118076

estebank · 2023-11-19T23:53:45Z

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() {}
   |                  ++++++++++++++++++++++++++++++            +

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.

When encountering multiple mutable borrows, suggest cloning and adding
derive annotations as needed.

error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference
  --> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9
   |
LL |     foo(&mut sm.x);
   |         ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable
   |
help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str`
  --> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21
   |
LL |     let mut sm = sr.clone();
   |                     ^^^^^^^
help: consider annotating `Str` with `#[derive(Clone)]`
   |
LL + #[derive(Clone)]
LL | struct Str {
   |
help: consider specifying this binding's type
   |
LL |     let mut sm: &mut Str = sr.clone();
   |               ++++++++++

Fix #34629. Fix #76643. Fix #91532.

rustbot · 2023-11-19T23:53:53Z

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

davidtwco

Apologies for the delay in getting to this, LGTM.

davidtwco · 2023-12-04T14:13:52Z

@bors r+

bors · 2023-12-04T14:13:55Z

📌 Commit d42d3fd has been approved by davidtwco

It is now in the queue for this repository.

bors · 2023-12-04T14:13:56Z

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

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() {} | ++++++++++++++++++++++++++++++ + ``` 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 rust-lang#109429. When encountering multiple mutable borrows, suggest cloning and adding derive annotations as needed. ``` error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference --> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9 | LL | foo(&mut sm.x); | ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str` --> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21 | LL | let mut sm = sr.clone(); | ^^^^^^^ help: consider annotating `Str` with `#[derive(Clone)]` | LL + #[derive(Clone)] LL | struct Str { | help: consider specifying this binding's type | LL | let mut sm: &mut Str = sr.clone(); | ++++++++++ ``` Fix rust-lang#34629. Fix rust-lang#76643. Fix rust-lang#91532.

…llaumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#118076 (Tweak `.clone()` suggestion to work in more cases) - rust-lang#118508 (rustdoc: do not escape quotes in body text) - rust-lang#118565 (interpret: make numeric_intrinsic accessible from Miri) - rust-lang#118591 (portable-simd: fix test suite build) - rust-lang#118600 ([rustdoc] Don't generate the "Fields" heading if there is no field displayed) r? `@ghost` `@rustbot` modify labels: rollup

GuillaumeGomez · 2023-12-04T19:45:24Z

I think this PR failed in #118617 (comment).

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 rust-lang#109429.

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 rust-lang#109429.

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 # rust-lang#41825.

When encountering multiple mutable borrows, suggest cloning and adding derive annotations as needed. ``` error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference --> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9 | LL | foo(&mut sm.x); | ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str` --> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21 | LL | let mut sm = sr.clone(); | ^^^^^^^ help: consider annotating `Str` with `#[derive(Clone)]` | LL + #[derive(Clone)] LL | struct Str { | help: consider specifying this binding's type | LL | let mut sm: &mut Str = sr.clone(); | ++++++++++ ``` ``` error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference --> $DIR/issue-91206.rs:14:5 | LL | inner.clear(); | ^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior --> $DIR/issue-91206.rs:11:17 | LL | let inner = client.get_inner_ref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider specifying this binding's type | LL | let inner: &mut Vec<usize> = client.get_inner_ref(); | +++++++++++++++++ ```

estebank · 2023-12-04T22:01:07Z

@bors r=davidtwco

bors · 2023-12-04T22:01:09Z

📌 Commit a8faa82 has been approved by davidtwco

It is now in the queue for this repository.

bors · 2023-12-05T06:34:47Z

⌛ Testing commit a8faa82 with merge f67523d...

bors · 2023-12-05T08:32:21Z

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing f67523d to master...

rust-timer · 2023-12-05T10:08:23Z

Finished benchmarking commit (f67523d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.7%	[-1.7%, -1.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.7%	[-1.7%, -1.7%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	6.1%	[6.1%, 6.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.321s -> 674.295s (-0.15%)
Artifact size: 314.15 MiB -> 314.16 MiB (0.00%)

rustbot assigned davidtwco Nov 19, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 19, 2023

This comment has been minimized.

Sign in to view

davidtwco approved these changes Dec 4, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2023

GuillaumeGomez mentioned this pull request Dec 4, 2023

Rollup of 5 pull requests #118617

Closed

estebank added 8 commits December 4, 2023 21:54

Tweak output on specific case

90db536

Mark more tests as run-rustfix

1c69c6a

Deduplicate some logic

210a672

Fix rebase

a8faa82

estebank force-pushed the issue-109429 branch from d42d3fd to a8faa82 Compare December 4, 2023 22:00

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 5, 2023

bors merged commit f67523d into rust-lang:master Dec 5, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Dec 5, 2023

bors mentioned this pull request Dec 5, 2023

Remove "consider specifying this binding's type" when reference differs in mutability #115648

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak `.clone()` suggestion to work in more cases #118076

Tweak `.clone()` suggestion to work in more cases #118076

estebank commented Nov 19, 2023 •

edited

Loading

rustbot commented Nov 19, 2023

This comment has been minimized.

davidtwco left a comment

davidtwco commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 4, 2023

GuillaumeGomez commented Dec 4, 2023

estebank commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 5, 2023

bors commented Dec 5, 2023

rust-timer commented Dec 5, 2023

Tweak .clone() suggestion to work in more cases #118076

Tweak .clone() suggestion to work in more cases #118076

Conversation

estebank commented Nov 19, 2023 • edited Loading

rustbot commented Nov 19, 2023

This comment has been minimized.

davidtwco left a comment

Choose a reason for hiding this comment

davidtwco commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 4, 2023

GuillaumeGomez commented Dec 4, 2023

estebank commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 5, 2023

bors commented Dec 5, 2023

rust-timer commented Dec 5, 2023

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Tweak `.clone()` suggestion to work in more cases #118076

Tweak `.clone()` suggestion to work in more cases #118076

estebank commented Nov 19, 2023 •

edited

Loading