needless_borrow uses dropped_without_further_use

Author: smoelius

clippy_lints/src/dereference.rs

@@ -1,4 +1,5 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
+use clippy_utils::mir::{dropped_without_further_use, enclosing_mir, expr_local, local_assignments};
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
 use clippy_utils::sugg::has_enclosing_paren;
 use clippy_utils::ty::{expr_sig, is_copy, peel_mid_ty_refs, ty_sig, variant_of_res};
@@ -18,6 +19,7 @@ use rustc_hir::{
 use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::mir::{Rvalue, StatementKind};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 use rustc_middle::ty::{
     self, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind,
@@ -1082,10 +1084,10 @@ fn needless_borrow_impl_arg_position<'tcx>(
     // elements are modified each time `check_referent` is called.
     let mut substs_with_referent_ty = substs_with_expr_ty.to_vec();
 
-    let mut check_referent = |referent| {
+    let mut check_reference_and_referent = |reference, referent| {
         let referent_ty = cx.typeck_results().expr_ty(referent);
 
-        if !is_copy(cx, referent_ty) {
+        if !(is_copy(cx, referent_ty) || referent_dropped_without_further_use(cx.tcx, reference)) {
             return false;
         }
 
@@ -1127,7 +1129,7 @@ fn needless_borrow_impl_arg_position<'tcx>(
 
     let mut needless_borrow = false;
     while let ExprKind::AddrOf(_, _, referent) = expr.kind {
-        if !check_referent(referent) {
+        if !check_reference_and_referent(expr, referent) {
             break;
         }
         expr = referent;
@@ -1155,6 +1157,20 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool {
         })
 }
 
+fn referent_dropped_without_further_use(tcx: TyCtxt<'_>, reference: &Expr<'_>) -> bool {
+    let mir = enclosing_mir(tcx, reference.hir_id);
+    if let Some(local) = expr_local(tcx, reference)
+        && let [location] = *local_assignments(mir, local).as_slice()
+        && let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) =
+            mir.basic_blocks[location.block].statements[location.statement_index].kind
+        && !place.has_deref()
+    {
+        dropped_without_further_use(mir, place.local, location).unwrap_or(false)
+    } else {
+        false
+    }
+}
+
 // Iteratively replaces `param_ty` with `new_ty` in `substs`, and similarly for each resulting
 // projected type that is a type parameter. Returns `false` if replacing the types would have an
 // effect on the function signature beyond substituting `new_ty` for `param_ty`.

clippy_utils/src/mir.rs

@@ -1,5 +1,7 @@
+use rustc_hir::{Expr, HirId};
 use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
-use rustc_middle::mir::{traversal, Body, Local, Location, Place};
+use rustc_middle::mir::{traversal, Body, Local, Location, Place, StatementKind};
+use rustc_middle::ty::TyCtxt;
 
 #[derive(Clone, Default)]
 pub struct LocalUsage {
@@ -69,3 +71,52 @@ impl<'a, 'tcx> Visitor<'tcx> for V<'a> {
         }
     }
 }
+
+/// Convenience wrapper around `visit_local_usage`.
+pub fn dropped_without_further_use(mir: &Body<'_>, local: Local, location: Location) -> Option<bool> {
+    visit_local_usage(&[local], mir, location).map(|mut vec| {
+        let LocalUsage {
+            local_use_loc,
+            local_consume_or_mutate_loc,
+        } = vec.remove(0);
+        local_use_loc.is_none() && local_consume_or_mutate_loc.is_none()
+    })
+}
+
+/// Returns the `mir::Body` containing the node associated with `hir_id`.
+#[allow(clippy::module_name_repetitions)]
+pub fn enclosing_mir(tcx: TyCtxt<'_>, hir_id: HirId) -> &Body<'_> {
+    let body_owner_hir_id = tcx.hir().enclosing_body_owner(hir_id);
+    let body_id = tcx.hir().body_owned_by(body_owner_hir_id);
+    let body_owner_local_def_id = tcx.hir().body_owner_def_id(body_id);
+    tcx.optimized_mir(body_owner_local_def_id.to_def_id())
+}
+
+/// Tries to determine the `Local` corresponding to `expr`, if any.
+/// This function is expensive and should be used sparingly.
+pub fn expr_local(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> Option<Local> {
+    let mir = enclosing_mir(tcx, expr.hir_id);
+    mir.local_decls.iter_enumerated().find_map(|(local, local_decl)| {
+        if local_decl.source_info.span == expr.span {
+            Some(local)
+        } else {
+            None
+        }
+    })
+}
+
+/// Returns a vector of `mir::Location` where `local` is assigned. Each statement referred to has
+/// kind `StatementKind::Assign`.
+pub fn local_assignments(mir: &Body<'_>, local: Local) -> Vec<Location> {
+    let mut locations = Vec::new();
+    for (block, data) in mir.basic_blocks.iter_enumerated() {
+        for (statement_index, statement) in data.statements.iter().enumerate() {
+            if let StatementKind::Assign(box (place, _)) = statement.kind {
+                if place.as_local() == Some(local) {
+                    locations.push(Location { block, statement_index });
+                }
+            }
+        }
+    }
+    locations
+}

tests/ui/needless_borrow.fixed

@@ -3,7 +3,7 @@
 #![feature(custom_inner_attributes, lint_reasons)]
 
 #[warn(clippy::all, clippy::needless_borrow)]
-#[allow(unused_variables, clippy::unnecessary_mut_passed)]
+#[allow(unused_variables, clippy::unnecessary_mut_passed, clippy::unnecessary_to_owned)]
 fn main() {
     let a = 5;
     let ref_a = &a;
@@ -134,6 +134,7 @@ fn main() {
     multiple_constraints([[""]]);
     multiple_constraints_normalizes_to_same(X, X);
     let _ = Some("").unwrap_or("");
+    let _ = std::fs::write("x", "".to_string());
 
     only_sized(&""); // Don't lint. `Sized` is only bound
     let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
@@ -276,8 +277,9 @@ mod copyable_iterator {
     fn dont_warn(mut x: Iter) {
         takes_iter(&mut x);
     }
+    #[allow(unused_mut)]
     fn warn(mut x: &mut Iter) {
-        takes_iter(&mut x)
+        takes_iter(x)
     }
 }
 

tests/ui/needless_borrow.rs

@@ -3,7 +3,7 @@
 #![feature(custom_inner_attributes, lint_reasons)]
 
 #[warn(clippy::all, clippy::needless_borrow)]
-#[allow(unused_variables, clippy::unnecessary_mut_passed)]
+#[allow(unused_variables, clippy::unnecessary_mut_passed, clippy::unnecessary_to_owned)]
 fn main() {
     let a = 5;
     let ref_a = &a;
@@ -134,6 +134,7 @@ fn main() {
     multiple_constraints(&[[""]]);
     multiple_constraints_normalizes_to_same(&X, X);
     let _ = Some("").unwrap_or(&"");
+    let _ = std::fs::write("x", &"".to_string());
 
     only_sized(&""); // Don't lint. `Sized` is only bound
     let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
@@ -276,6 +277,7 @@ mod copyable_iterator {
     fn dont_warn(mut x: Iter) {
         takes_iter(&mut x);
     }
+    #[allow(unused_mut)]
     fn warn(mut x: &mut Iter) {
         takes_iter(&mut x)
     }

tests/ui/needless_borrow.stderr

@@ -156,23 +156,35 @@ error: this expression creates a reference which is immediately dereferenced by
 LL |     let _ = Some("").unwrap_or(&"");
    |                                ^^^ help: change this to: `""`
 
+error: the borrowed expression implements the required traits
+  --> $DIR/needless_borrow.rs:137:33
+   |
+LL |     let _ = std::fs::write("x", &"".to_string());
+   |                                 ^^^^^^^^^^^^^^^ help: change this to: `"".to_string()`
+
 error: this expression borrows a value the compiler would automatically borrow
-  --> $DIR/needless_borrow.rs:187:13
+  --> $DIR/needless_borrow.rs:188:13
    |
 LL |             (&self.f)()
    |             ^^^^^^^^^ help: change this to: `(self.f)`
 
 error: this expression borrows a value the compiler would automatically borrow
-  --> $DIR/needless_borrow.rs:196:13
+  --> $DIR/needless_borrow.rs:197:13
    |
 LL |             (&mut self.f)()
    |             ^^^^^^^^^^^^^ help: change this to: `(self.f)`
 
 error: the borrowed expression implements the required traits
-  --> $DIR/needless_borrow.rs:298:55
+  --> $DIR/needless_borrow.rs:282:20
+   |
+LL |         takes_iter(&mut x)
+   |                    ^^^^^^ help: change this to: `x`
+
+error: the borrowed expression implements the required traits
+  --> $DIR/needless_borrow.rs:300:55
    |
 LL |         let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
    |                                                       ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
 
-error: aborting due to 29 previous errors
+error: aborting due to 31 previous errors