False positive: redundant_clone #10577
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
glittershark
added
C-bug
|
Narrowing down the range, I don't think this triggered as recently as |
ilyagr
added a commit
to ilyagr/jj
that referenced
this issue
Apr 1, 2023
ilyagr
added a commit
to ilyagr/jj
that referenced
this issue
Apr 1, 2023
ilyagr
added a commit
to ilyagr/jj
that referenced
this issue
Apr 1, 2023
ilyagr
added a commit
to martinvonz/jj
that referenced
this issue
Apr 2, 2023
ilyagr
added a commit
to ilyagr/jj
that referenced
this issue
Apr 2, 2023
|
To help narrow down the range — the issue started sometime between 2023-03-16 and 2023-03-18. The last successful build for me was on 16 March 2023, and the first failed one was on 18 March. |
|
FWIW I bumped into this too with this code in zbus. |
|
We are hitting this as well on the https://github.com/solana-labs/solana repo when trying to upgrade our nightly version to 2023-04-15. (Our nightly is currently on 2023-03-04.) Here's the full (long!) output |
|
hey, i created minimal test case: use std::sync::Arc;
#[derive(Clone)]
struct Account {
data: Arc<usize>,
}
fn take<T>(_a: &T) {
}
fn main() {
let account = Account {data: Arc::new(30)};
let data = account.data.clone();
take(&account);
take(&data);
} |
|
also, i further narrowed down the regression window (thank to the prior work: #10577 (comment)): so, it looks like the regression window is quite narrow: rust-lang/rust@ab65486...511364e this is a hunch but i suspect rust-lang/rust#108944 is the culprit, flipping this line: |
This was referenced May 18, 2023
This was referenced May 26, 2023
|
It looks like this bug is now in stable 1.70, I'm getting errors in https://github.com/printfn/fend. |
|
I'm also seeing this error, with an even simpler minimal test case than was produced above (no need for |
|
yeah, fyi, I confirmed that the root cause is rustc's internal changes (ref: rust-lang/rust#108944 (comment)). hehe, as i guessed in the post, this is getting more attention because now 1.70 is shipped. ;) according to rust-lang/rust#108944 (comment), it seems clippy needs to be fixed for its misuse somehow... fyi, i only bisected as a normal rust community member, currently not actively trying fix the clippy. |
|
Another instance in indicatif: rust-lang/rust#112227 Excerpt of original output of 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 When applying the suggestion: Meta
Extra |
|
Another set of instances is in my crate |
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`
acheroncrypto
added a commit
to acheroncrypto/anchor
that referenced
this issue
Jul 11, 2023
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
I looked and this didn't look like any of the existing
redundant_clonefalse positives. Sometime betweennightly-2022-10-01andnightly-2023-03-18(sorry for the wide range)clippy::redundant_clonestarted false-positive firing here. If thatclone()call is removed, compilation fails due toindex_onbeing used hereLint Name
clippy::redundant_clone
Reproducer
See links
Version
Additional Labels
No response
The text was updated successfully, but these errors were encountered: