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

PhantomData should implement Clone #29005

Closed
Yoric opened this issue Oct 12, 2015 · 7 comments
Closed

PhantomData should implement Clone #29005

Yoric opened this issue Oct 12, 2015 · 7 comments

Comments

@Yoric
Copy link
Contributor

Yoric commented Oct 12, 2015

I don't see any good reason to prevent it, and this would have saved me lots of manual implementations of Clone.

@apasel422
Copy link
Contributor

PhantomData does implement Clone: https://doc.rust-lang.org/stable/std/marker/struct.PhantomData.html.

What version of Rust are you using? Can you post some source code that doesn't compile due to this issue?

@alexcrichton
Copy link
Member

Agreed with @apasel422, so closing, but feel free to post an error message here so we can help debug!

@Yoric
Copy link
Contributor Author

Yoric commented Oct 13, 2015

Here's a minimized misleading sample:

use std::marker::PhantomData;

trait Foo<T>: Clone {
}

#[derive(Clone)]
pub struct KeyedIgnoring<T> {
    witness: PhantomData<T>,
}

impl<T> Foo<T> for KeyedIgnoring<T> /* where Foo<T> : Clone */ {
    // error: the trait `core::clone::Clone` is not implemented for the type `T`
}

fn main() {
    println!("Hello, world!");
}

Now, if I uncomment where Foo<T>: Clone, this works. But I only tried this after you had closed the bug, because the error message led me to believe that the issue was in PhantomData.

@Yoric
Copy link
Contributor Author

Yoric commented Oct 13, 2015

Also, adding where Foo<T>: Clone doesn't scale very well in my code, as I find myself adding numerous such clauses for each impl.

@eefriedman
Copy link
Contributor

See #26925; you should be able to get around the issue by implementing Clone manually. (impl<T> Clone for KeyedIgnoring<T> { fn clone(&self) -> Self { KeyedIgnoring { witness: PhantomData } } }).

@Yoric
Copy link
Contributor Author

Yoric commented Oct 13, 2015

Yes, hence

this would have saved me lots of manual implementations of Clone

in my original comment.

As it is, I had to revert my change in which I added where Foo<T>: Clone, as it breaks client code.

@alexcrichton
Copy link
Member

Yes currently derive(Clone) will put a Clone requirement on all type parameters of the struct, so the dependency on Clone for T comes from KeyedIgnoring instead of the PhantomData itself.

hhvm-bot pushed a commit to facebook/hhvm that referenced this issue Jul 23, 2019
Summary:
Major refactoring of the parser in hope of squeezing
noticeable gains by not passing around SmartConstructors state.

Basically, replace the following:

  make_XYZ(state: State, ...) -> (State, R)

with

  make_XYZ(&mut self, ...) -> R

and make each of the 5 implementors of SmartConstructors stateful
by turning the old method `initial_state(...)` into constructor `new`.

Unfortunately, this means more boilerplate in the **user code**
(i.e., implementors of SmartConstructors):
- implementing Clone for each implementor of StateType
- implementing Clone for each implementor of SmartConstructors

this *cannot* be auto-generated via `#[derive(Clone)` because of
the [`PhantomData`](rust-lang/rust#29005 (comment)) that is needed to narrow down trait impl of `StateType`.

Reviewed By: shiqicao

Differential Revision: D16374645

fbshipit-source-id: 37c248e70d93dd505e5478a57b646ad469907d7c
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

4 participants