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

Warn on non-derived Clone impl on Copy types? #31086

Closed
SimonSapin opened this issue Jan 21, 2016 · 9 comments
Closed

Warn on non-derived Clone impl on Copy types? #31086

SimonSapin opened this issue Jan 21, 2016 · 9 comments

Comments

@SimonSapin
Copy link
Contributor

From #27750 (comment)

a Clone impl can diverge from the Copy one (because Clone is manually done)

This sounds bad, maybe?

Should we have a lint that warns in this case? Maybe even make it an error after a while?

Sort of related: #31085

@sfackler
Copy link
Member

There are cases where you can't rely on the derived versions (the bounds may be overly conservative for example), but warning when one of the impls is derived an the other is not may be doable?

@alexcrichton
Copy link
Member

I would not want a lint for something like this personally as there are types that are Copy but not Clone (unfortunately), such as:

  • Large fixed-size arrays
  • Many kinds of function pointers
  • Large tuples I believe

I think this is a compiler bug kinda but it's unfortunately what we have today. This means that #[derive(Clone, Copy)] is not always possible if #[derive(Copy)] is present.

@SimonSapin
Copy link
Contributor Author

@sfackler Is there a case like that where you also have Copy?

@alexcrichton If this is a lint, #[allow(…)] can be used in these case which will hopefully become more rare as we fix compiler bugs/limitations.

@sfackler
Copy link
Member

struct Foo<T>(PhantomData<T>) is one case where you could want an unconditional impl of Copy and Clone for all T, not just T: Clone.

@alexcrichton
Copy link
Member

@SimonSapin indeed! My personal opinions on lints is to be far more restrictive to the point that if I ever have to write #![allow] at all I would rather the lint not exist (e.g. false positives in my opinion kill the usefulness of the lint).

Either way, however, that's just my personal opinion, and I don't want to digress this issue to talking about lines, I mainly just wanted to point out that such a lint would indeed have false positives (including the cases that @sfackler is mentioning)

@SimonSapin
Copy link
Contributor Author

Alright, that’s fair enough. Maybe this lint would belong in Clippy more than rustc? CC @Manishearth

@Manishearth
Copy link
Member

Yes, this makes sense, we just got a PR for a similar lint for Hash/PartialEq

@Manishearth
Copy link
Member

cc @mcarton

@steveklabnik
Copy link
Member

Closing since this has been implemented in Clippy.

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

5 participants