about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPhil Hansch <dev@phansch.net>2019-10-04 08:08:58 +0200
committerGitHub <noreply@github.com>2019-10-04 08:08:58 +0200
commit8d2912ec00a42698e44db924495bcc30a090ee2b (patch)
treef5fd4950c2ae59975eff323e6191578a74564e12
parentb824f021d61ddee59695f20c7128c39d80e9e5b6 (diff)
parent4cded6d9012bbfc77d13c7ef2a413402199a2a03 (diff)
downloadrust-8d2912ec00a42698e44db924495bcc30a090ee2b.tar.gz
rust-8d2912ec00a42698e44db924495bcc30a090ee2b.zip
Rollup merge of #4509 - sinkuu:redundant_clone_fix, r=llogiq
Fix false-positive of redundant_clone and move to clippy::perf

This PR introduces dataflow analysis to `redundant_clone` lint to filter out borrowed variables, which had been incorrectly detected.

Depends on https://github.com/rust-lang/rust/pull/64207.

changelog: Moved `redundant_clone` lint to `perf` group

# What this lint catches

## `clone`/`to_owned`

```rust
let s = String::new();
let t = s.clone();
```

```rust
// MIR
_1 = String::new();
_2 = &_1;
_3 = clone(_2); // (*)
```

We can turn this `clone` call into a move if

1. `_2` is the sole borrow of `_1` at the statement `(*)`
2. `_1` is not used hereafter

## `Deref` + type-specific `to_owned` method

```rust
let s = std::path::PathBuf::new();
let t = s.to_path_buf();
```

```rust
// MIR
_1 = PathBuf::new();
_2 = &1;
_3 = call deref(_2);
_4 = _3;                         // Copies borrow
StorageDead(_2);
_5 = Path::to_path_buf(_4); // (*)
```

We can turn this `to_path_buf` call into a move if

1. `_3` `_4` are the sole borrow of `_1` at `(*)`
2. `_1` is not used hereafter

# What this PR introduces

1. `MaybeStorageLive` that determines whether a local lives at a particular location
2. `PossibleBorrowerVisitor` that constructs [`TransitiveRelation`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/transitive_relation/struct.TransitiveRelation.html) of possible borrows, e.g. visiting `_2 = &1; _3 = &_2:` will result in `_3 -> _2 -> _1` relation. Then `_3` and `_2` will be counted as possible borrowers of `_1` in the sole-borrow analysis above.
-rw-r--r--clippy_dev/src/fmt.rs5
-rw-r--r--clippy_lints/src/booleans.rs2
-rw-r--r--clippy_lints/src/lib.rs5
-rw-r--r--clippy_lints/src/redundant_clone.rs423
-rw-r--r--src/lintlist/mod.rs2
-rw-r--r--tests/compile-test.rs3
-rw-r--r--tests/ui/crashes/auxiliary/proc_macro_crash.rs2
-rw-r--r--tests/ui/escape_analysis.rs7
-rw-r--r--tests/ui/escape_analysis.stderr6
-rw-r--r--tests/ui/map_clone.fixed2
-rw-r--r--tests/ui/map_clone.rs2
-rw-r--r--tests/ui/needless_pass_by_value.rs3
-rw-r--r--tests/ui/needless_pass_by_value.stderr52
-rw-r--r--tests/ui/ptr_arg.rs2
-rw-r--r--tests/ui/question_mark.rs2
-rw-r--r--tests/ui/redundant_clone.fixed132
-rw-r--r--tests/ui/redundant_clone.rs98
-rw-r--r--tests/ui/redundant_clone.stderr150
-rw-r--r--tests/ui/swap.rs8
-rw-r--r--tests/ui/swap.stderr14
-rw-r--r--tests/ui/unnecessary_clone.rs2
21 files changed, 717 insertions, 205 deletions
diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs
index 1f4925db1be..3c90aa4b507 100644
--- a/clippy_dev/src/fmt.rs
+++ b/clippy_dev/src/fmt.rs
@@ -100,10 +100,7 @@ pub fn run(check: bool, verbose: bool) {
 }
 
 fn format_command(program: impl AsRef<OsStr>, dir: impl AsRef<Path>, args: &[impl AsRef<OsStr>]) -> String {
-    let arg_display: Vec<_> = args
-        .iter()
-        .map(|a| escape(a.as_ref().to_string_lossy()).to_owned())
-        .collect();
+    let arg_display: Vec<_> = args.iter().map(|a| escape(a.as_ref().to_string_lossy())).collect();
 
     format!(
         "cd {} && {} {}",
diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs
index 4309eaa7879..c5da0af6f4d 100644
--- a/clippy_lints/src/booleans.rs
+++ b/clippy_lints/src/booleans.rs
@@ -343,7 +343,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
 
             let stats = terminal_stats(&expr);
             let mut simplified = expr.simplify();
-            for simple in Bool::Not(Box::new(expr.clone())).simplify() {
+            for simple in Bool::Not(Box::new(expr)).simplify() {
                 match simple {
                     Bool::Not(_) | Bool::True | Bool::False => {},
                     _ => simplified.push(Bool::Not(Box::new(simple.clone()))),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 10a14f3b906..490f47424b1 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -27,6 +27,8 @@ extern crate rustc_driver;
 #[allow(unused_extern_crates)]
 extern crate rustc_errors;
 #[allow(unused_extern_crates)]
+extern crate rustc_index;
+#[allow(unused_extern_crates)]
 extern crate rustc_mir;
 #[allow(unused_extern_crates)]
 extern crate rustc_target;
@@ -864,6 +866,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         ranges::RANGE_MINUS_ONE,
         ranges::RANGE_PLUS_ONE,
         ranges::RANGE_ZIP_WITH_LEN,
+        redundant_clone::REDUNDANT_CLONE,
         redundant_field_names::REDUNDANT_FIELD_NAMES,
         redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
         redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
@@ -1169,6 +1172,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         methods::SINGLE_CHAR_PATTERN,
         misc::CMP_OWNED,
         mutex_atomic::MUTEX_ATOMIC,
+        redundant_clone::REDUNDANT_CLONE,
         slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
         trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF,
         types::BOX_VEC,
@@ -1188,7 +1192,6 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         mutex_atomic::MUTEX_INTEGER,
         needless_borrow::NEEDLESS_BORROW,
         path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE,
-        redundant_clone::REDUNDANT_CLONE,
     ]);
 }
 
diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs
index 09a55d26424..478ca2e04ff 100644
--- a/clippy_lints/src/redundant_clone.rs
+++ b/clippy_lints/src/redundant_clone.rs
@@ -9,12 +9,16 @@ use rustc::hir::{def_id, Body, FnDecl, HirId};
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::mir::{
     self, traversal,
-    visit::{MutatingUseContext, PlaceContext, Visitor},
-    TerminatorKind,
+    visit::{MutatingUseContext, PlaceContext, Visitor as _},
 };
-use rustc::ty::{self, Ty};
+use rustc::ty::{self, fold::TypeVisitor, Ty};
 use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_data_structures::{fx::FxHashMap, transitive_relation::TransitiveRelation};
 use rustc_errors::Applicability;
+use rustc_index::bit_set::{BitSet, HybridBitSet};
+use rustc_mir::dataflow::{
+    do_dataflow, BitDenotation, BottomValue, DataflowResults, DataflowResultsCursor, DebugFormatted, GenKillSet,
+};
 use std::convert::TryFrom;
 use syntax::source_map::{BytePos, Span};
 
@@ -36,17 +40,7 @@ declare_clippy_lint! {
     ///
     /// **Known problems:**
     ///
-    /// * Suggestions made by this lint could require NLL to be enabled.
-    /// * False-positive if there is a borrow preventing the value from moving out.
-    ///
-    /// ```rust
-    /// # fn foo(x: String) {}
-    /// let x = String::new();
-    ///
-    /// let y = &x;
-    ///
-    /// foo(x.clone()); // This lint suggests to remove this `clone()`
-    /// ```
+    /// False-negatives: analysis performed by this lint is conservative and limited.
     ///
     /// **Example:**
     /// ```rust
@@ -68,7 +62,7 @@ declare_clippy_lint! {
     /// Path::new("/a/b").join("c").to_path_buf();
     /// ```
     pub REDUNDANT_CLONE,
-    nursery,
+    perf,
     "`clone()` of an owned value that is going to be dropped immediately"
 }
 
@@ -88,6 +82,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
         let def_id = cx.tcx.hir().body_owner_def_id(body.id());
         let mir = cx.tcx.optimized_mir(def_id);
 
+        let dead_unwinds = BitSet::new_empty(mir.basic_blocks().len());
+        let maybe_storage_live_result = do_dataflow(
+            cx.tcx,
+            mir,
+            def_id,
+            &[],
+            &dead_unwinds,
+            MaybeStorageLive::new(mir),
+            |bd, p| DebugFormatted::new(&bd.body.local_decls[p]),
+        );
+        let mut possible_borrower = {
+            let mut vis = PossibleBorrowerVisitor::new(cx, mir);
+            vis.visit_body(mir);
+            vis.into_map(cx, maybe_storage_live_result)
+        };
+
         for (bb, bbdata) in mir.basic_blocks().iter_enumerated() {
             let terminator = bbdata.terminator();
 
@@ -114,29 +124,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
                 continue;
             }
 
-            // _1 in MIR `{ _2 = &_1; clone(move _2); }` or `{ _2 = _1; to_path_buf(_2); } (from_deref)
-            // In case of `from_deref`, `arg` is already a reference since it is `deref`ed in the previous
-            // block.
-            let (cloned, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(
-                cx,
-                mir,
-                arg,
-                from_borrow,
-                bbdata.statements.iter()
-            ));
-
-            if from_borrow && cannot_move_out {
-                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));
+
+            let loc = mir::Location {
+                block: bb,
+                statement_index: bbdata.statements.len(),
+            };
+
+            // Cloned local
+            let local = if from_borrow {
+                // `res = clone(arg)` can be turned into `res = move arg;`
+                // if `arg` is the only borrow of `cloned` at this point.
+
+                if cannot_move_out || !possible_borrower.only_borrowers(&[arg], cloned, loc) {
+                    continue;
+                }
+
+                cloned
+            } else {
+                // `arg` is a reference as it is `.deref()`ed in the previous block.
+                // Look into the predecessor block and find out the source of deref.
 
-            // _1 in MIR `{ _2 = &_1; _3 = deref(move _2); } -> { _4 = _3; to_path_buf(move _4); }`
-            let referent = if from_deref {
                 let ps = mir.predecessors_for(bb);
                 if ps.len() != 1 {
                     continue;
                 }
                 let pred_terminator = mir[ps[0]].terminator();
 
+                // receiver of the `deref()` call
                 let pred_arg = if_chain! {
                     if let Some((pred_fn_def_id, pred_arg, pred_arg_ty, Some(res))) =
                         is_call_with_ref_arg(cx, mir, &pred_terminator.kind);
@@ -151,21 +167,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
                     }
                 };
 
-                let (local, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(
-                    cx,
-                    mir,
-                    pred_arg,
-                    true,
-                    mir[ps[0]].statements.iter()
-                ));
-                if cannot_move_out {
+                let (local, cannot_move_out) =
+                    unwrap_or_continue!(find_stmt_assigns_to(cx, mir, pred_arg, true, ps[0]));
+                let loc = mir::Location {
+                    block: bb,
+                    statement_index: mir.basic_blocks()[bb].statements.len(),
+                };
+
+                // This can be turned into `res = move local` if `arg` and `cloned` are not borrowed
+                // at the last statement:
+                //
+                // ```
+                // pred_arg = &local;
+                // cloned = deref(pred_arg);
+                // arg = &cloned;
+                // StorageDead(pred_arg);
+                // res = to_path_buf(cloned);
+                // ```
+                if cannot_move_out || !possible_borrower.only_borrowers(&[arg, cloned], local, loc) {
                     continue;
                 }
+
                 local
-            } else {
-                cloned
             };
 
+            // `local` cannot be moved out if it is used later
             let used_later = traversal::ReversePostorder::new(&mir, bb).skip(1).any(|(tbb, tdata)| {
                 // Give up on loops
                 if tdata.terminator().successors().any(|s| *s == bb) {
@@ -173,7 +199,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
                 }
 
                 let mut vis = LocalUseVisitor {
-                    local: referent,
+                    local,
                     used_other_than_drop: false,
                 };
                 vis.visit_basic_block_data(tbb, tdata);
@@ -195,13 +221,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
                         let sugg_span = span.with_lo(
                             span.lo() + BytePos(u32::try_from(dot).unwrap())
                         );
+                        let mut app = Applicability::MaybeIncorrect;
+
+                        let mut call_snip = &snip[dot + 1..];
+                        // Machine applicable when `call_snip` looks like `foobar()`
+                        if call_snip.ends_with("()") {
+                            call_snip = call_snip[..call_snip.len()-2].trim();
+                            if call_snip.as_bytes().iter().all(|b| b.is_ascii_alphabetic() || *b == b'_') {
+                                app = Applicability::MachineApplicable;
+                            }
+                        }
 
                         span_lint_hir_and_then(cx, REDUNDANT_CLONE, node, sugg_span, "redundant clone", |db| {
                             db.span_suggestion(
                                 sugg_span,
                                 "remove this",
                                 String::new(),
-                                Applicability::MaybeIncorrect,
+                                app,
                             );
                             db.span_note(
                                 span.with_hi(span.lo() + BytePos(u32::try_from(dot).unwrap())),
@@ -224,7 +260,7 @@ fn is_call_with_ref_arg<'tcx>(
     kind: &'tcx mir::TerminatorKind<'tcx>,
 ) -> Option<(def_id::DefId, mir::Local, Ty<'tcx>, Option<&'tcx mir::Place<'tcx>>)> {
     if_chain! {
-        if let TerminatorKind::Call { func, args, destination, .. } = kind;
+        if let mir::TerminatorKind::Call { func, args, destination, .. } = kind;
         if args.len() == 1;
         if let mir::Operand::Move(mir::Place { base: mir::PlaceBase::Local(local), .. }) = &args[0];
         if let ty::FnDef(def_id, _) = func.ty(&*mir, cx.tcx).kind;
@@ -241,42 +277,35 @@ fn is_call_with_ref_arg<'tcx>(
 type CannotMoveOut = bool;
 
 /// Finds the first `to = (&)from`, and returns
-/// ``Some((from, [`true` if `from` cannot be moved out]))``.
-fn find_stmt_assigns_to<'a, 'tcx: 'a>(
+/// ``Some((from, whether `from` cannot be moved out))``.
+fn find_stmt_assigns_to<'tcx>(
     cx: &LateContext<'_, 'tcx>,
     mir: &mir::Body<'tcx>,
-    to: mir::Local,
+    to_local: mir::Local,
     by_ref: bool,
-    stmts: impl DoubleEndedIterator<Item = &'a mir::Statement<'tcx>>,
+    bb: mir::BasicBlock,
 ) -> Option<(mir::Local, CannotMoveOut)> {
-    stmts
-        .rev()
-        .find_map(|stmt| {
-            if let mir::StatementKind::Assign(box (
-                mir::Place {
-                    base: mir::PlaceBase::Local(local),
-                    ..
-                },
-                v,
-            )) = &stmt.kind
-            {
-                if *local == to {
-                    return Some(v);
-                }
-            }
+    let rvalue = mir.basic_blocks()[bb].statements.iter().rev().find_map(|stmt| {
+        if let mir::StatementKind::Assign(box (
+            mir::Place {
+                base: mir::PlaceBase::Local(local),
+                ..
+            },
+            v,
+        )) = &stmt.kind
+        {
+            return if *local == to_local { Some(v) } else { None };
+        }
 
-            None
-        })
-        .and_then(|v| {
-            if by_ref {
-                if let mir::Rvalue::Ref(_, _, ref place) = v {
-                    return base_local_and_movability(cx, mir, place);
-                }
-            } else if let mir::Rvalue::Use(mir::Operand::Copy(ref place)) = v {
-                return base_local_and_movability(cx, mir, place);
-            }
-            None
-        })
+        None
+    })?;
+
+    match (by_ref, &*rvalue) {
+        (true, mir::Rvalue::Ref(_, _, place)) | (false, mir::Rvalue::Use(mir::Operand::Copy(place))) => {
+            base_local_and_movability(cx, mir, place)
+        },
+        _ => None,
+    }
 }
 
 /// Extracts and returns the undermost base `Local` of given `place`. Returns `place` itself
@@ -288,8 +317,6 @@ fn base_local_and_movability<'tcx>(
     mir: &mir::Body<'tcx>,
     place: &mir::Place<'tcx>,
 ) -> Option<(mir::Local, CannotMoveOut)> {
-    use rustc::mir::Place;
-    use rustc::mir::PlaceBase;
     use rustc::mir::PlaceRef;
 
     // Dereference. You cannot move things out from a borrowed value.
@@ -301,13 +328,15 @@ fn base_local_and_movability<'tcx>(
         base: place_base,
         mut projection,
     } = place.as_ref();
-    if let PlaceBase::Local(local) = place_base {
+    if let mir::PlaceBase::Local(local) = place_base {
         while let [base @ .., elem] = projection {
             projection = base;
-            deref = matches!(elem, mir::ProjectionElem::Deref);
-            field = !field
-                && matches!(elem, mir::ProjectionElem::Field(..))
-                && has_drop(cx, Place::ty_from(place_base, projection, &mir.local_decls, cx.tcx).ty);
+            deref |= matches!(elem, mir::ProjectionElem::Deref);
+            field |= matches!(elem, mir::ProjectionElem::Field(..))
+                && has_drop(
+                    cx,
+                    mir::Place::ty_from(place_base, projection, &mir.local_decls, cx.tcx).ty,
+                );
         }
 
         Some((*local, deref || field))
@@ -353,3 +382,233 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor {
         }
     }
 }
+
+/// Determines liveness of each local purely based on `StorageLive`/`Dead`.
+#[derive(Copy, Clone)]
+struct MaybeStorageLive<'a, 'tcx> {
+    body: &'a mir::Body<'tcx>,
+}
+
+impl<'a, 'tcx> MaybeStorageLive<'a, 'tcx> {
+    fn new(body: &'a mir::Body<'tcx>) -> Self {
+        MaybeStorageLive { body }
+    }
+}
+
+impl<'a, 'tcx> BitDenotation<'tcx> for MaybeStorageLive<'a, 'tcx> {
+    type Idx = mir::Local;
+    fn name() -> &'static str {
+        "maybe_storage_live"
+    }
+    fn bits_per_block(&self) -> usize {
+        self.body.local_decls.len()
+    }
+
+    fn start_block_effect(&self, on_entry: &mut BitSet<mir::Local>) {
+        for arg in self.body.args_iter() {
+            on_entry.insert(arg);
+        }
+    }
+
+    fn statement_effect(&self, trans: &mut GenKillSet<mir::Local>, loc: mir::Location) {
+        let stmt = &self.body[loc.block].statements[loc.statement_index];
+
+        match stmt.kind {
+            mir::StatementKind::StorageLive(l) => trans.gen(l),
+            mir::StatementKind::StorageDead(l) => trans.kill(l),
+            _ => (),
+        }
+    }
+
+    fn terminator_effect(&self, _trans: &mut GenKillSet<mir::Local>, _loc: mir::Location) {}
+
+    fn propagate_call_return(
+        &self,
+        _in_out: &mut BitSet<mir::Local>,
+        _call_bb: mir::BasicBlock,
+        _dest_bb: mir::BasicBlock,
+        _dest_place: &mir::Place<'tcx>,
+    ) {
+        // Nothing to do when a call returns successfully
+    }
+}
+
+impl<'a, 'tcx> BottomValue for MaybeStorageLive<'a, 'tcx> {
+    /// bottom = dead
+    const BOTTOM_VALUE: bool = false;
+}
+
+/// Collects the possible borrowers of each local.
+/// For example, `b = &a; c = &a;` will make `b` and (transitively) `c`
+/// possible borrowers of `a`.
+struct PossibleBorrowerVisitor<'a, 'tcx> {
+    possible_borrower: TransitiveRelation<mir::Local>,
+    body: &'a mir::Body<'tcx>,
+    cx: &'a LateContext<'a, 'tcx>,
+}
+
+impl<'a, 'tcx> PossibleBorrowerVisitor<'a, 'tcx> {
+    fn new(cx: &'a LateContext<'a, 'tcx>, body: &'a mir::Body<'tcx>) -> Self {
+        Self {
+            possible_borrower: TransitiveRelation::default(),
+            cx,
+            body,
+        }
+    }
+
+    fn into_map(
+        self,
+        cx: &LateContext<'a, 'tcx>,
+        maybe_live: DataflowResults<'tcx, MaybeStorageLive<'a, 'tcx>>,
+    ) -> PossibleBorrower<'a, 'tcx> {
+        let mut map = FxHashMap::default();
+        for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) {
+            if is_copy(cx, self.body.local_decls[row].ty) {
+                continue;
+            }
+
+            let borrowers = self.possible_borrower.reachable_from(&row);
+            if !borrowers.is_empty() {
+                let mut bs = HybridBitSet::new_empty(self.body.local_decls.len());
+                for &c in borrowers {
+                    if c != mir::Local::from_usize(0) {
+                        bs.insert(c);
+                    }
+                }
+
+                if !bs.is_empty() {
+                    map.insert(row, bs);
+                }
+            }
+        }
+
+        let bs = BitSet::new_empty(self.body.local_decls.len());
+        PossibleBorrower {
+            map,
+            maybe_live: DataflowResultsCursor::new(maybe_live, self.body),
+            bitset: (bs.clone(), bs),
+        }
+    }
+}
+
+impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'tcx> {
+    fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) {
+        if let mir::PlaceBase::Local(lhs) = place.base {
+            match rvalue {
+                mir::Rvalue::Ref(_, _, borrowed) => {
+                    if let mir::PlaceBase::Local(borrowed_local) = borrowed.base {
+                        self.possible_borrower.add(borrowed_local, lhs);
+                    }
+                },
+                other => {
+                    if !ContainsRegion.visit_ty(place.ty(&self.body.local_decls, self.cx.tcx).ty) {
+                        return;
+                    }
+                    rvalue_locals(other, |rhs| {
+                        if lhs != rhs {
+                            self.possible_borrower.add(rhs, lhs);
+                        }
+                    });
+                },
+            }
+        }
+    }
+
+    fn visit_terminator(&mut self, terminator: &mir::Terminator<'_>, _loc: mir::Location) {
+        if let mir::TerminatorKind::Call {
+            args,
+            destination:
+                Some((
+                    mir::Place {
+                        base: mir::PlaceBase::Local(dest),
+                        ..
+                    },
+                    _,
+                )),
+            ..
+        } = &terminator.kind
+        {
+            // If the call returns something with lifetimes,
+            // let's conservatively assume the returned value contains lifetime of all the arguments.
+            // For example, given `let y: Foo<'a> = foo(x)`, `y` is considered to be a possible borrower of `x`.
+            if !ContainsRegion.visit_ty(&self.body.local_decls[*dest].ty) {
+                return;
+            }
+
+            for op in args {
+                match op {
+                    mir::Operand::Copy(p) | mir::Operand::Move(p) => {
+                        if let mir::PlaceBase::Local(arg) = p.base {
+                            self.possible_borrower.add(arg, *dest);
+                        }
+                    },
+                    _ => (),
+                }
+            }
+        }
+    }
+}
+
+struct ContainsRegion;
+
+impl TypeVisitor<'_> for ContainsRegion {
+    fn visit_region(&mut self, _: ty::Region<'_>) -> bool {
+        true
+    }
+}
+
+fn rvalue_locals(rvalue: &mir::Rvalue<'_>, mut visit: impl FnMut(mir::Local)) {
+    use rustc::mir::Rvalue::*;
+
+    let mut visit_op = |op: &mir::Operand<'_>| match op {
+        mir::Operand::Copy(p) | mir::Operand::Move(p) => {
+            if let mir::PlaceBase::Local(l) = p.base {
+                visit(l)
+            }
+        },
+        _ => (),
+    };
+
+    match rvalue {
+        Use(op) | Repeat(op, _) | Cast(_, op, _) | UnaryOp(_, op) => visit_op(op),
+        Aggregate(_, ops) => ops.iter().for_each(visit_op),
+        BinaryOp(_, lhs, rhs) | CheckedBinaryOp(_, lhs, rhs) => {
+            visit_op(lhs);
+            visit_op(rhs);
+        },
+        _ => (),
+    }
+}
+
+/// Result of `PossibleBorrowerVisitor`.
+struct PossibleBorrower<'a, 'tcx> {
+    /// Mapping `Local -> its possible borrowers`
+    map: FxHashMap<mir::Local, HybridBitSet<mir::Local>>,
+    maybe_live: DataflowResultsCursor<'a, 'tcx, MaybeStorageLive<'a, 'tcx>>,
+    // Caches to avoid allocation of `BitSet` on every query
+    bitset: (BitSet<mir::Local>, BitSet<mir::Local>),
+}
+
+impl PossibleBorrower<'_, '_> {
+    /// Returns true if the set of borrowers of `borrowed` living at `at` matches with `borrowers`.
+    fn only_borrowers(&mut self, borrowers: &[mir::Local], borrowed: mir::Local, at: mir::Location) -> bool {
+        self.maybe_live.seek(at);
+
+        self.bitset.0.clear();
+        let maybe_live = &mut self.maybe_live;
+        if let Some(bitset) = self.map.get(&borrowed) {
+            for b in bitset.iter().filter(move |b| maybe_live.contains(*b)) {
+                self.bitset.0.insert(b);
+            }
+        } else {
+            return false;
+        }
+
+        self.bitset.1.clear();
+        for b in borrowers {
+            self.bitset.1.insert(*b);
+        }
+
+        self.bitset.0 == self.bitset.1
+    }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 383be8b1da4..d56ccdfd089 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1542,7 +1542,7 @@ pub const ALL_LINTS: [Lint; 318] = [
     },
     Lint {
         name: "redundant_clone",
-        group: "nursery",
+        group: "perf",
         desc: "`clone()` of an owned value that is going to be dropped immediately",
         deprecation: None,
         module: "redundant_clone",
diff --git a/tests/compile-test.rs b/tests/compile-test.rs
index e0b1ebf4b8b..e65a4a9a40a 100644
--- a/tests/compile-test.rs
+++ b/tests/compile-test.rs
@@ -38,8 +38,7 @@ fn config(mode: &str, dir: PathBuf) -> compiletest::Config {
 
     let cfg_mode = mode.parse().expect("Invalid mode");
     if let Ok(name) = var::<&str>("TESTNAME") {
-        let s: String = name.to_owned();
-        config.filter = Some(s)
+        config.filter = Some(name)
     }
 
     if rustc_test_suite().is_some() {
diff --git a/tests/ui/crashes/auxiliary/proc_macro_crash.rs b/tests/ui/crashes/auxiliary/proc_macro_crash.rs
index 71b10ed4db4..086548e58ed 100644
--- a/tests/ui/crashes/auxiliary/proc_macro_crash.rs
+++ b/tests/ui/crashes/auxiliary/proc_macro_crash.rs
@@ -30,7 +30,7 @@ pub fn macro_test(input_stream: TokenStream) -> TokenStream {
                 TokenTree::Ident(Ident::new("true", Span::call_site())),
                 TokenTree::Group(clause.clone()),
                 TokenTree::Ident(Ident::new("else", Span::call_site())),
-                TokenTree::Group(clause.clone()),
+                TokenTree::Group(clause),
             ])
         })),
     ])
diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs
index 78d332c7a31..d435484d3e3 100644
--- a/tests/ui/escape_analysis.rs
+++ b/tests/ui/escape_analysis.rs
@@ -1,5 +1,10 @@
 #![feature(box_syntax)]
-#![allow(clippy::borrowed_box, clippy::needless_pass_by_value, clippy::unused_unit)]
+#![allow(
+    clippy::borrowed_box,
+    clippy::needless_pass_by_value,
+    clippy::unused_unit,
+    clippy::redundant_clone
+)]
 #![warn(clippy::boxed_local)]
 
 #[derive(Clone)]
diff --git a/tests/ui/escape_analysis.stderr b/tests/ui/escape_analysis.stderr
index 3944acd87f2..73fa9bfe19b 100644
--- a/tests/ui/escape_analysis.stderr
+++ b/tests/ui/escape_analysis.stderr
@@ -1,5 +1,5 @@
 error: local variable doesn't need to be boxed here
-  --> $DIR/escape_analysis.rs:34:13
+  --> $DIR/escape_analysis.rs:39:13
    |
 LL | fn warn_arg(x: Box<A>) {
    |             ^
@@ -7,13 +7,13 @@ LL | fn warn_arg(x: Box<A>) {
    = note: `-D clippy::boxed-local` implied by `-D warnings`
 
 error: local variable doesn't need to be boxed here
-  --> $DIR/escape_analysis.rs:125:12
+  --> $DIR/escape_analysis.rs:130:12
    |
 LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
    |            ^^^^^^^^^^^
 
 error: local variable doesn't need to be boxed here
-  --> $DIR/escape_analysis.rs:165:23
+  --> $DIR/escape_analysis.rs:170:23
    |
 LL |     fn closure_borrow(x: Box<A>) {
    |                       ^
diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed
index c8b9bc04944..2029c81d0d5 100644
--- a/tests/ui/map_clone.fixed
+++ b/tests/ui/map_clone.fixed
@@ -1,7 +1,7 @@
 // run-rustfix
 #![warn(clippy::all, clippy::pedantic)]
 #![allow(clippy::iter_cloned_collect)]
-#![allow(clippy::clone_on_copy)]
+#![allow(clippy::clone_on_copy, clippy::redundant_clone)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::redundant_closure_for_method_calls)]
 
diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs
index 5f216823eb4..495c18f311f 100644
--- a/tests/ui/map_clone.rs
+++ b/tests/ui/map_clone.rs
@@ -1,7 +1,7 @@
 // run-rustfix
 #![warn(clippy::all, clippy::pedantic)]
 #![allow(clippy::iter_cloned_collect)]
-#![allow(clippy::clone_on_copy)]
+#![allow(clippy::clone_on_copy, clippy::redundant_clone)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::redundant_closure_for_method_calls)]
 
diff --git a/tests/ui/needless_pass_by_value.rs b/tests/ui/needless_pass_by_value.rs
index f031dd105c2..ca94daa24e8 100644
--- a/tests/ui/needless_pass_by_value.rs
+++ b/tests/ui/needless_pass_by_value.rs
@@ -4,7 +4,8 @@
     clippy::single_match,
     clippy::redundant_pattern_matching,
     clippy::many_single_char_names,
-    clippy::option_option
+    clippy::option_option,
+    clippy::redundant_clone
 )]
 
 use std::borrow::Borrow;
diff --git a/tests/ui/needless_pass_by_value.stderr b/tests/ui/needless_pass_by_value.stderr
index ad0e6461c22..5efeea0685c 100644
--- a/tests/ui/needless_pass_by_value.stderr
+++ b/tests/ui/needless_pass_by_value.stderr
@@ -1,5 +1,5 @@
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:16:23
+  --> $DIR/needless_pass_by_value.rs:17:23
    |
 LL | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> {
    |                       ^^^^^^ help: consider changing the type to: `&[T]`
@@ -7,25 +7,25 @@ LL | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T
    = note: `-D clippy::needless-pass-by-value` implied by `-D warnings`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:30:11
+  --> $DIR/needless_pass_by_value.rs:31:11
    |
 LL | fn bar(x: String, y: Wrapper) {
    |           ^^^^^^ help: consider changing the type to: `&str`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:30:22
+  --> $DIR/needless_pass_by_value.rs:31:22
    |
 LL | fn bar(x: String, y: Wrapper) {
    |                      ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:36:71
+  --> $DIR/needless_pass_by_value.rs:37:71
    |
 LL | fn test_borrow_trait<T: Borrow<str>, U: AsRef<str>, V>(t: T, u: U, v: V) {
    |                                                                       ^ help: consider taking a reference instead: `&V`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:48:18
+  --> $DIR/needless_pass_by_value.rs:49:18
    |
 LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
    |                  ^^^^^^^^^^^^^^^^^^^^^^
@@ -36,13 +36,13 @@ LL |     match *x {
    |
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:61:24
+  --> $DIR/needless_pass_by_value.rs:62:24
    |
 LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
    |                        ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:61:36
+  --> $DIR/needless_pass_by_value.rs:62:36
    |
 LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
    |                                    ^^^^^^^
@@ -55,19 +55,19 @@ LL |     let Wrapper(_) = *y; // still not moved
    |
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:77:49
+  --> $DIR/needless_pass_by_value.rs:78:49
    |
 LL | fn test_blanket_ref<T: Foo, S: Serialize>(_foo: T, _serializable: S) {}
    |                                                 ^ help: consider taking a reference instead: `&T`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:79:18
+  --> $DIR/needless_pass_by_value.rs:80:18
    |
 LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                  ^^^^^^ help: consider taking a reference instead: `&String`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:79:29
+  --> $DIR/needless_pass_by_value.rs:80:29
    |
 LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                             ^^^^^^
@@ -81,13 +81,13 @@ LL |     let _ = t.to_string();
    |             ^^^^^^^^^^^^^
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:79:40
+  --> $DIR/needless_pass_by_value.rs:80:40
    |
 LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                                        ^^^^^^^^ help: consider taking a reference instead: `&Vec<i32>`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:79:53
+  --> $DIR/needless_pass_by_value.rs:80:53
    |
 LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) {
    |                                                     ^^^^^^^^
@@ -101,61 +101,61 @@ LL |     let _ = v.to_owned();
    |             ^^^^^^^^^^^^
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:92:12
+  --> $DIR/needless_pass_by_value.rs:93:12
    |
 LL |         s: String,
    |            ^^^^^^ help: consider changing the type to: `&str`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:93:12
+  --> $DIR/needless_pass_by_value.rs:94:12
    |
 LL |         t: String,
    |            ^^^^^^ help: consider taking a reference instead: `&String`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:102:23
+  --> $DIR/needless_pass_by_value.rs:103:23
    |
 LL |     fn baz(&self, _u: U, _s: Self) {}
    |                       ^ help: consider taking a reference instead: `&U`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:102:30
+  --> $DIR/needless_pass_by_value.rs:103:30
    |
 LL |     fn baz(&self, _u: U, _s: Self) {}
    |                              ^^^^ help: consider taking a reference instead: `&Self`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:124:24
+  --> $DIR/needless_pass_by_value.rs:125:24
    |
 LL | fn bar_copy(x: u32, y: CopyWrapper) {
    |                        ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
    |
 help: consider marking this type as Copy
-  --> $DIR/needless_pass_by_value.rs:122:1
+  --> $DIR/needless_pass_by_value.rs:123:1
    |
 LL | struct CopyWrapper(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:130:29
+  --> $DIR/needless_pass_by_value.rs:131:29
    |
 LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
    |                             ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
    |
 help: consider marking this type as Copy
-  --> $DIR/needless_pass_by_value.rs:122:1
+  --> $DIR/needless_pass_by_value.rs:123:1
    |
 LL | struct CopyWrapper(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:130:45
+  --> $DIR/needless_pass_by_value.rs:131:45
    |
 LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
    |                                             ^^^^^^^^^^^
    |
 help: consider marking this type as Copy
-  --> $DIR/needless_pass_by_value.rs:122:1
+  --> $DIR/needless_pass_by_value.rs:123:1
    |
 LL | struct CopyWrapper(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -168,13 +168,13 @@ LL |     let CopyWrapper(_) = *y; // still not moved
    |
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:130:61
+  --> $DIR/needless_pass_by_value.rs:131:61
    |
 LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
    |                                                             ^^^^^^^^^^^
    |
 help: consider marking this type as Copy
-  --> $DIR/needless_pass_by_value.rs:122:1
+  --> $DIR/needless_pass_by_value.rs:123:1
    |
 LL | struct CopyWrapper(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -185,13 +185,13 @@ LL |     let CopyWrapper(s) = *z; // moved
    |
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:142:40
+  --> $DIR/needless_pass_by_value.rs:143:40
    |
 LL | fn some_fun<'b, S: Bar<'b, ()>>(_item: S) {}
    |                                        ^ help: consider taking a reference instead: `&S`
 
 error: this argument is passed by value, but not consumed in the function body
-  --> $DIR/needless_pass_by_value.rs:147:20
+  --> $DIR/needless_pass_by_value.rs:148:20
    |
 LL | fn more_fun(_item: impl Club<'static, i32>) {}
    |                    ^^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&impl Club<'static, i32>`
diff --git a/tests/ui/ptr_arg.rs b/tests/ui/ptr_arg.rs
index 1ce6081bf94..30f39e9b063 100644
--- a/tests/ui/ptr_arg.rs
+++ b/tests/ui/ptr_arg.rs
@@ -1,4 +1,4 @@
-#![allow(unused, clippy::many_single_char_names)]
+#![allow(unused, clippy::many_single_char_names, clippy::redundant_clone)]
 #![warn(clippy::ptr_arg)]
 
 use std::borrow::Cow;
diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs
index 56ccf1d432f..57237c52e8c 100644
--- a/tests/ui/question_mark.rs
+++ b/tests/ui/question_mark.rs
@@ -104,7 +104,7 @@ fn main() {
     };
     move_struct.ref_func();
     move_struct.clone().mov_func_reuse();
-    move_struct.clone().mov_func_no_use();
+    move_struct.mov_func_no_use();
 
     let so = SeemsOption::Some(45);
     returns_something_similar_to_option(so);
diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed
new file mode 100644
index 00000000000..614a9bf4d90
--- /dev/null
+++ b/tests/ui/redundant_clone.fixed
@@ -0,0 +1,132 @@
+// run-rustfix
+// rustfix-only-machine-applicable
+use std::ffi::OsString;
+use std::path::Path;
+
+fn main() {
+    let _s = ["lorem", "ipsum"].join(" ");
+
+    let s = String::from("foo");
+    let _s = s;
+
+    let s = String::from("foo");
+    let _s = s;
+
+    let s = String::from("foo");
+    let _s = s;
+
+    let _s = Path::new("/a/b/").join("c");
+
+    let _s = Path::new("/a/b/").join("c");
+
+    let _s = OsString::new();
+
+    let _s = OsString::new();
+
+    // Check that lint level works
+    #[allow(clippy::redundant_clone)]
+    let _s = String::new().to_string();
+
+    let tup = (String::from("foo"),);
+    let _t = tup.0;
+
+    let tup_ref = &(String::from("foo"),);
+    let _s = tup_ref.0.clone(); // this `.clone()` cannot be removed
+
+    {
+        let x = String::new();
+        let y = &x;
+
+        let _x = x.clone(); // ok; `x` is borrowed by `y`
+
+        let _ = y.len();
+    }
+
+    let x = (String::new(),);
+    let _ = Some(String::new()).unwrap_or_else(|| x.0.clone()); // ok; closure borrows `x`
+
+    with_branch(Alpha, true);
+    cannot_move_from_type_with_drop();
+    borrower_propagation();
+}
+
+#[derive(Clone)]
+struct Alpha;
+fn with_branch(a: Alpha, b: bool) -> (Alpha, Alpha) {
+    if b {
+        (a.clone(), a)
+    } else {
+        (Alpha, a)
+    }
+}
+
+struct TypeWithDrop {
+    x: String,
+}
+
+impl Drop for TypeWithDrop {
+    fn drop(&mut self) {}
+}
+
+fn cannot_move_from_type_with_drop() -> String {
+    let s = TypeWithDrop { x: String::new() };
+    s.x.clone() // removing this `clone()` summons E0509
+}
+
+fn borrower_propagation() {
+    let s = String::new();
+    let t = String::new();
+
+    {
+        fn b() -> bool {
+            unimplemented!()
+        }
+        let _u = if b() { &s } else { &t };
+
+        // ok; `s` and `t` are possibly borrowed
+        let _s = s.clone();
+        let _t = t.clone();
+    }
+
+    {
+        let _u = || s.len();
+        let _v = [&t; 32];
+        let _s = s.clone(); // ok
+        let _t = t.clone(); // ok
+    }
+
+    {
+        let _u = {
+            let u = Some(&s);
+            let _ = s.clone(); // ok
+            u
+        };
+        let _s = s.clone(); // ok
+    }
+
+    {
+        use std::convert::identity as id;
+        let _u = id(id(&s));
+        let _s = s.clone(); // ok, `u` borrows `s`
+    }
+
+    let _s = s;
+    let _t = t;
+
+    #[derive(Clone)]
+    struct Foo {
+        x: usize,
+    }
+
+    {
+        let f = Foo { x: 123 };
+        let _x = Some(f.x);
+        let _f = f;
+    }
+
+    {
+        let f = Foo { x: 123 };
+        let _x = &f.x;
+        let _f = f.clone(); // ok
+    }
+}
diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs
index 6e9ad71e55b..48687c82c2f 100644
--- a/tests/ui/redundant_clone.rs
+++ b/tests/ui/redundant_clone.rs
@@ -1,37 +1,53 @@
-#![warn(clippy::redundant_clone)]
-
+// run-rustfix
+// rustfix-only-machine-applicable
 use std::ffi::OsString;
 use std::path::Path;
 
 fn main() {
-    let _ = ["lorem", "ipsum"].join(" ").to_string();
+    let _s = ["lorem", "ipsum"].join(" ").to_string();
 
     let s = String::from("foo");
-    let _ = s.clone();
+    let _s = s.clone();
 
     let s = String::from("foo");
-    let _ = s.to_string();
+    let _s = s.to_string();
 
     let s = String::from("foo");
-    let _ = s.to_owned();
+    let _s = s.to_owned();
 
-    let _ = Path::new("/a/b/").join("c").to_owned();
+    let _s = Path::new("/a/b/").join("c").to_owned();
 
-    let _ = Path::new("/a/b/").join("c").to_path_buf();
+    let _s = Path::new("/a/b/").join("c").to_path_buf();
 
-    let _ = OsString::new().to_owned();
+    let _s = OsString::new().to_owned();
 
-    let _ = OsString::new().to_os_string();
+    let _s = OsString::new().to_os_string();
 
     // Check that lint level works
     #[allow(clippy::redundant_clone)]
-    let _ = String::new().to_string();
+    let _s = String::new().to_string();
 
     let tup = (String::from("foo"),);
-    let _ = tup.0.clone();
+    let _t = tup.0.clone();
 
     let tup_ref = &(String::from("foo"),);
     let _s = tup_ref.0.clone(); // this `.clone()` cannot be removed
+
+    {
+        let x = String::new();
+        let y = &x;
+
+        let _x = x.clone(); // ok; `x` is borrowed by `y`
+
+        let _ = y.len();
+    }
+
+    let x = (String::new(),);
+    let _ = Some(String::new()).unwrap_or_else(|| x.0.clone()); // ok; closure borrows `x`
+
+    with_branch(Alpha, true);
+    cannot_move_from_type_with_drop();
+    borrower_propagation();
 }
 
 #[derive(Clone)]
@@ -56,3 +72,61 @@ fn cannot_move_from_type_with_drop() -> String {
     let s = TypeWithDrop { x: String::new() };
     s.x.clone() // removing this `clone()` summons E0509
 }
+
+fn borrower_propagation() {
+    let s = String::new();
+    let t = String::new();
+
+    {
+        fn b() -> bool {
+            unimplemented!()
+        }
+        let _u = if b() { &s } else { &t };
+
+        // ok; `s` and `t` are possibly borrowed
+        let _s = s.clone();
+        let _t = t.clone();
+    }
+
+    {
+        let _u = || s.len();
+        let _v = [&t; 32];
+        let _s = s.clone(); // ok
+        let _t = t.clone(); // ok
+    }
+
+    {
+        let _u = {
+            let u = Some(&s);
+            let _ = s.clone(); // ok
+            u
+        };
+        let _s = s.clone(); // ok
+    }
+
+    {
+        use std::convert::identity as id;
+        let _u = id(id(&s));
+        let _s = s.clone(); // ok, `u` borrows `s`
+    }
+
+    let _s = s.clone();
+    let _t = t.clone();
+
+    #[derive(Clone)]
+    struct Foo {
+        x: usize,
+    }
+
+    {
+        let f = Foo { x: 123 };
+        let _x = Some(f.x);
+        let _f = f.clone();
+    }
+
+    {
+        let f = Foo { x: 123 };
+        let _x = &f.x;
+        let _f = f.clone(); // ok
+    }
+}
diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr
index c8f6cacab2b..feafbd78b4e 100644
--- a/tests/ui/redundant_clone.stderr
+++ b/tests/ui/redundant_clone.stderr
@@ -1,123 +1,159 @@
 error: redundant clone
-  --> $DIR/redundant_clone.rs:7:41
+  --> $DIR/redundant_clone.rs:7:42
    |
-LL |     let _ = ["lorem", "ipsum"].join(" ").to_string();
-   |                                         ^^^^^^^^^^^^ help: remove this
+LL |     let _s = ["lorem", "ipsum"].join(" ").to_string();
+   |                                          ^^^^^^^^^^^^ help: remove this
    |
    = note: `-D clippy::redundant-clone` implied by `-D warnings`
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:7:13
+  --> $DIR/redundant_clone.rs:7:14
    |
-LL |     let _ = ["lorem", "ipsum"].join(" ").to_string();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     let _s = ["lorem", "ipsum"].join(" ").to_string();
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:10:14
+  --> $DIR/redundant_clone.rs:10:15
    |
-LL |     let _ = s.clone();
-   |              ^^^^^^^^ help: remove this
+LL |     let _s = s.clone();
+   |               ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:10:13
+  --> $DIR/redundant_clone.rs:10:14
    |
-LL |     let _ = s.clone();
-   |             ^
+LL |     let _s = s.clone();
+   |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:13:14
+  --> $DIR/redundant_clone.rs:13:15
    |
-LL |     let _ = s.to_string();
-   |              ^^^^^^^^^^^^ help: remove this
+LL |     let _s = s.to_string();
+   |               ^^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:13:13
+  --> $DIR/redundant_clone.rs:13:14
    |
-LL |     let _ = s.to_string();
-   |             ^
+LL |     let _s = s.to_string();
+   |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:16:14
+  --> $DIR/redundant_clone.rs:16:15
    |
-LL |     let _ = s.to_owned();
-   |              ^^^^^^^^^^^ help: remove this
+LL |     let _s = s.to_owned();
+   |               ^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:16:13
+  --> $DIR/redundant_clone.rs:16:14
    |
-LL |     let _ = s.to_owned();
-   |             ^
+LL |     let _s = s.to_owned();
+   |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:18:41
+  --> $DIR/redundant_clone.rs:18:42
    |
-LL |     let _ = Path::new("/a/b/").join("c").to_owned();
-   |                                         ^^^^^^^^^^^ help: remove this
+LL |     let _s = Path::new("/a/b/").join("c").to_owned();
+   |                                          ^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:18:13
+  --> $DIR/redundant_clone.rs:18:14
    |
-LL |     let _ = Path::new("/a/b/").join("c").to_owned();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     let _s = Path::new("/a/b/").join("c").to_owned();
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:20:41
+  --> $DIR/redundant_clone.rs:20:42
    |
-LL |     let _ = Path::new("/a/b/").join("c").to_path_buf();
-   |                                         ^^^^^^^^^^^^^^ help: remove this
+LL |     let _s = Path::new("/a/b/").join("c").to_path_buf();
+   |                                          ^^^^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:20:13
+  --> $DIR/redundant_clone.rs:20:14
    |
-LL |     let _ = Path::new("/a/b/").join("c").to_path_buf();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     let _s = Path::new("/a/b/").join("c").to_path_buf();
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:22:28
+  --> $DIR/redundant_clone.rs:22:29
    |
-LL |     let _ = OsString::new().to_owned();
-   |                            ^^^^^^^^^^^ help: remove this
+LL |     let _s = OsString::new().to_owned();
+   |                             ^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:22:13
+  --> $DIR/redundant_clone.rs:22:14
    |
-LL |     let _ = OsString::new().to_owned();
-   |             ^^^^^^^^^^^^^^^
+LL |     let _s = OsString::new().to_owned();
+   |              ^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:24:28
+  --> $DIR/redundant_clone.rs:24:29
    |
-LL |     let _ = OsString::new().to_os_string();
-   |                            ^^^^^^^^^^^^^^^ help: remove this
+LL |     let _s = OsString::new().to_os_string();
+   |                             ^^^^^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:24:13
+  --> $DIR/redundant_clone.rs:24:14
    |
-LL |     let _ = OsString::new().to_os_string();
-   |             ^^^^^^^^^^^^^^^
+LL |     let _s = OsString::new().to_os_string();
+   |              ^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:31:18
+  --> $DIR/redundant_clone.rs:31:19
    |
-LL |     let _ = tup.0.clone();
-   |                  ^^^^^^^^ help: remove this
+LL |     let _t = tup.0.clone();
+   |                   ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:31:13
+  --> $DIR/redundant_clone.rs:31:14
    |
-LL |     let _ = tup.0.clone();
-   |             ^^^^^
+LL |     let _t = tup.0.clone();
+   |              ^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:41:22
+  --> $DIR/redundant_clone.rs:57:22
    |
 LL |         (a.clone(), a.clone())
    |                      ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:41:21
+  --> $DIR/redundant_clone.rs:57:21
    |
 LL |         (a.clone(), a.clone())
    |                     ^
 
-error: aborting due to 10 previous errors
+error: redundant clone
+  --> $DIR/redundant_clone.rs:113:15
+   |
+LL |     let _s = s.clone();
+   |               ^^^^^^^^ help: remove this
+   |
+note: this value is dropped without further use
+  --> $DIR/redundant_clone.rs:113:14
+   |
+LL |     let _s = s.clone();
+   |              ^
+
+error: redundant clone
+  --> $DIR/redundant_clone.rs:114:15
+   |
+LL |     let _t = t.clone();
+   |               ^^^^^^^^ help: remove this
+   |
+note: this value is dropped without further use
+  --> $DIR/redundant_clone.rs:114:14
+   |
+LL |     let _t = t.clone();
+   |              ^
+
+error: redundant clone
+  --> $DIR/redundant_clone.rs:124:19
+   |
+LL |         let _f = f.clone();
+   |                   ^^^^^^^^ help: remove this
+   |
+note: this value is dropped without further use
+  --> $DIR/redundant_clone.rs:124:18
+   |
+LL |         let _f = f.clone();
+   |                  ^
+
+error: aborting due to 13 previous errors
 
diff --git a/tests/ui/swap.rs b/tests/ui/swap.rs
index 093cd7fd04a..b508c1ee009 100644
--- a/tests/ui/swap.rs
+++ b/tests/ui/swap.rs
@@ -1,5 +1,11 @@
 #![warn(clippy::all)]
-#![allow(clippy::blacklisted_name, clippy::no_effect, redundant_semicolon, unused_assignments)]
+#![allow(
+    clippy::blacklisted_name,
+    clippy::no_effect,
+    clippy::redundant_clone,
+    redundant_semicolon,
+    unused_assignments
+)]
 
 struct Foo(u32);
 
diff --git a/tests/ui/swap.stderr b/tests/ui/swap.stderr
index bda0a0bf38b..b45187b5805 100644
--- a/tests/ui/swap.stderr
+++ b/tests/ui/swap.stderr
@@ -1,5 +1,5 @@
 error: this looks like you are swapping elements of `foo` manually
-  --> $DIR/swap.rs:27:5
+  --> $DIR/swap.rs:33:5
    |
 LL | /     let temp = foo[0];
 LL | |     foo[0] = foo[1];
@@ -9,7 +9,7 @@ LL | |     foo[1] = temp;
    = note: `-D clippy::manual-swap` implied by `-D warnings`
 
 error: this looks like you are swapping elements of `foo` manually
-  --> $DIR/swap.rs:36:5
+  --> $DIR/swap.rs:42:5
    |
 LL | /     let temp = foo[0];
 LL | |     foo[0] = foo[1];
@@ -17,7 +17,7 @@ LL | |     foo[1] = temp;
    | |_________________^ help: try: `foo.swap(0, 1)`
 
 error: this looks like you are swapping elements of `foo` manually
-  --> $DIR/swap.rs:45:5
+  --> $DIR/swap.rs:51:5
    |
 LL | /     let temp = foo[0];
 LL | |     foo[0] = foo[1];
@@ -25,7 +25,7 @@ LL | |     foo[1] = temp;
    | |_________________^ help: try: `foo.swap(0, 1)`
 
 error: this looks like you are swapping `a` and `b` manually
-  --> $DIR/swap.rs:65:7
+  --> $DIR/swap.rs:71:7
    |
 LL |       ; let t = a;
    |  _______^
@@ -36,7 +36,7 @@ LL | |     b = t;
    = note: or maybe you should use `std::mem::replace`?
 
 error: this looks like you are swapping `c.0` and `a` manually
-  --> $DIR/swap.rs:74:7
+  --> $DIR/swap.rs:80:7
    |
 LL |       ; let t = c.0;
    |  _______^
@@ -47,7 +47,7 @@ LL | |     a = t;
    = note: or maybe you should use `std::mem::replace`?
 
 error: this looks like you are trying to swap `a` and `b`
-  --> $DIR/swap.rs:62:5
+  --> $DIR/swap.rs:68:5
    |
 LL | /     a = b;
 LL | |     b = a;
@@ -57,7 +57,7 @@ LL | |     b = a;
    = note: or maybe you should use `std::mem::replace`?
 
 error: this looks like you are trying to swap `c.0` and `a`
-  --> $DIR/swap.rs:71:5
+  --> $DIR/swap.rs:77:5
    |
 LL | /     c.0 = a;
 LL | |     a = c.0;
diff --git a/tests/ui/unnecessary_clone.rs b/tests/ui/unnecessary_clone.rs
index a0a50fee180..6dff8d87bae 100644
--- a/tests/ui/unnecessary_clone.rs
+++ b/tests/ui/unnecessary_clone.rs
@@ -1,7 +1,7 @@
 // does not test any rustfixable lints
 
 #![warn(clippy::clone_on_ref_ptr)]
-#![allow(unused)]
+#![allow(unused, clippy::redundant_clone)]
 
 use std::cell::RefCell;
 use std::rc::{self, Rc};