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

rustc: Accept PartialOrd/PartialOrdEq for Eq/Ord #14492

Merged
merged 1 commit into from
May 29, 2014

Conversation

alexcrichton
Copy link
Member

This is a transitionary step towards completing #12517. This change modifies the
compiler to accept Partial{Ord,Eq} as deriving modes which will currently expand
to implementations of PartialOrd and PartialEq (synonyms for Eq/Ord).

After a snapshot, all of deriving(Eq, Ord) will be removed, and after a snapshot
of that, TotalEq/TotalOrd will be renamed to Eq/Ord.

This is a transitionary step towards completing rust-lang#12517. This change modifies the
compiler to accept Partial{Ord,Eq} as deriving modes which will currently expand
to implementations of PartialOrd and PartialEq (synonyms for Eq/Ord).

After a snapshot, all of deriving(Eq, Ord) will be removed, and after a snapshot
of that, TotalEq/TotalOrd will be renamed to Eq/Ord.
@pnkfelix
Copy link
Member

I cannot tell from #12517: Is the plan to support deriving(PartialEq) and deriving(PartialOrd) when this is all done, (in addition to supporting deriving(Eq) and deriving(Ord), that is)?

If so, then this PR should be updated to include tests of deriving(PartialEq) / deriving(PartialOrd), right?

@alexcrichton
Copy link
Member Author

That is the plan, yes. I have updated some examples in the gist of mine.

You won't be able to say deriving(Eq, PartialEq) on the same type, but you will be able to derive at most one of them (either one).

I could add some tests, but I figured that the common code path for Eq/PartialEq would cover it.

@pnkfelix
Copy link
Member

Its not just about being confident that your code is correct; tests in scenarios like this can be useful as reference examples when someone other than the PR author is grepping the code base.

It also sounds like there is behavior regarding the interaction when erroneously deriving both that would be worth including a compile-fail test for.

@pnkfelix
Copy link
Member

(but it also sounds like it would be premature to try to include the aforementioned compile-fail test now.)

bors added a commit that referenced this pull request May 29, 2014
This is a transitionary step towards completing #12517. This change modifies the
compiler to accept Partial{Ord,Eq} as deriving modes which will currently expand
to implementations of PartialOrd and PartialEq (synonyms for Eq/Ord).

After a snapshot, all of deriving(Eq, Ord) will be removed, and after a snapshot
of that, TotalEq/TotalOrd will be renamed to Eq/Ord.
@bors bors closed this May 29, 2014
@bors bors merged commit f786f9b into rust-lang:master May 29, 2014
@alexcrichton alexcrichton deleted the totaleq branch May 30, 2014 05:31
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

Successfully merging this pull request may close these issues.

3 participants