Lint needless_borrow on most union field accesses

clippy_lints/src/dereference.rs

@@ -5,6 +5,7 @@ use clippy_utils::ty::{implements_trait, peel_mid_ty_refs};
 use clippy_utils::{
     expr_use_ctxt, get_parent_expr, get_parent_node, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode,
 };
+use core::mem;
 use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::Applicability;
@@ -342,8 +343,24 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                             TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return())
                         });
                         let can_auto_borrow = match use_cx.node {
-                            ExprUseNode::Callee => true,
-                            ExprUseNode::FieldAccess(_) => adjusted_ty.ty_adt_def().map_or(true, |adt| !adt.is_union()),
+                            ExprUseNode::FieldAccess(_)
+                                if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
+                            {
+                                // `ManuallyDrop` fields of a union will not automatically call
+                                // `deref_mut` when accessing a field.
+                                // Note: if the `ManuallyDrop` value is not directly a field of a
+                                // union then `DerefMut` will work as normal.
+                                //
+                                // This means we need to not modify an expression such as `(&mut x.y).z`
+                                // if accessing `z` would require `DerefMut`.
+                                let mut ty = expr_ty;
+                                !use_cx.adjustments.iter().any(|a| {
+                                    let ty = mem::replace(&mut ty, a.target);
+                                    matches!(a.kind, Adjust::Deref(Some(ref op)) if op.mutbl == Mutability::Mut)
+                                        && ty.ty_adt_def().map_or(false, |def| def.is_manually_drop())
+                                })
+                            },
+                            ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true,
                             ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
                                 // Check for calls to trait methods where the trait is implemented
                                 // on a reference.

tests/ui/needless_borrow.fixed

@@ -190,27 +190,48 @@ fn issue9383() {
     // Should not lint because unions need explicit deref when accessing field
     use std::mem::ManuallyDrop;
 
-    union Coral {
-        crab: ManuallyDrop<Vec<i32>>,
+    #[derive(Clone, Copy)]
+    struct Wrap<T>(T);
+    impl<T> core::ops::Deref for Wrap<T> {
+        type Target = T;
+        fn deref(&self) -> &T {
+            &self.0
+        }
+    }
+    impl<T> core::ops::DerefMut for Wrap<T> {
+        fn deref_mut(&mut self) -> &mut T {
+            &mut self.0
+        }
     }
 
-    union Ocean {
-        coral: ManuallyDrop<Coral>,
+    union U<T: Copy> {
+        u: T,
     }
 
-    let mut ocean = Ocean {
-        coral: ManuallyDrop::new(Coral {
-            crab: ManuallyDrop::new(vec![1, 2, 3]),
-        }),
-    };
+    #[derive(Clone, Copy)]
+    struct Foo {
+        x: u32,
+    }
 
     unsafe {
-        ManuallyDrop::drop(&mut (&mut ocean.coral).crab);
+        let mut x = U {
+            u: ManuallyDrop::new(Foo { x: 0 }),
+        };
+        let _ = &mut (&mut x.u).x;
+        let _ = &mut { x.u }.x;
+        let _ = &mut ({ &mut x.u }).x;
 
-        (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]);
-        ManuallyDrop::drop(&mut (*ocean.coral).crab);
+        let mut x = U {
+            u: Wrap(ManuallyDrop::new(Foo { x: 0 })),
+        };
+        let _ = &mut (&mut x.u).x;
+        let _ = &mut { x.u }.x;
+        let _ = &mut ({ &mut x.u }).x;
 
-        ManuallyDrop::drop(&mut ocean.coral);
+        let mut x = U { u: Wrap(Foo { x: 0 }) };
+        let _ = &mut x.u.x;
+        let _ = &mut { x.u }.x;
+        let _ = &mut ({ &mut x.u }).x;
     }
 }
 

tests/ui/needless_borrow.rs

@@ -190,27 +190,48 @@ fn issue9383() {
     // Should not lint because unions need explicit deref when accessing field
     use std::mem::ManuallyDrop;
 
-    union Coral {
-        crab: ManuallyDrop<Vec<i32>>,
+    #[derive(Clone, Copy)]
+    struct Wrap<T>(T);
+    impl<T> core::ops::Deref for Wrap<T> {
+        type Target = T;
+        fn deref(&self) -> &T {
+            &self.0
+        }
+    }
+    impl<T> core::ops::DerefMut for Wrap<T> {
+        fn deref_mut(&mut self) -> &mut T {
+            &mut self.0
+        }
     }
 
-    union Ocean {
-        coral: ManuallyDrop<Coral>,
+    union U<T: Copy> {
+        u: T,
     }
 
-    let mut ocean = Ocean {
-        coral: ManuallyDrop::new(Coral {
-            crab: ManuallyDrop::new(vec![1, 2, 3]),
-        }),
-    };
+    #[derive(Clone, Copy)]
+    struct Foo {
+        x: u32,
+    }
 
     unsafe {
-        ManuallyDrop::drop(&mut (&mut ocean.coral).crab);
+        let mut x = U {
+            u: ManuallyDrop::new(Foo { x: 0 }),
+        };
+        let _ = &mut (&mut x.u).x;
+        let _ = &mut (&mut { x.u }).x;
+        let _ = &mut ({ &mut x.u }).x;
 
-        (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]);
-        ManuallyDrop::drop(&mut (*ocean.coral).crab);
+        let mut x = U {
+            u: Wrap(ManuallyDrop::new(Foo { x: 0 })),
+        };
+        let _ = &mut (&mut x.u).x;
+        let _ = &mut (&mut { x.u }).x;
+        let _ = &mut ({ &mut x.u }).x;
 
-        ManuallyDrop::drop(&mut ocean.coral);
+        let mut x = U { u: Wrap(Foo { x: 0 }) };
+        let _ = &mut (&mut x.u).x;
+        let _ = &mut (&mut { x.u }).x;
+        let _ = &mut ({ &mut x.u }).x;
     }
 }
 

tests/ui/needless_borrow.stderr

@@ -133,5 +133,29 @@ error: this expression borrows a value the compiler would automatically borrow
 LL |             (&mut self.f)()
    |             ^^^^^^^^^^^^^ help: change this to: `(self.f)`
 
-error: aborting due to 22 previous errors
+error: this expression borrows a value the compiler would automatically borrow
+  --> $DIR/needless_borrow.rs:221:22
+   |
+LL |         let _ = &mut (&mut { x.u }).x;
+   |                      ^^^^^^^^^^^^^^ help: change this to: `{ x.u }`
+
+error: this expression borrows a value the compiler would automatically borrow
+  --> $DIR/needless_borrow.rs:228:22
+   |
+LL |         let _ = &mut (&mut { x.u }).x;
+   |                      ^^^^^^^^^^^^^^ help: change this to: `{ x.u }`
+
+error: this expression borrows a value the compiler would automatically borrow
+  --> $DIR/needless_borrow.rs:232:22
+   |
+LL |         let _ = &mut (&mut x.u).x;
+   |                      ^^^^^^^^^^ help: change this to: `x.u`
+
+error: this expression borrows a value the compiler would automatically borrow
+  --> $DIR/needless_borrow.rs:233:22
+   |
+LL |         let _ = &mut (&mut { x.u }).x;
+   |                      ^^^^^^^^^^^^^^ help: change this to: `{ x.u }`
+
+error: aborting due to 26 previous errors