Support #[derive(Copy, Clone)] and #[derive(Clone)] for unions with Copy fields

[petrochenkov]

#[derive(Clone)] // OK, both `u8` and `u16` are `Copy`, `clone` is implemented as `memcpy`
union U {
    a: u8,
    b: u16,
}

This wasn't implemented in #36016 for reasons described in #36016 (comment)
cc #32836 #31414 @durka

[durka]

I knew that hack would come back to bite us :)

We can just remove it if we agree on the policy that all Clone + !Copy
types will eventually be fixed...

On Sat, Aug 27, 2016 at 12:03 PM, Vadim Petrochenkov <
[email protected]> wrote:

#[derive(Clone)] // OK, both u8 and u16 are Copy, clone is implemented as memcpy
union U {
a: u8,
b: u16,
}

This wasn't implemented in #36016
#36016 for reasons described in #36016
(comment)
#36016 (comment)
cc #32836 #32836 #31414
#31414 @durka
https://github.com/durka

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36043, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC3n3LCGRuz0AbI9KLb4xGplwrZxsYFks5qkF-6gaJpZM4JuuYD
.

[durka]

Reproducing @petrochenkov's comment from the union PR:

There are some difficulties with the current implementation of #[derive(Clone)], it generates pattern matching even for the "shallow clone" case to check bounds, but these patterns are not appropriate for unions, this needs to be done in some other way somehow in some other PR.

#[derive(Copy, Clone)]
struct S {
    a: u8,
    b: u8,
}

=>

impl ::std::clone::Clone for S {
    #[inline]
    fn clone(&self) -> S {
        match *self {

            S { a: ref __self_0_0, b: ref __self_0_1 } => { // <== BAD

                ::std::clone::assert_receiver_is_clone(&(*__self_0_0));
                ::std::clone::assert_receiver_is_clone(&(*__self_0_1));
                *self
            }
        }
    }
}
impl ::std::marker::Copy for S { }
struct S {
    a: u8,
    b: u8,
}

---

Can we instead remove the parameter from assert_receiver_is_clone and generate something like this?

// libcore
pub fn assert_receiver_is_clone<T: Clone + ?Sized>() {}

union U { a: u8, b: u16 }

impl ::std::clone::Clone for U {
    #[inline]
    fn clone(&self) -> U {
        ::std::clone::assert_receiver_is_clone::<u8>();
        ::std::clone::assert_receiver_is_clone::<u16>();
        *self
    }
}

[nagisa]

For unions derive(Clone) without derive(Copy) makes little sense IMO, since the fields must be Copy to derive either, so we should just require users to derive both at all times for unions.

[Stebalien]

(@durka pedantic !Clone + Copy)

[durka]

@Stebalien oops yeah.

On Sat, Aug 27, 2016 at 8:46 PM, Steven Allen [email protected]
wrote:

(@durka https://github.com/durka pedantic !Clone + Copy)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36043 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC3nxS6M-5dEOg8k7g49UKYPvPSi_dXks5qkNpSgaJpZM4JuuYD
.

[petrochenkov]

By the way, why do #[derive(Clone, Copy)] and #[derive(Copy, Clone)] generate different code? (Unoptimized in the first case.)

[durka]

Uh, they aren't supposed to. I added the check to the #[derive_Clone] handler because I figured even if you wrote #[derive(Clone, Copy)] then all the attributes would be attached before the handlers are called. But I guess I thought wrong? Or maybe the behavior changed?