]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/redundant_clone.rs
Auto merge of #6278 - ThibsG:DerefAddrOf, r=llogiq
[rust.git] / clippy_lints / src / redundant_clone.rs
index 898bd2aef8bf02620b983d7773c6b5a7a783de3c..b4a9804fb25a5a5984356f019f4236ce36a3e643 100644 (file)
@@ -1,6 +1,6 @@
 use crate::utils::{
-    fn_has_unsatisfiable_preds, has_drop, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_hir,
-    span_lint_hir_and_then, walk_ptrs_ty_depth,
+    fn_has_unsatisfiable_preds, has_drop, is_copy, is_type_diagnostic_item, match_def_path, match_type, paths,
+    snippet_opt, span_lint_hir, span_lint_hir_and_then, walk_ptrs_ty_depth,
 };
 use if_chain::if_chain;
 use rustc_data_structures::{fx::FxHashMap, transitive_relation::TransitiveRelation};
     visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _},
 };
 use rustc_middle::ty::{self, fold::TypeVisitor, Ty};
-use rustc_mir::dataflow::BottomValue;
 use rustc_mir::dataflow::{Analysis, AnalysisDomain, GenKill, GenKillAnalysis, ResultsCursor};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::{BytePos, Span};
+use rustc_span::sym;
 use std::convert::TryFrom;
+use std::ops::ControlFlow;
 
 macro_rules! unwrap_or_continue {
     ($x:expr) => {
@@ -66,11 +67,11 @@ macro_rules! unwrap_or_continue {
 
 declare_lint_pass!(RedundantClone => [REDUNDANT_CLONE]);
 
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
+impl<'tcx> LateLintPass<'tcx> for RedundantClone {
     #[allow(clippy::too_many_lines)]
     fn check_fn(
         &mut self,
-        cx: &LateContext<'a, 'tcx>,
+        cx: &LateContext<'tcx>,
         _: FnKind<'tcx>,
         _: &'tcx FnDecl<'_>,
         body: &'tcx Body<'_>,
@@ -85,15 +86,15 @@ fn check_fn(
         }
 
         let mir = cx.tcx.optimized_mir(def_id.to_def_id());
-        let mir_read_only = mir.unwrap_read_only();
 
         let maybe_storage_live_result = MaybeStorageLive
-            .into_engine(cx.tcx, mir, def_id.to_def_id())
+            .into_engine(cx.tcx, mir)
+            .pass_name("redundant_clone")
             .iterate_to_fixpoint()
             .into_results_cursor(mir);
         let mut possible_borrower = {
             let mut vis = PossibleBorrowerVisitor::new(cx, mir);
-            vis.visit_body(&mir_read_only);
+            vis.visit_body(&mir);
             vis.into_map(cx, maybe_storage_live_result)
         };
 
@@ -114,7 +115,8 @@ fn check_fn(
 
             let from_borrow = match_def_path(cx, fn_def_id, &paths::CLONE_TRAIT_METHOD)
                 || match_def_path(cx, fn_def_id, &paths::TO_OWNED_METHOD)
-                || (match_def_path(cx, fn_def_id, &paths::TO_STRING_METHOD) && match_type(cx, arg_ty, &paths::STRING));
+                || (match_def_path(cx, fn_def_id, &paths::TO_STRING_METHOD)
+                    && is_type_diagnostic_item(cx, arg_ty, sym::string_type));
 
             let from_deref = !from_borrow
                 && (match_def_path(cx, fn_def_id, &paths::PATH_TO_PATH_BUF)
@@ -124,6 +126,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));
 
@@ -146,7 +154,7 @@ fn check_fn(
                 // `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.
 
-                let ps = mir_read_only.predecessors_for(bb);
+                let ps = &mir.predecessors()[bb];
                 if ps.len() != 1 {
                     continue;
                 }
@@ -191,7 +199,7 @@ fn check_fn(
                 (local, deref_clone_ret)
             };
 
-            let is_temp = mir_read_only.local_kind(ret_local) == mir::LocalKind::Temp;
+            let is_temp = mir.local_kind(ret_local) == mir::LocalKind::Temp;
 
             // 1. `local` can be moved out if it is not used later.
             // 2. If `ret_local` is a temporary and is neither consumed nor mutated, we can remove this `clone`
@@ -234,10 +242,9 @@ fn check_fn(
                         );
                         let mut app = Applicability::MaybeIncorrect;
 
-                        let mut call_snip = &snip[dot + 1..];
+                        let 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 let Some(call_snip) = call_snip.strip_suffix("()").map(str::trim) {
                             if call_snip.as_bytes().iter().all(|b| b.is_ascii_alphabetic() || *b == b'_') {
                                 app = Applicability::MachineApplicable;
                             }
@@ -273,7 +280,7 @@ 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>,
+    cx: &LateContext<'tcx>,
     mir: &'tcx mir::Body<'tcx>,
     kind: &'tcx mir::TerminatorKind<'tcx>,
 ) -> Option<(def_id::DefId, mir::Local, Ty<'tcx>, mir::Local)> {
@@ -281,7 +288,7 @@ fn is_call_with_ref_arg<'tcx>(
         if let mir::TerminatorKind::Call { func, args, destination, .. } = kind;
         if args.len() == 1;
         if let mir::Operand::Move(mir::Place { local, .. }) = &args[0];
-        if let ty::FnDef(def_id, _) = func.ty(&*mir, cx.tcx).kind;
+        if let ty::FnDef(def_id, _) = *func.ty(&*mir, cx.tcx).kind();
         if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx));
         if !is_copy(cx, inner_ty);
         then {
@@ -297,7 +304,7 @@ fn is_call_with_ref_arg<'tcx>(
 /// Finds the first `to = (&)from`, and returns
 /// ``Some((from, whether `from` cannot be moved out))``.
 fn find_stmt_assigns_to<'tcx>(
-    cx: &LateContext<'_, 'tcx>,
+    cx: &LateContext<'tcx>,
     mir: &mir::Body<'tcx>,
     to_local: mir::Local,
     by_ref: bool,
@@ -331,7 +338,7 @@ fn find_stmt_assigns_to<'tcx>(
 ///
 /// Also reports whether given `place` cannot be moved out.
 fn base_local_and_movability<'tcx>(
-    cx: &LateContext<'_, 'tcx>,
+    cx: &LateContext<'tcx>,
     mir: &mir::Body<'tcx>,
     place: mir::Place<'tcx>,
 ) -> Option<(mir::Local, CannotMoveOut)> {
@@ -405,14 +412,15 @@ fn visit_place(&mut self, place: &mir::Place<'tcx>, ctx: PlaceContext, _: mir::L
 struct MaybeStorageLive;
 
 impl<'tcx> AnalysisDomain<'tcx> for MaybeStorageLive {
-    type Idx = mir::Local;
+    type Domain = BitSet<mir::Local>;
     const NAME: &'static str = "maybe_storage_live";
 
-    fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize {
-        body.local_decls.len()
+    fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain {
+        // bottom = dead
+        BitSet::new_empty(body.local_decls.len())
     }
 
-    fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet<Self::Idx>) {
+    fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut Self::Domain) {
         for arg in body.args_iter() {
             state.insert(arg);
         }
@@ -420,6 +428,8 @@ fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet<Self
 }
 
 impl<'tcx> GenKillAnalysis<'tcx> for MaybeStorageLive {
+    type Idx = mir::Local;
+
     fn statement_effect(&self, trans: &mut impl GenKill<Self::Idx>, stmt: &mir::Statement<'tcx>, _: mir::Location) {
         match stmt.kind {
             mir::StatementKind::StorageLive(l) => trans.gen(l),
@@ -448,22 +458,17 @@ fn call_return_effect(
     }
 }
 
-impl BottomValue for MaybeStorageLive {
-    /// 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>,
+    cx: &'a LateContext<'tcx>,
 }
 
 impl<'a, 'tcx> PossibleBorrowerVisitor<'a, 'tcx> {
-    fn new(cx: &'a LateContext<'a, 'tcx>, body: &'a mir::Body<'tcx>) -> Self {
+    fn new(cx: &'a LateContext<'tcx>, body: &'a mir::Body<'tcx>) -> Self {
         Self {
             possible_borrower: TransitiveRelation::default(),
             cx,
@@ -473,7 +478,7 @@ fn new(cx: &'a LateContext<'a, 'tcx>, body: &'a mir::Body<'tcx>) -> Self {
 
     fn into_map(
         self,
-        cx: &LateContext<'a, 'tcx>,
+        cx: &LateContext<'tcx>,
         maybe_live: ResultsCursor<'tcx, 'tcx, MaybeStorageLive>,
     ) -> PossibleBorrowerMap<'a, 'tcx> {
         let mut map = FxHashMap::default();
@@ -514,7 +519,10 @@ fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _
                 self.possible_borrower.add(borrowed.local, lhs);
             },
             other => {
-                if !ContainsRegion.visit_ty(place.ty(&self.body.local_decls, self.cx.tcx).ty) {
+                if ContainsRegion
+                    .visit_ty(place.ty(&self.body.local_decls, self.cx.tcx).ty)
+                    .is_continue()
+                {
                     return;
                 }
                 rvalue_locals(other, |rhs| {
@@ -536,7 +544,7 @@ fn visit_terminator(&mut self, terminator: &mir::Terminator<'_>, _loc: mir::Loca
             // 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) {
+            if ContainsRegion.visit_ty(&self.body.local_decls[*dest].ty).is_continue() {
                 return;
             }
 
@@ -555,8 +563,8 @@ fn visit_terminator(&mut self, terminator: &mir::Terminator<'_>, _loc: mir::Loca
 struct ContainsRegion;
 
 impl TypeVisitor<'_> for ContainsRegion {
-    fn visit_region(&mut self, _: ty::Region<'_>) -> bool {
-        true
+    fn visit_region(&mut self, _: ty::Region<'_>) -> ControlFlow<()> {
+        ControlFlow::BREAK
     }
 }
 
@@ -591,7 +599,7 @@ struct PossibleBorrowerMap<'a, 'tcx> {
 impl PossibleBorrowerMap<'_, '_> {
     /// 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_after(at);
+        self.maybe_live.seek_after_primary_effect(at);
 
         self.bitset.0.clear();
         let maybe_live = &mut self.maybe_live;