Auto merge of #9490 - kraktus:needless_borrow, r=Jarcho

fix [`needless_borrow`], [`explicit_auto_deref`] FPs on unions

fix #9383

changelog: fix [`needless_borrow`] false positive on unions
changelog: fix [`explicit_auto_deref`] false positive on unions

Left a couple debug derived impls on purpose I needed to debug as I don't think it's noise

Author: bors

clippy_lints/src/dereference.rs

@@ -184,19 +184,22 @@ impl Dereferencing {
     }
 }
 
+#[derive(Debug)]
 struct StateData {
     /// Span of the top level expression
     span: Span,
     hir_id: HirId,
     position: Position,
 }
 
+#[derive(Debug)]
 struct DerefedBorrow {
     count: usize,
     msg: &'static str,
     snip_expr: Option<HirId>,
 }
 
+#[derive(Debug)]
 enum State {
     // Any number of deref method calls.
     DerefMethod {
@@ -276,10 +279,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
             (None, kind) => {
                 let expr_ty = typeck.expr_ty(expr);
                 let (position, adjustments) = walk_parents(cx, expr, self.msrv);
-
                 match kind {
                     RefOp::Deref => {
-                        if let Position::FieldAccess(name) = position
+                        if let Position::FieldAccess {
+                            name,
+                            of_union: false,
+                        } = position
                             && !ty_contains_field(typeck.expr_ty(sub_expr), name)
                         {
                             self.state = Some((
@@ -451,7 +456,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
             (Some((State::DerefedBorrow(state), data)), RefOp::Deref) => {
                 let position = data.position;
                 report(cx, expr, State::DerefedBorrow(state), data);
-                if let Position::FieldAccess(name) = position
+                if let Position::FieldAccess{name, ..} = position
                     && !ty_contains_field(typeck.expr_ty(sub_expr), name)
                 {
                     self.state = Some((
@@ -616,14 +621,17 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool {
 }
 
 /// The position of an expression relative to it's parent.
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 enum Position {
     MethodReceiver,
     /// The method is defined on a reference type. e.g. `impl Foo for &T`
     MethodReceiverRefImpl,
     Callee,
     ImplArg(HirId),
-    FieldAccess(Symbol),
+    FieldAccess {
+        name: Symbol,
+        of_union: bool,
+    }, // union fields cannot be auto borrowed
     Postfix,
     Deref,
     /// Any other location which will trigger auto-deref to a specific time.
@@ -645,7 +653,10 @@ impl Position {
     }
 
     fn can_auto_borrow(self) -> bool {
-        matches!(self, Self::MethodReceiver | Self::FieldAccess(_) | Self::Callee)
+        matches!(
+            self,
+            Self::MethodReceiver | Self::FieldAccess { of_union: false, .. } | Self::Callee
+        )
     }
 
     fn lint_explicit_deref(self) -> bool {
@@ -657,7 +668,7 @@ impl Position {
             Self::MethodReceiver
             | Self::MethodReceiverRefImpl
             | Self::Callee
-            | Self::FieldAccess(_)
+            | Self::FieldAccess { .. }
             | Self::Postfix => PREC_POSTFIX,
             Self::ImplArg(_) | Self::Deref => PREC_PREFIX,
             Self::DerefStable(p, _) | Self::ReborrowStable(p) | Self::Other(p) => p,
@@ -844,7 +855,10 @@ fn walk_parents<'tcx>(
                         }
                     })
                 },
-                ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess(name.name)),
+                ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess {
+                    name: name.name,
+                    of_union: is_union(cx.typeck_results(), child),
+                }),
                 ExprKind::Unary(UnOp::Deref, child) if child.hir_id == e.hir_id => Some(Position::Deref),
                 ExprKind::Match(child, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar)
                 | ExprKind::Index(child, _)
@@ -865,6 +879,13 @@ fn walk_parents<'tcx>(
     (position, adjustments)
 }
 
+fn is_union<'tcx>(typeck: &'tcx TypeckResults<'_>, path_expr: &'tcx Expr<'_>) -> bool {
+    typeck
+        .expr_ty_adjusted(path_expr)
+        .ty_adt_def()
+        .map_or(false, rustc_middle::ty::AdtDef::is_union)
+}
+
 fn closure_result_position<'tcx>(
     cx: &LateContext<'tcx>,
     closure: &'tcx Closure<'_>,

tests/ui/needless_borrow.fixed

@@ -298,3 +298,32 @@ mod meets_msrv {
         let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap();
     }
 }
+
+#[allow(unused)]
+fn issue9383() {
+    // Should not lint because unions need explicit deref when accessing field
+    use std::mem::ManuallyDrop;
+
+    union Coral {
+        crab: ManuallyDrop<Vec<i32>>,
+    }
+
+    union Ocean {
+        coral: ManuallyDrop<Coral>,
+    }
+
+    let mut ocean = Ocean {
+        coral: ManuallyDrop::new(Coral {
+            crab: ManuallyDrop::new(vec![1, 2, 3]),
+        }),
+    };
+
+    unsafe {
+        ManuallyDrop::drop(&mut (&mut ocean.coral).crab);
+
+        (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]);
+        ManuallyDrop::drop(&mut (*ocean.coral).crab);
+
+        ManuallyDrop::drop(&mut ocean.coral);
+    }
+}

tests/ui/needless_borrow.rs

@@ -298,3 +298,32 @@ mod meets_msrv {
         let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
     }
 }
+
+#[allow(unused)]
+fn issue9383() {
+    // Should not lint because unions need explicit deref when accessing field
+    use std::mem::ManuallyDrop;
+
+    union Coral {
+        crab: ManuallyDrop<Vec<i32>>,
+    }
+
+    union Ocean {
+        coral: ManuallyDrop<Coral>,
+    }
+
+    let mut ocean = Ocean {
+        coral: ManuallyDrop::new(Coral {
+            crab: ManuallyDrop::new(vec![1, 2, 3]),
+        }),
+    };
+
+    unsafe {
+        ManuallyDrop::drop(&mut (&mut ocean.coral).crab);
+
+        (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]);
+        ManuallyDrop::drop(&mut (*ocean.coral).crab);
+
+        ManuallyDrop::drop(&mut ocean.coral);
+    }
+}