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

Clippy unexpectedly suggests that clone is redundant #112227

Closed
foresterre opened this issue Jun 2, 2023 · 1 comment
Closed

Clippy unexpectedly suggests that clone is redundant #112227

foresterre opened this issue Jun 2, 2023 · 1 comment
Labels
C-bug Category: This is a bug.

Comments

@foresterre
Copy link
Contributor

foresterre commented Jun 2, 2023

Excerpt of original output of cargo clippy --fix in console-rs/indicatif#547, which suggested to create an issue here:

Checking indicatif v0.17.4 (C:\foresterre\indicatif)
warning: failed to automatically apply fixes suggested by rustc to crate `multi_autodrop`                                                                                

after fixes were automatically applied the compiler reported errors within these files:

  * tests\multi-autodrop.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

indicatif pipeline

I tried this code (can probably be made more minimal):

use std::sync::{atomic::AtomicU32, atomic::Ordering, Arc, Mutex, RwLock};

// Reproduction derived from indicatif::MultiProgress and ProgressBar where this Clippy suggestion was found.
// With Clippy 0.1.71 (2023-06-01 d59363a)
// With Rust 1.70.0

#[derive(Clone)]
struct MultiProgress {
    state: Arc<RwLock<State>>,
}

impl Default for MultiProgress {
    fn default() -> Self {
        Self {
            state: Arc::new(RwLock::new(State)),
        }
    }
}

impl MultiProgress {
    fn add(&self, pb: ProgressBar) -> ProgressBar {
        let state = self.state.write().unwrap();
        drop(state);

        pb.set_remote(self.state.clone());
        pb
    }
}

#[derive(Clone)]
struct ProgressBar {
    value: Arc<AtomicU32>,
    remote: Arc<Mutex<Arc<RwLock<State>>>>,
}

impl Default for ProgressBar {
    fn default() -> Self {
        Self {
            value: Arc::new(AtomicU32::new(0)),
            remote: Arc::new(Mutex::new(Arc::new(RwLock::new(State)))),
        }
    }
}

impl ProgressBar {
    fn set_remote(&self, state: Arc<RwLock<State>>) {
        *self.remote.lock().unwrap() = state;
    }

    fn inc(&self, delta: u32) {
        self.value.fetch_add(delta, Ordering::SeqCst);
    }

    fn reset(&self) {
        self.value.fetch_add(0, Ordering::SeqCst);
    }
}

struct State;

fn main() {
    let pb = {
        let m = MultiProgress::default();
        m.add(ProgressBar::default())
        // m is dropped here
    };

    {
        // Clippy faults here: it suggests to remove the clone.
        let pb2 = pb.clone();
        for _ in 0..10 {
            pb2.inc(1);
        }
    }

    pb.reset();
}

Running cargo clippy:

hecking playground v0.0.1 (/playground)
warning: redundant clone
  --> src/main.rs:70:21
   |
70 |         let pb2 = pb.clone();
   |                     ^^^^^^^^ help: remove this
   |
note: cloned value is neither consumed nor mutated
  --> src/main.rs:70:19
   |
70 |         let pb2 = pb.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

warning: `playground` (bin "playground") generated 1 warning (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.42s

When applying the suggestion:

 Checking playground v0.0.1 (/playground)
error[E0382]: borrow of moved value: `pb`
  --> src/main.rs:76:5
   |
62 |     let pb = {
   |         -- move occurs because `pb` has type `ProgressBar`, which does not implement the `Copy` trait
...
70 |         let pb2 = pb;
   |                   -- value moved here
...
76 |     pb.reset();
   |     ^^^^^^^^^^ value borrowed here after move
   |
help: consider cloning the value if the performance cost is acceptable
   |
70 |         let pb2 = pb.clone();
   |                     ++++++++

For more information about this error, try `rustc --explain E0382`.
error: could not compile `playground` (bin "playground") due to previous error

Meta

rustc --version --verbose:

rustc 1.70.0 (90c541806 2023-05-31)
binary: rustc
commit-hash: 90c541806f23a127002de5b4038be731ba1458ca
commit-date: 2023-05-31
host: x86_64-pc-windows-msvc
release: 1.70.0
LLVM version: 16.0.2

cargo clippy --version --verbose:

clippy 0.1.70 (90c54180 2023-05-31)

Extra

Playground
Gist
Clippy lint

@foresterre
Copy link
Contributor Author

Whoops, wrong repo (altough cargo clippy --fix does suggest creating an issue here). Moved to comment on equivalent issue: rust-lang/rust-clippy#10577 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

1 participant