fix [needless_borrow] and [explicit_auto_deref] false positive on unions

Author: kraktus

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,11 +279,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
             (None, kind) => {
                 let expr_ty = typeck.expr_ty(expr);
                 let (position, adjustments) = walk_parents(cx, expr, self.msrv);
+                let is_field_of_union = is_field_of_union(typeck, sub_expr);
 
                 match kind {
                     RefOp::Deref => {
                         if let Position::FieldAccess(name) = position
                             && !ty_contains_field(typeck.expr_ty(sub_expr), name)
+                            && !is_field_of_union
                         {
                             self.state = Some((
                                 State::ExplicitDerefField { name },
@@ -355,7 +360,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
                         let borrow_msg = "this expression borrows a value the compiler would automatically borrow";
                         let impl_msg = "the borrowed expression implements the required traits";
 
-                        let (required_refs, msg, snip_expr) = if position.can_auto_borrow() {
+                        let (required_refs, msg, snip_expr) = if position.can_auto_borrow(is_field_of_union) {
                             (1, if deref_count == 1 { borrow_msg } else { deref_msg }, None)
                         } else if let Position::ImplArg(hir_id) = position {
                             (0, impl_msg, Some(hir_id))
@@ -572,6 +577,17 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
     extract_msrv_attr!(LateContext);
 }
 
+// deref needs to be explicit on union fields
+fn is_field_of_union<'tcx>(typeck: &'tcx TypeckResults<'_>, expr: &'tcx Expr<'_>) -> bool {
+    if let ExprKind::Field(path_expr, _) = expr.kind {
+        let ty = typeck.expr_ty_adjusted(path_expr);
+        if let Some(adt_def) = ty.ty_adt_def() {
+            return adt_def.is_union();
+        }
+    }
+    false
+}
+
 fn try_parse_ref_op<'tcx>(
     tcx: TyCtxt<'tcx>,
     typeck: &'tcx TypeckResults<'_>,
@@ -616,7 +632,7 @@ 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`
@@ -644,8 +660,9 @@ impl Position {
         matches!(self, Self::DerefStable(..) | Self::ReborrowStable(_))
     }
 
-    fn can_auto_borrow(self) -> bool {
-        matches!(self, Self::MethodReceiver | Self::FieldAccess(_) | Self::Callee)
+    fn can_auto_borrow(self, is_field_of_union: bool) -> bool {
+        // union fields cannot be auto borrowed
+        !is_field_of_union && matches!(self, Self::MethodReceiver | Self::FieldAccess(_) | Self::Callee)
     }
 
     fn lint_explicit_deref(self) -> bool {

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);
+    }
+}