From a5cdd4aeb11fad6b0bf73d342398700a27c4484b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Mon, 20 Jul 2020 00:00:00 +0000 Subject: [PATCH] Ignore not really redundant clones of ManuallyDrop "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(data: *const ()) { // Retain Arc, but don't touch refcount by wrapping in ManuallyDrop let arc = mem::ManuallyDrop::new(Arc::::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. --- clippy_lints/src/redundant_clone.rs | 6 +++++ clippy_lints/src/utils/paths.rs | 1 + tests/ui/redundant_clone.fixed | 15 ++++++++++++ tests/ui/redundant_clone.rs | 15 ++++++++++++ tests/ui/redundant_clone.stderr | 38 ++++++++++++++--------------- 5 files changed, 56 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index fda7480194d..7932be0d4b1 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -124,6 +124,12 @@ fn check_fn( 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)); diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 4c3462802e9..a515ee29c82 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -59,6 +59,7 @@ 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"]; diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed index 764c10a6d39..cdeefda4c23 100644 --- a/tests/ui/redundant_clone.fixed +++ b/tests/ui/redundant_clone.fixed @@ -52,6 +52,7 @@ fn main() { borrower_propagation(); not_consumed(); issue_5405(); + manually_drop(); } #[derive(Clone)] @@ -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); + } +} diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs index 839747b131d..acb7ffb305f 100644 --- a/tests/ui/redundant_clone.rs +++ b/tests/ui/redundant_clone.rs @@ -52,6 +52,7 @@ fn main() { borrower_propagation(); not_consumed(); issue_5405(); + manually_drop(); } #[derive(Clone)] @@ -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); + } +} diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr index eced198283c..89b39254299 100644 --- a/tests/ui/redundant_clone.stderr +++ b/tests/ui/redundant_clone.stderr @@ -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 - | -LL | let _s = s.clone(); - | ^^^^^^^^ help: remove this - | -note: this value is dropped without further use - --> $DIR/redundant_clone.rs:121:14 - | -LL | let _s = s.clone(); - | ^ - error: redundant clone --> $DIR/redundant_clone.rs:122:15 | -LL | let _t = t.clone(); +LL | let _s = s.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use --> $DIR/redundant_clone.rs:122:14 | +LL | let _s = s.clone(); + | ^ + +error: redundant clone + --> $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: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"); | ^^^^^^^^^