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 PartialOrd when possible #882

Closed
fitzgen opened this issue Aug 1, 2017 · 5 comments
Closed

Derive PartialOrd when possible #882

fitzgen opened this issue Aug 1, 2017 · 5 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Aug 1, 2017

Should be very similar to deriving Default, see src/ir/analysis/derive_default.rs and the CanDeriveDefault trait that gets used in codegen.

Easier than this, see #882 (comment) below.

@photoszzt
Copy link
Contributor

photoszzt commented Aug 15, 2017

@fitzgen Looking at the definition of PartialOrd, if you can implement PartialEq you should be able to implement PartialOrd.

@highfive
Copy link

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

@fitzgen
Copy link
Member Author

fitzgen commented Sep 18, 2017

@fitzgen Looking at the definition of PartialOrd, if you can implement PartialEq you should be able to implement PartialOrd.

(Somehow missed this comment when it was originally posted)

This insight should make it rather easy to implement this functionality then :)

I think we can just rename the analysis from cannot_derive_partialeq to cannot_derive_partialeq_or_partialord (and other relevant names) and then all we have to do is add the new members to BindgenOptions and Builder to track whether the user has requested deriving PartialOrd, and in src/codegen/mod.rs where we emit the derive(...) bits, check the same results as we do for PartialEq when emitting PartialOrd.

@pepyakin
Copy link
Contributor

@highfive: assign me

@highfive
Copy link

Hey @pepyakin! Thanks for your interest in working on this issue. It's now assigned to you!

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

No branches or pull requests

4 participants