Skip to content

Copy derive is ignored by rustc #56008

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

Closed
stevenroose opened this issue Nov 16, 2018 · 10 comments
Closed

Copy derive is ignored by rustc #56008

stevenroose opened this issue Nov 16, 2018 · 10 comments

Comments

@stevenroose
Copy link

stevenroose commented Nov 16, 2018

(Related with #26925)

I have a type Amount defined like this (took all irrelevant pieces out):

use std::marker::PhantomData;

pub trait Precision {}

pub struct Satoshi;
impl Precision for Satoshi {}

type Inner = i64;

#[derive(Copy, Clone)]
pub struct Amount<P: Precision = Satoshi>(Inner, PhantomData<P>);

Now, in some unit test, I'm trying to do something like this (again, redacted):

#[test]
fn string_parse_roundtrip() {
    let amt = Amount::sat(42); // returns Amount<Satoshi>
    assert_eq!(Amount::from_str(&amt.to_string()), Ok(amt)); // line 647
    assert_eq!(Amount::from_str(&amt.to_string()), Ok(amt)); // line 648
    // repeats multiple times because in fact it's not to_string, 
    // but a slightly different method
}

For which rustc gives me the following error:

error[E0382]: use of moved value: `amt`                                                                                                                       
   --> src/util/amount.rs:648:38                                                                                                                              
    |                                                                                                                                                         
647 |         assert_eq!(Amount::from_str(&amt.to_string()), Ok(amt));                                                                           
    |                                                           --- value moved here                                                             
648 |         assert_eq!(Amount::from_str(&amt.to_string()), Ok(amt));                                                                      
    |                                      ^^^ value used here after move                                                                                     
    |                                                                                                                                                         
    = note: move occurs because `amt` has type `util::amount::Amount`, which does not implement the `Copy` trait 
@steveklabnik
Copy link
Member

I don't believe this is a bug. Your original code expands to

#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std;

use std::marker::PhantomData;

pub trait Precision {}

pub struct Satoshi;
impl Precision for Satoshi {}

type Inner = i64;

#[rustc_copy_clone_marker]
pub struct Amount<P: Precision = Satoshi>(Inner, PhantomData<P>);
#[automatically_derived]
#[allow(unused_qualifications)]
impl<P: ::std::marker::Copy + Precision> ::std::marker::Copy for Amount<P> {}
#[automatically_derived]
#[allow(unused_qualifications)]
impl<P: ::std::clone::Clone + Precision> ::std::clone::Clone for Amount<P> {
    #[inline]
    fn clone(&self) -> Amount<P> {
        match *self {
            Amount(ref __self_0_0, ref __self_0_1) => Amount(
                ::std::clone::Clone::clone(&(*__self_0_0)),
                ::std::clone::Clone::clone(&(*__self_0_1)),
            ),
        }
    }
}
~\tmp\foo [master +14 ~0 -0 !]>

Note that Copy is only implemented if P also implements Copy. Satoshi does not implement Copy, and therefore, neither does Amount.

@stevenroose
Copy link
Author

stevenroose commented Nov 16, 2018

Well, before I used PhantomData, this was true. I had to define Precision: Copy, Clone (the compiler complained). PhantomData implements Copy without the Copy requirement on the type parameter.

With this snippet, the compiler doesn't complain about the derives.

I double checked, if I remove that one specific unit test that tries to copy, all other tests run fine.

@stevenroose
Copy link
Author

Here is the original PR that contains this code: rust-bitcoin/rust-bitcoin#192

@cuviper
Copy link
Member

cuviper commented Nov 16, 2018

PhantomData implements Copy without the Copy requirement on the type parameter.

It does, but #[derive] doesn't try to be that clever. See #26925.

@bddap
Copy link

bddap commented Nov 17, 2018

I ran into the same issue.

use std::marker::PhantomData;

#[derive(Copy, Clone)]
struct SpookyThing<T> {
    phantom: PhantomData<*const T>,
}

#[derive(Clone)]
struct NotCopyable;

fn main() {
    let scary = SpookyThing::<NotCopyable> {
        phantom: PhantomData,
    };
    creep(scary, scary);
}

fn creep<T>(_creep1: SpookyThing<T>, _creep2: SpookyThing<T>) {}
error[E0382]: use of moved value: `scary`
  --> src/main.rs:15:18
   |                      
15 |     creep(scary, scary);
   |           -----  ^^^^^ value used here after move
   |           | 
   |           value moved here
   |
   = note: move occurs because `scary` has type `SpookyThing<NotCopyable>`, which does not implement the `Copy` trait

It seems derive pays attention to the type of template argument, not the actual data items.

@bddap
Copy link

bddap commented Nov 17, 2018

You can work around the issue by implementing Clone and Copy manually.

struct SpookyThing<T> {
    phantom: PhantomData<*const T>,
}

impl<T> Clone for SpookyThing<T> {
    fn clone(&self) -> Self {
        SpookyThing {
            phantom: PhantomData
        }
    }
}

impl<T> Copy for SpookyThing<T> {}

If you attempt to derive Copy, and implement Copy manually, rustc does report an error.

#[derive(Copy, Clone)]
struct SpookyThing<T> {
    phantom: PhantomData<*const T>,
}

impl<T> Clone for SpookyThing<T> {
    fn clone(&self) -> Self {
        SpookyThing {
            phantom: PhantomData
        }
    }
}

impl<T> Copy for SpookyThing<T> {}
error[E0119]: conflicting implementations of trait `std::marker::Copy` for type `SpookyThing<_>`:
  --> src/main.rs:3:10
   |
3  | #[derive(Copy, Clone)]
   |          ^^^^ conflicting implementation for `SpookyThing<_>`
...
16 | impl<T> Copy for SpookyThing<T> {}
   | ------------------------------- first implementation here

error[E0119]: conflicting implementations of trait `std::clone::Clone` for type `SpookyThing<_>`:
 --> src/main.rs:3:16
  |
3 | #[derive(Copy, Clone)]
  |                ^^^^^ conflicting implementation for `SpookyThing<_>`
...
8 | impl<T> Clone for SpookyThing<T> {
  | -------------------------------- first implementation here

@stevenroose
Copy link
Author

Yeah I also implemented the traits manually as a workaround. I put the original issue (#26925) in the OP.

@rodrimati1992
Copy link
Contributor

rodrimati1992 commented Nov 19, 2018

You can work around this by creating a type alias which passes a PhantomData of P,
and using that type alias instead of the type.

use std::marker::PhantomData;

pub trait Precision {}

pub struct Satoshi;
impl Precision for Satoshi {}

impl<T:Precision> Precision for PhantomData<T>{}

type Inner = i64;

pub type Ammount<P=Satoshi>
    __Ammount<PhantomData<P>>;

#[derive(Copy, Clone)]
pub struct __Amount<P: Precision>(
    Inner,
    PhantomData<P>
);

@stevenroose
Copy link
Author

@rodrimati1992 Hmm I don't see how that is better than just implementing trivial Copy and Clone traits on the type manually.

@jonas-schievink
Copy link
Contributor

This is a duplicate of #26925, I suggest closing in favor of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants