Skip to content

Derived Clone impl is not simplified if Clone is derived before Copy #124794

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
LunarLambda opened this issue May 6, 2024 · 3 comments
Open
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@LunarLambda
Copy link

Of these 4 ways to derive Copy and Clone:

#[derive(Copy, Clone)]
struct Foo(i32);

#[derive(Clone, Copy)]
struct Bar(i32);

#[derive(Copy)]
#[derive(Clone)]
struct Baz(i32);

#[derive(Clone)]
#[derive(Copy)]
struct Qux(i32);

The last one (derive Clone, then derive Copy) does not use the simplified form of the Clone derive for Copy types:

struct Foo(i32);
#[automatically_derived]
impl ::core::marker::Copy for Foo { }
#[automatically_derived]
impl ::core::clone::Clone for Foo {
    #[inline]
    fn clone(&self) -> Foo {
        let _: ::core::clone::AssertParamIsClone<i32>;
        *self
    }
}

struct Bar(i32);
#[automatically_derived]
impl ::core::clone::Clone for Bar {
    #[inline]
    fn clone(&self) -> Bar {
        let _: ::core::clone::AssertParamIsClone<i32>;
        *self
    }
}
#[automatically_derived]
impl ::core::marker::Copy for Bar { }

struct Baz(i32);
#[automatically_derived]
impl ::core::clone::Clone for Baz {
    #[inline]
    fn clone(&self) -> Baz {
        let _: ::core::clone::AssertParamIsClone<i32>;
        *self
    }
}
#[automatically_derived]
impl ::core::marker::Copy for Baz { }

struct Qux(i32);
#[automatically_derived]
impl ::core::marker::Copy for Qux { }
#[automatically_derived]
impl ::core::clone::Clone for Qux {
    #[inline]
    fn clone(&self) -> Qux { Qux(::core::clone::Clone::clone(&self.0)) }
}

I don't think this leads to any problems currently, but it's an odd (kind of amusing) inconsistency and may (citation needed) make for slightly worse codegen.

@LunarLambda LunarLambda added the C-bug Category: This is a bug. label May 6, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 6, 2024
@theemathas
Copy link
Contributor

See also #31085

@saethlin saethlin added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 11, 2024
@saethlin
Copy link
Member

I don't think this leads to any problems currently, but it's an odd (kind of amusing) inconsistency and may (citation needed) make for slightly worse codegen.

The codegen is fine: https://godbolt.org/z/58ovG4vvb (all the functions get deduplicated together, delete all but Qux if you want to be sure).

The MIR though is quite silly, and not going to help compile time:

fn <impl at /app/example.rs:8:10: 8:15>::clone(_1: &Baz) -> Baz {
    debug self => _1;
    let mut _0: Baz;
    scope 1 {
    }

    bb0: {
        _0 = (*_1);
        return;
    }
}

fn <impl at /app/example.rs:11:10: 11:15>::clone(_1: &Qux) -> Qux {
    debug self => _1;
    let mut _0: Qux;
    let mut _2: i32;
    let mut _3: &i32;
    scope 1 (inlined clone::impls::<impl Clone for i32>::clone) {
        debug self => _3;
    }

    bb0: {
        StorageLive(_2);
        StorageLive(_3);
        _3 = &((*_1).0: i32);
        _2 = ((*_1).0: i32);
        StorageDead(_3);
        _0 = Qux(move _2);
        StorageDead(_2);
        return;
    }
}

@theemathas
Copy link
Contributor

Turns out that, in some cases, the codegen is not fine #128081 (comment)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants