-
Notifications
You must be signed in to change notification settings - Fork 25
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
Implement derives for generic wrapper types #37
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool -- Generics::split_for_impl
makes this a lot easier than I thought it would be.
Could you rebase for the conflicts from #35? Mostly just need to change _num_traits
to a dynamic #import
.
2caa306
to
f2c286e
Compare
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just missed a couple spots.
f2c286e
to
c9764cd
Compare
oops, resolved and grepped to see if there are any others (there weren't). |
Oh. I just realized that #[derive(
Debug,
Clone,
Copy,
PartialEq,
PartialOrd,
ToPrimitive,
FromPrimitive,
NumOps,
NumCast,
One,
Zero,
Num,
)]
struct MyThing<T>(Wrapping<T>); does not work, because there's no |
)] | ||
struct MyThing<T>(Wrapping<T>); | ||
|
||
impl<T> PartialEq for MyThing<T> where Wrapping<T>: PartialEq { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Num
requires PartialEq
, and the libstd derive causes PartialEq
to get a T: PartialEq
bound instead of a Wrapping<T>: PartialEq
bound, this workaround is required to use the new num derives from this PR with such types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a meaningful difference? The std
implementation looks like this:
impl<T> PartialEq<Wrapping<T>> for Wrapping<T>
where
T: PartialEq<T>,
You can't impl PartialEq for Wrapping<LocalType>
, so it seem like T: PartialEq
and Wrapping<T>: PartialEq
should be equivalent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why there's a difference, but there definitely is one. I'll look into it
Hmm, now it seems like we're in that gray area of rust-lang/rust#26925, where it's not really obvious whether you should want bounds on the generic parameters or on the structural types. By adding So far we've avoided this question because the newtype derivations are unconditional. |
fixes #19