Refactor

diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs
index 1a8a627335863a6881ea1d75d8b2186448a0f47b..85f8b525677be506f9cde1d34bd72971facaa06b 100644 (file)
--- a/clippy_lints/src/redundant_clone.rs
+++ b/clippy_lints/src/redundant_clone.rs
@@ -29,6 +29,15 @@
 use if_chain::if_chain;
 use std::convert::TryFrom;
 
+macro_rules! unwrap_or_continue {
+    ($x:expr) => {
+        match $x {
+            Some(x) => x,
+            None => continue,
+        }
+    };
+}
+
 /// **What it does:** Checks for a redudant `clone()` (and its relatives) which clones an owned
 /// value that is going to be dropped without further use.
 ///
@@ -87,40 +96,15 @@ fn check_fn(
         let def_id = cx.tcx.hir.body_owner_def_id(body.id());
         let mir = cx.tcx.optimized_mir(def_id);
 
-        // Looks for `call(x: &T)` where `T: !Copy`
-        let call = |kind: &mir::TerminatorKind<'tcx>| -> Option<(def_id::DefId, mir::Local, ty::Ty<'tcx>)> {
-            if_chain! {
-                if let TerminatorKind::Call { func, args, .. } = kind;
-                if args.len() == 1;
-                if let mir::Operand::Move(mir::Place::Local(local)) = &args[0];
-                if let ty::FnDef(def_id, _) = func.ty(&*mir, cx.tcx).sty;
-                if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx));
-                if !is_copy(cx, inner_ty);
-                then {
-                    Some((def_id, *local, inner_ty))
-                } else {
-                    None
-                }
-            }
-        };
-
         for (bb, bbdata) in mir.basic_blocks().iter_enumerated() {
-            let terminator = if let Some(terminator) = &bbdata.terminator {
-                terminator
-            } else {
-                continue;
-            };
+            let terminator = unwrap_or_continue!(&bbdata.terminator);
 
             // Give up on loops
             if terminator.successors().any(|s| *s == bb) {
                 continue;
             }
 
-            let (fn_def_id, arg, arg_ty) = if let Some(t) = call(&terminator.kind) {
-                t
-            } else {
-                continue;
-            };
+            let (fn_def_id, arg, arg_ty, _) = unwrap_or_continue!(is_call_with_ref_arg(cx, mir, &terminator.kind));
 
             let from_borrow = match_def_path(cx.tcx, fn_def_id, &paths::CLONE_TRAIT_METHOD)
                 || match_def_path(cx.tcx, fn_def_id, &paths::TO_OWNED_METHOD)
@@ -135,43 +119,23 @@ fn check_fn(
                 continue;
             }
 
-            // _1 in MIR `{ _2 = &_1; clone(move _2); }` or `{ _2 = _1; to_path_buf(_2); }
-            let cloned = if let Some(referent) = bbdata
-                .statements
-                .iter()
-                .rev()
-                .filter_map(|stmt| {
-                    if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
-                        if *local == arg {
-                            if from_deref {
-                                // `r` is already a reference.
-                                if let mir::Rvalue::Use(mir::Operand::Copy(mir::Place::Local(r))) = **v {
-                                    return Some(r);
-                                }
-                            } else if let mir::Rvalue::Ref(_, _, mir::Place::Local(r)) = **v {
-                                return Some(r);
-                            }
-                        }
-                    }
-
-                    None
-                })
-                .next()
-            {
-                referent
-            } else {
-                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 = unwrap_or_continue!(find_stmt_assigns_to(arg, from_borrow, bbdata.statements.iter().rev()));
 
             // _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 = unwrap_or_continue!(&mir[ps[0]].terminator);
+
                 let pred_arg = if_chain! {
-                    if ps.len() == 1;
-                    if let Some(pred_terminator) = &mir[ps[0]].terminator;
-                    if let mir::TerminatorKind::Call { destination: Some((res, _)), .. } = &pred_terminator.kind;
+                    if let Some((pred_fn_def_id, pred_arg, pred_arg_ty, Some(res))) =
+                        is_call_with_ref_arg(cx, mir, &pred_terminator.kind);
                     if *res == mir::Place::Local(cloned);
-                    if let Some((pred_fn_def_id, pred_arg, pred_arg_ty)) = call(&pred_terminator.kind);
                     if match_def_path(cx.tcx, pred_fn_def_id, &paths::DEREF_TRAIT_METHOD);
                     if match_type(cx, pred_arg_ty, &paths::PATH_BUF)
                         || match_type(cx, pred_arg_ty, &paths::OS_STRING);
@@ -182,27 +146,7 @@ fn check_fn(
                     }
                 };
 
-                if let Some(referent) = mir[ps[0]]
-                    .statements
-                    .iter()
-                    .rev()
-                    .filter_map(|stmt| {
-                        if let mir::StatementKind::Assign(mir::Place::Local(l), v) = &stmt.kind {
-                            if *l == pred_arg {
-                                if let mir::Rvalue::Ref(_, _, mir::Place::Local(referent)) = **v {
-                                    return Some(referent);
-                                }
-                            }
-                        }
-
-                        None
-                    })
-                    .next()
-                {
-                    referent
-                } else {
-                    continue;
-                }
+                unwrap_or_continue!(find_stmt_assigns_to(pred_arg, true, mir[ps[0]].statements.iter().rev()))
             } else {
                 cloned
             };
@@ -261,6 +205,50 @@ fn check_fn(
     }
 }
 
+/// If `kind` is `y = func(x: &T)` where `T: !Copy`, returns `(DefId of func, x, T, y)`.
+fn is_call_with_ref_arg<'tcx>(
+    cx: &LateContext<'_, 'tcx>,
+    mir: &'tcx mir::Mir<'tcx>,
+    kind: &'tcx mir::TerminatorKind<'tcx>,
+) -> Option<(def_id::DefId, mir::Local, ty::Ty<'tcx>, Option<&'tcx mir::Place<'tcx>>)> {
+    if_chain! {
+        if let TerminatorKind::Call { func, args, destination, .. } = kind;
+        if args.len() == 1;
+        if let mir::Operand::Move(mir::Place::Local(local)) = &args[0];
+        if let ty::FnDef(def_id, _) = func.ty(&*mir, cx.tcx).sty;
+        if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx));
+        if !is_copy(cx, inner_ty);
+        then {
+            Some((def_id, *local, inner_ty, destination.as_ref().map(|(dest, _)| dest)))
+        } else {
+            None
+        }
+    }
+}
+
+/// Finds the first `to = (&)from`, and returns `Some(from)`.
+fn find_stmt_assigns_to<'a, 'tcx: 'a>(
+    to: mir::Local,
+    by_ref: bool,
+    mut stmts: impl Iterator<Item = &'a mir::Statement<'tcx>>,
+) -> Option<mir::Local> {
+    stmts.find_map(|stmt| {
+        if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
+            if *local == to {
+                if by_ref {
+                    if let mir::Rvalue::Ref(_, _, mir::Place::Local(r)) = **v {
+                        return Some(r);
+                    }
+                } else if let mir::Rvalue::Use(mir::Operand::Copy(mir::Place::Local(r))) = **v {
+                    return Some(r);
+                }
+            }
+        }
+
+        None
+    })
+}
+
 struct LocalUseVisitor {
     local: mir::Local,
     used_other_than_drop: bool,