Skip to content

Commit

Permalink
Ignore not really redundant clones of ManuallyDrop
Browse files Browse the repository at this point in the history
"Redundant" clones of `ManuallyDrop` are sometimes used for the side effect of
invoking the clone, without running the drop implementation of the inner type.
In other words, they aren't really redundant. For example, futures-rs crate:

```rust
#[allow(clippy::redundant_clone)] // The clone here isn't actually redundant.
unsafe fn increase_refcount<T: ArcWake>(data: *const ()) {
    // Retain Arc, but don't touch refcount by wrapping in ManuallyDrop
    let arc = mem::ManuallyDrop::new(Arc::<T>::from_raw(data as *const T));
    // Now increase refcount, but don't drop new refcount either
    let _arc_clone: mem::ManuallyDrop<_> = arc.clone();
}
```

Ignore redundant clone lint for ManuallyDrop.
  • Loading branch information
tmiasko committed Jul 19, 2020
1 parent 8cf4219 commit a5cdd4a
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 10 deletions.
6 changes: 6 additions & 0 deletions clippy_lints/src/redundant_clone.rs
Expand Up @@ -124,6 +124,12 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
continue;
}

if let ty::Adt(ref def, _) = arg_ty.kind {
if match_def_path(cx, def.did, &paths::MEM_MANUALLY_DROP) {
continue;
}
}

// `{ cloned = &arg; clone(move cloned); }` or `{ cloned = &arg; to_path_buf(cloned); }`
let (cloned, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(cx, mir, arg, from_borrow, bb));

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Expand Up @@ -59,6 +59,7 @@ pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "Link
pub const LINT: [&str; 3] = ["rustc_session", "lint", "Lint"];
pub const MEM_DISCRIMINANT: [&str; 3] = ["core", "mem", "discriminant"];
pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"];
pub const MEM_MANUALLY_DROP: [&str; 4] = ["core", "mem", "manually_drop", "ManuallyDrop"];
pub const MEM_MAYBEUNINIT: [&str; 4] = ["core", "mem", "maybe_uninit", "MaybeUninit"];
pub const MEM_MAYBEUNINIT_UNINIT: [&str; 5] = ["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"];
pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"];
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/redundant_clone.fixed
Expand Up @@ -52,6 +52,7 @@ fn main() {
borrower_propagation();
not_consumed();
issue_5405();
manually_drop();
}

#[derive(Clone)]
Expand Down Expand Up @@ -170,3 +171,17 @@ fn issue_5405() {
let c: [usize; 2] = [2, 3];
let _d: usize = c[1].clone();
}

fn manually_drop() {
use std::mem::ManuallyDrop;
use std::sync::Arc;

let a = ManuallyDrop::new(Arc::new("Hello!".to_owned()));
let _ = a.clone(); // OK

let p: *const String = Arc::into_raw(ManuallyDrop::into_inner(a));
unsafe {
Arc::from_raw(p);
Arc::from_raw(p);
}
}
15 changes: 15 additions & 0 deletions tests/ui/redundant_clone.rs
Expand Up @@ -52,6 +52,7 @@ fn main() {
borrower_propagation();
not_consumed();
issue_5405();
manually_drop();
}

#[derive(Clone)]
Expand Down Expand Up @@ -170,3 +171,17 @@ fn issue_5405() {
let c: [usize; 2] = [2, 3];
let _d: usize = c[1].clone();
}

fn manually_drop() {
use std::mem::ManuallyDrop;
use std::sync::Arc;

let a = ManuallyDrop::new(Arc::new("Hello!".to_owned()));
let _ = a.clone(); // OK

let p: *const String = Arc::into_raw(ManuallyDrop::into_inner(a));
unsafe {
Arc::from_raw(p);
Arc::from_raw(p);
}
}
20 changes: 10 additions & 10 deletions tests/ui/redundant_clone.stderr
Expand Up @@ -108,61 +108,61 @@ LL | let _t = tup.0.clone();
| ^^^^^

error: redundant clone
--> $DIR/redundant_clone.rs:61:22
--> $DIR/redundant_clone.rs:62:22
|
LL | (a.clone(), a.clone())
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/redundant_clone.rs:61:21
--> $DIR/redundant_clone.rs:62:21
|
LL | (a.clone(), a.clone())
| ^

error: redundant clone
--> $DIR/redundant_clone.rs:121:15
--> $DIR/redundant_clone.rs:122:15
|
LL | let _s = s.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/redundant_clone.rs:121:14
--> $DIR/redundant_clone.rs:122:14
|
LL | let _s = s.clone();
| ^

error: redundant clone
--> $DIR/redundant_clone.rs:122:15
--> $DIR/redundant_clone.rs:123:15
|
LL | let _t = t.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/redundant_clone.rs:122:14
--> $DIR/redundant_clone.rs:123:14
|
LL | let _t = t.clone();
| ^

error: redundant clone
--> $DIR/redundant_clone.rs:132:19
--> $DIR/redundant_clone.rs:133:19
|
LL | let _f = f.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/redundant_clone.rs:132:18
--> $DIR/redundant_clone.rs:133:18
|
LL | let _f = f.clone();
| ^

error: redundant clone
--> $DIR/redundant_clone.rs:144:14
--> $DIR/redundant_clone.rs:145:14
|
LL | let y = x.clone().join("matthias");
| ^^^^^^^^ help: remove this
|
note: cloned value is neither consumed nor mutated
--> $DIR/redundant_clone.rs:144:13
--> $DIR/redundant_clone.rs:145:13
|
LL | let y = x.clone().join("matthias");
| ^^^^^^^^^
Expand Down

0 comments on commit a5cdd4a

Please sign in to comment.