Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[regression in 1.70] [clippy] redundant clone ; cloned value is neither consumed nor mutated #10870

Open
glandium opened this issue Jun 2, 2023 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@glandium
Copy link

glandium commented Jun 2, 2023

Code

#[derive(Clone)]
pub struct Foo(&'static str);

pub fn foo(a: Foo, b: Option<Foo>) -> String {
    let b = if let Some(b) = b { b } else { a.clone() };
    let mut result = String::new();
    result.push_str(a.0);
    result.push_str(b.0);
    result
}

Current output

warning: redundant clone
 --> src/lib.rs:5:46
  |
5 |     let b = if let Some(b) = b { b } else { a.clone() };
  |                                              ^^^^^^^^ help: remove this
  |
note: cloned value is neither consumed nor mutated
 --> src/lib.rs:5:45
  |
5 |     let b = if let Some(b) = b { b } else { a.clone() };
  |                                             ^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
  = note: `#[warn(clippy::redundant_clone)]` on by default

Desired output

Nothing

Rationale and extra context

This is what the compiler says without the clone:

error[[E0382]](https://doc.rust-lang.org/stable/error_codes/E0382.html): borrow of moved value: `a`
 --> src/lib.rs:7:21
  |
4 | pub fn foo(a: Foo, b: Option<Foo>) -> String {
  |            - move occurs because `a` has type `Foo`, which does not implement the `Copy` trait
5 |     let b = if let Some(b) = b { b } else { a };
  |                                             - value moved here
6 |     let mut result = String::new();
7 |     result.push_str(a.0);
  |                     ^^^ value borrowed here after move
  |
help: consider cloning the value if the performance cost is acceptable
  |
5 |     let b = if let Some(b) = b { b } else { a.clone() };
  |                                              ++++++++

Other cases

No response

Anything else?

No response

@matthiaskrgr matthiaskrgr transferred this issue from rust-lang/rust Jun 2, 2023
@jsdw
Copy link

jsdw commented Jun 2, 2023

I ran into a similar false positive; this snippet causes the clippy warning on the rust playground:

fn main() {
    #[derive(Clone, Debug, PartialEq)]
    struct Foo { n: usize }
    
    let mut foo = Foo { n: 1 };
    let foo2 = foo.clone(); // clippy: redundant clone for this
    
    foo.n = 2;
    assert_eq!(foo, foo2);
}

The code could be rejigged of course, but the clone here is clearly necessary since the other value is being mutated and we want to keep a copy of the original.

@Alexendoo
Copy link
Member

See also rust-lang/rust#108944 (comment)

bors added a commit that referenced this issue Jun 5, 2023
Move `redundant_clone` to `nursery`

changelog: [`redundant_clone`]: Move to `nursery`

A bunch of FPs in `redundant_clone` have sprung up after upstream MIR changes: rust-lang/rust#108944

- #10870
- #10577
- #10545
- #10517

r? `@flip1995`
@KisaragiEffective
Copy link
Contributor

KisaragiEffective commented Jun 15, 2023

@rustbot label I-false-positive C-bug

@rustbot rustbot added I-false-positive Issue: The lint was triggered on code it shouldn't have C-bug Category: Clippy is not doing the correct thing labels Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants