CloneUsage -> LocalUsage in mir.rs

Author: smoelius

clippy_lints/src/redundant_clone/mod.rs

@@ -1,4 +1,5 @@
 use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
+use clippy_utils::mir::{visit_local_usage, LocalUsage};
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::{has_drop, is_copy, is_type_diagnostic_item, walk_ptrs_ty_depth};
 use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths};
@@ -7,10 +8,7 @@ use rustc_errors::Applicability;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{def_id, Body, FnDecl, HirId};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::mir::{
-    self, traversal,
-    visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _},
-};
+use rustc_middle::mir::{self, visit::Visitor as _};
 use rustc_middle::ty::{self, Ty};
 use rustc_mir_dataflow::Analysis;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -381,86 +379,42 @@ struct CloneUsage {
     /// Whether the clone value is mutated.
     clone_consumed_or_mutated: bool,
 }
-fn visit_clone_usage(cloned: mir::Local, clone: mir::Local, mir: &mir::Body<'_>, bb: mir::BasicBlock) -> CloneUsage {
-    struct V {
-        cloned: mir::Local,
-        clone: mir::Local,
-        result: CloneUsage,
-    }
-    impl<'tcx> mir::visit::Visitor<'tcx> for V {
-        fn visit_basic_block_data(&mut self, block: mir::BasicBlock, data: &mir::BasicBlockData<'tcx>) {
-            let statements = &data.statements;
-            for (statement_index, statement) in statements.iter().enumerate() {
-                self.visit_statement(statement, mir::Location { block, statement_index });
-            }
 
-            self.visit_terminator(
-                data.terminator(),
-                mir::Location {
-                    block,
-                    statement_index: statements.len(),
-                },
-            );
+fn visit_clone_usage(cloned: mir::Local, clone: mir::Local, mir: &mir::Body<'_>, bb: mir::BasicBlock) -> CloneUsage {
+    if let Some(
+        &[
+            LocalUsage {
+                local_use_loc: cloned_use_loc,
+                local_consume_or_mutate_loc: cloned_consume_or_mutate_loc,
+            },
+            LocalUsage {
+                local_use_loc: _,
+                local_consume_or_mutate_loc: clone_consume_or_mutate_loc,
+            },
+        ],
+    ) = visit_local_usage(
+        &[cloned, clone],
+        mir,
+        mir::Location {
+            block: bb,
+            statement_index: mir.basic_blocks()[bb].statements.len(),
+        },
+    )
+    .as_deref()
+    {
+        CloneUsage {
+            cloned_used: cloned_use_loc.is_some(),
+            cloned_consume_or_mutate_loc,
+            // Consider non-temporary clones consumed.
+            // TODO: Actually check for mutation of non-temporaries.
+            clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp
+                || clone_consume_or_mutate_loc.is_some(),
         }
-
-        fn visit_place(&mut self, place: &mir::Place<'tcx>, ctx: PlaceContext, loc: mir::Location) {
-            let local = place.local;
-
-            if local == self.cloned
-                && !matches!(
-                    ctx,
-                    PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_)
-                )
-            {
-                self.result.cloned_used = true;
-                self.result.cloned_consume_or_mutate_loc = self.result.cloned_consume_or_mutate_loc.or_else(|| {
-                    matches!(
-                        ctx,
-                        PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)
-                            | PlaceContext::MutatingUse(MutatingUseContext::Borrow)
-                    )
-                    .then(|| loc)
-                });
-            } else if local == self.clone {
-                match ctx {
-                    PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)
-                    | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => {
-                        self.result.clone_consumed_or_mutated = true;
-                    },
-                    _ => {},
-                }
-            }
+    } else {
+        CloneUsage {
+            cloned_used: true,
+            cloned_consume_or_mutate_loc: None,
+            clone_consumed_or_mutated: true,
         }
     }
-
-    let init = CloneUsage {
-        cloned_used: false,
-        cloned_consume_or_mutate_loc: None,
-        // Consider non-temporary clones consumed.
-        // TODO: Actually check for mutation of non-temporaries.
-        clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp,
-    };
-    traversal::ReversePostorder::new(mir, bb)
-        .skip(1)
-        .fold(init, |usage, (tbb, tdata)| {
-            // Short-circuit
-            if (usage.cloned_used && usage.clone_consumed_or_mutated) ||
-                // Give up on loops
-                tdata.terminator().successors().any(|s| s == bb)
-            {
-                return CloneUsage {
-                    cloned_used: true,
-                    clone_consumed_or_mutated: true,
-                    ..usage
-                };
-            }
-
-            let mut v = V {
-                cloned,
-                clone,
-                result: usage,
-            };
-            v.visit_basic_block_data(tbb, tdata);
-            v.result
-        })
 }

clippy_utils/src/mir.rs

@@ -1,104 +1,70 @@
-use rustc_middle::mir::{
-    self, traversal,
-    visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _},
-};
+use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
+use rustc_middle::mir::{traversal, Body, Local, Location, Place};
 
-#[derive(Default)]
-pub struct CloneUsage {
-    /// Whether the cloned value is used after the clone.
-    pub cloned_used: bool,
-    /// The first location where the cloned value is consumed or mutated, if any.
-    pub cloned_consume_or_mutate_loc: Option<mir::Location>,
-    /// Whether the clone value is mutated.
-    pub clone_consumed_or_mutated: bool,
+#[derive(Clone, Default)]
+pub struct LocalUsage {
+    /// The first location where the local is used, if any.
+    pub local_use_loc: Option<Location>,
+    /// The first location where the local is consumed or mutated, if any.
+    pub local_consume_or_mutate_loc: Option<Location>,
 }
 
-pub fn visit_clone_usage(
-    cloned: mir::Local,
-    clone: mir::Local,
-    mir: &mir::Body<'_>,
-    bb: mir::BasicBlock,
-) -> CloneUsage {
-    let init = CloneUsage {
-        cloned_used: false,
-        cloned_consume_or_mutate_loc: None,
-        // Consider non-temporary clones consumed.
-        // TODO: Actually check for mutation of non-temporaries.
-        clone_consumed_or_mutated: mir.local_kind(clone) != mir::LocalKind::Temp,
-    };
+pub fn visit_local_usage(locals: &[Local], mir: &Body<'_>, location: Location) -> Option<Vec<LocalUsage>> {
+    let init = vec![
+        LocalUsage {
+            local_use_loc: None,
+            local_consume_or_mutate_loc: None,
+        };
+        locals.len()
+    ];
 
-    traversal::ReversePostorder::new(mir, bb)
-        .skip(1)
-        .fold(init, |usage, (tbb, tdata)| {
-            // Short-circuit
-            if (usage.cloned_used && usage.clone_consumed_or_mutated) ||
-                // Give up on loops
-                tdata.terminator().successors().any(|s| s == bb)
-            {
-                return CloneUsage {
-                    cloned_used: true,
-                    clone_consumed_or_mutated: true,
-                    ..usage
-                };
-            }
+    traversal::ReversePostorder::new(mir, location.block).try_fold(init, |usage, (tbb, tdata)| {
+        // Give up on loops
+        if tdata.terminator().successors().any(|s| s == location.block) {
+            return None;
+        }
 
-            let mut v = V {
-                cloned,
-                clone,
-                result: usage,
-            };
-            v.visit_basic_block_data(tbb, tdata);
-            v.result
-        })
+        let mut v = V {
+            locals,
+            location,
+            result: usage,
+        };
+        v.visit_basic_block_data(tbb, tdata);
+        Some(v.result)
+    })
 }
 
-struct V {
-    cloned: mir::Local,
-    clone: mir::Local,
-    result: CloneUsage,
+struct V<'a> {
+    locals: &'a [Local],
+    location: Location,
+    result: Vec<LocalUsage>,
 }
 
-impl<'tcx> mir::visit::Visitor<'tcx> for V {
-    fn visit_basic_block_data(&mut self, block: mir::BasicBlock, data: &mir::BasicBlockData<'tcx>) {
-        let statements = &data.statements;
-        for (statement_index, statement) in statements.iter().enumerate() {
-            self.visit_statement(statement, mir::Location { block, statement_index });
+impl<'a, 'tcx> Visitor<'tcx> for V<'a> {
+    fn visit_place(&mut self, place: &Place<'tcx>, ctx: PlaceContext, loc: Location) {
+        if loc.block == self.location.block && loc.statement_index <= self.location.statement_index {
+            return;
         }
 
-        self.visit_terminator(
-            data.terminator(),
-            mir::Location {
-                block,
-                statement_index: statements.len(),
-            },
-        );
-    }
-
-    fn visit_place(&mut self, place: &mir::Place<'tcx>, ctx: PlaceContext, loc: mir::Location) {
         let local = place.local;
 
-        if local == self.cloned
-            && !matches!(
-                ctx,
-                PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_)
-            )
-        {
-            self.result.cloned_used = true;
-            self.result.cloned_consume_or_mutate_loc = self.result.cloned_consume_or_mutate_loc.or_else(|| {
-                matches!(
-                    ctx,
-                    PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)
-                        | PlaceContext::MutatingUse(MutatingUseContext::Borrow)
-                )
-                .then(|| loc)
-            });
-        } else if local == self.clone {
-            match ctx {
-                PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)
-                | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => {
-                    self.result.clone_consumed_or_mutated = true;
-                },
-                _ => {},
+        for (i, self_local) in self.locals.iter().enumerate() {
+            if local == *self_local {
+                self.result[i].local_use_loc = self.result[i].local_use_loc.or_else(|| {
+                    (!matches!(
+                        ctx,
+                        PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_)
+                    ))
+                    .then_some(loc)
+                });
+                self.result[i].local_consume_or_mutate_loc = self.result[i].local_consume_or_mutate_loc.or_else(|| {
+                    matches!(
+                        ctx,
+                        PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)
+                            | PlaceContext::MutatingUse(MutatingUseContext::Borrow)
+                    )
+                    .then_some(loc)
+                });
             }
         }
     }