Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

#[derive(Clone, Copy)] doesn't work #108894

Open
vporton opened this issue Mar 8, 2023 · 3 comments
Open

#[derive(Clone, Copy)] doesn't work #108894

vporton opened this issue Mar 8, 2023 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@vporton
Copy link

vporton commented Mar 8, 2023

I tried this code:

use std::marker::PhantomData;

#[derive(Clone, Copy)]
pub struct TypedAddress<T>{
    inner: u64,
    phantom: PhantomData<T>,
}

pub trait Memory {
    fn write_value<T>(&self, offset: TypedAddress<T>, value: &T);
    fn return_value<T>(&self, offset: TypedAddress<T>) -> T;
    fn update_value<T, F>(&self, offset: TypedAddress<T>, update: F)
        where F: FnOnce(T) -> T
    {
        let old = self.return_value(offset);
        let new = update(old);
        self.write_value(offset, &new);
    }
}

I expected to see it compiled.

Instead, this happened:

error[E0382]: use of moved value: `offset`
  --> src/test.rs:17:26
   |
12 |     fn update_value<T, F>(&self, offset: TypedAddress<T>, update: F)
   |                                  ------ move occurs because `offset` has type `test::TypedAddress<T>`, which does not implement the `Copy` trait
...
15 |         let old = self.return_value(offset);
   |                                     ------ value moved here
16 |         let new = update(old);
17 |         self.write_value(offset, &new);
   |                          ^^^^^^ value used here after move
   |
help: consider further restricting type parameter `T`
   |
13 |         where F: FnOnce(T) -> T, T: Copy
   |                                +++++++++

error[E0382]: use of moved value: `offset`
  --> src/ic.rs:53:26
   |
49 |     fn update_value<T, F>(&self, offset: TypedAddress<T>, update: F)
   |                                  ------ move occurs because `offset` has type `ic::TypedAddress<T>`, which does not implement the `Copy` trait
...
52 |         let old = self.return_value(offset);
   |                                     ------ value moved here
53 |         self.write_value(offset, &update(old));
   |                          ^^^^^^ value used here after move
   |
help: consider further restricting type parameter `T`
   |
50 |         where F: FnOnce(T) -> T, T: Copy
   |                                +++++++++

For more information about this error, try `rustc --explain E0382`.

Meta

rustc --version --verbose:

rustc 1.67.1 (d5a82bbd2 2023-02-07)
binary: rustc
commit-hash: d5a82bbd26e1ad8b7401f6a718a9c57c96905483
commit-date: 2023-02-07
host: x86_64-unknown-linux-gnu
release: 1.67.1
LLVM version: 15.0.6
$ rustc --version --verbose
rustc 1.69.0-nightly (9aa5c24b7 2023-02-17)
binary: rustc
commit-hash: 9aa5c24b7d763fb98d998819571128ff2eb8a3ca
commit-date: 2023-02-17
host: x86_64-unknown-linux-gnu
release: 1.69.0-nightly
LLVM version: 15.0.7
@vporton vporton added the C-bug Category: This is a bug. label Mar 8, 2023
@SkiFire13
Copy link
Contributor

Currently derive macros don't use the best possible bounds, but instead always bound the generic parameters by the given trait. See #26925 for why it's like this. This means your derive macro expands to something like this:

#[automatically_derived]
impl<T: ::core::clone::Clone> ::core::clone::Clone for TypedAddress<T> {
    #[inline]
    fn clone(&self) -> TypedAddress<T> {
        TypedAddress {
            inner: ::core::clone::Clone::clone(&self.inner),
            phantom: ::core::clone::Clone::clone(&self.phantom),
        }
    }
}
#[automatically_derived]
impl<T: ::core::marker::Copy> ::core::marker::Copy for TypedAddress<T> { }

Notice the T: ::core::marker::Copy bound. Since your update_value doesn't bound T: Copy then TypedAddress<T> doesn't implement Copy like you wanted.

You can solve this by manually implementing Copy and Clone like this:

impl<T> Clone for TypedAddress<T> {
    fn clone(&self) -> Self {
        *self
    }
}
impl<T> Copy for TypedAddress<T> {}

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints 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. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` and removed C-bug Category: This is a bug. labels Mar 14, 2023
@estebank
Copy link
Contributor

Given that this is true for all built-in derive macros, rustc should out-right state what @SkiFire13 explains above. Doing so might be as hard as implementing "perfect derives", but then again it might not be.

se-mo added a commit to se-mo/nearly that referenced this issue Aug 24, 2023
Copy and Clone are implemented explicitly since deriving these
traits doesn't work because of the generics used.
See related issue: rust-lang/rust#108894
@kgiebeler-xq-tec
Copy link

kgiebeler-xq-tec commented Nov 10, 2023

I have a similar problem with Partial/Eq where manual implementation has the drawback of breaking pattern-matching: https://users.rust-lang.org/t/derive-partialeq-eq-not-working-with-inner-types/102338/1

Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e0bd07c4577c18c44e11644201cebaed

use core::marker::PhantomData;

#[derive(PartialEq, Eq)]
pub struct Id<T>(PhantomData<T>);

// manual implementation which would break the usage of const patterns
// impl<T> PartialEq for Id<T> { fn eq(&self, _: &Id<T>) -> bool { true } }
// impl<T> Eq for Id<T> {}

// This derive is undesired but cannot be removed without
// breaking the usages below
#[derive(PartialEq, Eq)]
struct SomeNode();

fn accept_eq(_: &impl PartialEq) { }

fn main() {
    let node = Id::<SomeNode>(PhantomData);

    // this will only work if
    // - `Partial/Eq` is implemented manually, or
    // - `SomeNode` also needlessly(?) implements `Partial/Eq`
    // otherwise: error[E0277]: can't compare `SomeNode` with `SomeNode`
    accept_eq(&node);
    
    const CONST_ID: Id::<SomeNode> = Id::<SomeNode>(PhantomData);
    // this will work only when `Partial/Eq` is being derived
    // otherwise: error: to use a constant of type `Id<SomeNode>` in a pattern,
    //   `Id<SomeNode>` must be annotated with `#[derive(PartialEq, Eq)]`
    match node {
        CONST_ID => {}
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. 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