Auto merge of rust-lang#11508 - Jarcho:issue_11474, r=blyxyas

Lint `needless_borrow` and `explicit_auto_deref` on most union field accesses Changes both lints to follow rustc's rules around auto-deref through `ManuallyDrop` union fields rather than just bailing on union fields. changelog: [`needless_borrow`] & [`explicit_auto_deref`]: Lint on most union field accesses

Author: bors

clippy_lints/src/dereference.rs

@@ -1266,3 +1266,8 @@ pub fn normalize_with_regions<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>
  Err(_) => ty,
  }
 }
+
+/// Checks if the type is `core::mem::ManuallyDrop<_>`
+pub fn is_manually_drop(ty: Ty<'_>) -> bool {
+ ty.ty_adt_def().map_or(false, AdtDef::is_manually_drop)
+}

clippy_utils/src/ty.rs

@@ -301,24 +301,47 @@ fn main() {
  };
 
  // Issue #11474
- pub struct Variant {
- pub anonymous: Variant0,
+ #[derive(Clone, Copy)]
+ struct Wrap<T>(T);
+ impl<T> core::ops::Deref for Wrap<T> {
+ type Target = T;
+ fn deref(&self) -> &T {
+ &self.0
+ }
  }
-
- pub union Variant0 {
- pub anonymous: std::mem::ManuallyDrop<Variant00>,
+ impl<T> core::ops::DerefMut for Wrap<T> {
+ fn deref_mut(&mut self) -> &mut T {
+ &mut self.0
+ }
  }
 
- pub struct Variant00 {
- pub anonymous: Variant000,
+ union U<T: Copy> {
+ u: T,
  }
 
- pub union Variant000 {
- pub val: i32,
+ #[derive(Clone, Copy)]
+ struct S8 {
+ x: &'static str,
  }
 
  unsafe {
- let mut p = core::mem::zeroed::<Variant>();
- (*p.anonymous.anonymous).anonymous.val = 1;
+ let mut x = U {
+ u: core::mem::ManuallyDrop::new(S8 { x: "" }),
+ };
+ let _ = &mut (*x.u).x;
+ let _ = &mut { x.u }.x;
+ let _ = &mut ({ *x.u }).x;
+
+ let mut x = U {
+ u: Wrap(core::mem::ManuallyDrop::new(S8 { x: "" })),
+ };
+ let _ = &mut (*x.u).x;
+ let _ = &mut { x.u }.x;
+ let _ = &mut ({ **x.u }).x;
+
+ let mut x = U { u: Wrap(S8 { x: "" }) };
+ let _ = &mut x.u.x;
+ let _ = &mut { x.u }.x;
+ let _ = &mut ({ *x.u }).x;
  }
 }

tests/ui/explicit_auto_deref.fixed

@@ -301,24 +301,47 @@ fn main() {
  };
 
  // Issue #11474
- pub struct Variant {
- pub anonymous: Variant0,
+ #[derive(Clone, Copy)]
+ struct Wrap<T>(T);
+ impl<T> core::ops::Deref for Wrap<T> {
+ type Target = T;
+ fn deref(&self) -> &T {
+ &self.0
+ }
  }
-
- pub union Variant0 {
- pub anonymous: std::mem::ManuallyDrop<Variant00>,
+ impl<T> core::ops::DerefMut for Wrap<T> {
+ fn deref_mut(&mut self) -> &mut T {
+ &mut self.0
+ }
  }
 
- pub struct Variant00 {
- pub anonymous: Variant000,
+ union U<T: Copy> {
+ u: T,
  }
 
- pub union Variant000 {
- pub val: i32,
+ #[derive(Clone, Copy)]
+ struct S8 {
+ x: &'static str,
  }
 
  unsafe {
- let mut p = core::mem::zeroed::<Variant>();
- (*p.anonymous.anonymous).anonymous.val = 1;
+ let mut x = U {
+ u: core::mem::ManuallyDrop::new(S8 { x: "" }),
+ };
+ let _ = &mut (*x.u).x;
+ let _ = &mut (*{ x.u }).x;
+ let _ = &mut ({ *x.u }).x;
+
+ let mut x = U {
+ u: Wrap(core::mem::ManuallyDrop::new(S8 { x: "" })),
+ };
+ let _ = &mut (**x.u).x;
+ let _ = &mut (**{ x.u }).x;
+ let _ = &mut ({ **x.u }).x;
+
+ let mut x = U { u: Wrap(S8 { x: "" }) };
+ let _ = &mut (*x.u).x;
+ let _ = &mut (*{ x.u }).x;
+ let _ = &mut ({ *x.u }).x;
  }
 }

tests/ui/explicit_auto_deref.rs

@@ -241,5 +241,35 @@ error: deref which would be done by auto-deref
 LL | Some(x) => &mut *x,
  | ^^^^^^^ help: try: `x`
 
-error: aborting due to 40 previous errors
+error: deref which would be done by auto-deref
+ --> $DIR/explicit_auto_deref.rs:332:22
+ |
+LL | let _ = &mut (*{ x.u }).x;
+ | ^^^^^^^^^^ help: try: `{ x.u }`
+
+error: deref which would be done by auto-deref
+ --> $DIR/explicit_auto_deref.rs:338:22
+ |
+LL | let _ = &mut (**x.u).x;
+ | ^^^^^^^ help: try: `(*x.u)`
+
+error: deref which would be done by auto-deref
+ --> $DIR/explicit_auto_deref.rs:339:22
+ |
+LL | let _ = &mut (**{ x.u }).x;
+ | ^^^^^^^^^^^ help: try: `{ x.u }`
+
+error: deref which would be done by auto-deref
+ --> $DIR/explicit_auto_deref.rs:343:22
+ |
+LL | let _ = &mut (*x.u).x;
+ | ^^^^^^ help: try: `x.u`
+
+error: deref which would be done by auto-deref
+ --> $DIR/explicit_auto_deref.rs:344:22
+ |
+LL | let _ = &mut (*{ x.u }).x;
+ | ^^^^^^^^^^ help: try: `{ x.u }`
+
+error: aborting due to 45 previous errors
 

tests/ui/explicit_auto_deref.stderr

@@ -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);
-
- (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]);
- ManuallyDrop::drop(&mut (*ocean.coral).crab);
-
- ManuallyDrop::drop(&mut ocean.coral);
+ 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;
+
+ 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;
+
+ 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.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);
-
- (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]);
- ManuallyDrop::drop(&mut (*ocean.coral).crab);
-
- ManuallyDrop::drop(&mut ocean.coral);
+ 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;
+
+ 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;
+
+ 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.rs

@@ -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