The error looks to be specific to ManuallyDrop rather than Copy, e.g. this is an error even though everything is Copy

#[derive(Copy, Clone)]
struct Data {
    num: u64,
}

union U {
    data: ManuallyDrop<Data>,
}

fn x(mut a: U) {
    unsafe {
        a.data.num = 3;
    }
}

*a.x = 0; doesn't hit it because it doesn't use DerefMut

https://github.com/rust-lang/rust/blob/705421b52239d7393b4738764b192179d5c139c2/compiler/rustc_hir_typeck/src/place_op.rs#L311-L323

The error looks to be specific to ManuallyDrop rather than Copy, e.g. this is an error even though everything is Copy

Indeed. And it requires going through the parent exprs to check if a ManuallyDrop is mutably dereferenced. I have reused some code from reference.rs and shared it in clippy_utils::ty.

Currently this lints

use std::mem::ManuallyDrop;

#[derive(Clone, Copy)]
struct B {
    md: ManuallyDrop<[u8; 4]>
}

union U {
    b: B,
}

fn x(mut u: U) {
    unsafe {
        (*&mut u.b.md)[3] = 1;
        //     ~~~~~~ `expr`
    }
}

Currently we only check if u.b is a union, but we need to keep going while we still see field/index expressions since it doesn't have to directly in the union

The parent iter I believe can be replaced with is_manually_drop([type of expr])

I'll check this, thanks for the review. @rustbot author

Done, the cases you mention, and some similar ones, are no longer linted.

However, I am not sure that

The parent iter I believe can be replaced with is_manually_drop([type of expr])

holds, as not all ManuallyDrop are equal here: **&mut a.prim = 0 is linted and fixed as *a.prim = 0, as there is an explicit deref left. However, (*&mut a.data).num = 42 is currently no longer linted, as a.data.num = 42 would be an implicit deref, but we could suggest to keep the explicit dereference instead, as in (*a.data).num = 42.

I'll think more about this.

This is not as simple as keeping the reference and removing the dereference in some case where the implicit dereference happens later in the expression:

(*&mut a.tup).0.num = 42;

should be rewritten as

(*a.tup.0).num = 42;

if we want to remove the &num. That would require moving the explicit dereference to the point where the implicit DerefMut would take place, so this case should not be linted.

I've added a new commit which reinstates those cases and suggests the proper fix. I may squash the two commits after the review.

One thing I have not checked is if it is really wise to keep linting in macros, as the lint is currently doing. If a macro is used with a ManuallyDrop field reached through a union, the lint may incorrectly suggest fixing the macro in cases where it works fine, which would generate an error in other cases. I would be in favor of disabling the lint in macros, even if we have false negatives, this would be better than false positives. Thoughts?

@rustbot ready