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

Remove Ordering from the prelude. #497

Conversation

steveklabnik
Copy link
Member

Remove Ordering and its variants from the prelude.

rendered

@steveklabnik steveklabnik force-pushed the remove_ordering_from_the_prelude branch from ec90ca4 to f133ecc Compare December 5, 2014 13:28
@steveklabnik
Copy link
Member Author

/cc @bstrie because of rust-lang/rust#17967 (comment)

@Gankra
Copy link
Contributor

Gankra commented Dec 5, 2014

Edit: whoops, I'm in the rfcs repo. That learns me for reviewing PRs first thing in the morning.

@Gankra
Copy link
Contributor

Gankra commented Dec 5, 2014

This feels weird to me, but also pretty reasonable. Using the operators is overwhelmingly the convention. Having to branch specifically on Greater/Less/Equal is a very search-algorithm thing. This might become more common if/when we get some notion of comparators, but even then, most code will just pass the comparators around, not actually invoke them.

@ftxqxd
Copy link
Contributor

ftxqxd commented Dec 6, 2014

I’ve always liked the fact that Ordering is in the prelude—it’s just a cool and useful type. Combined with the fact that PartialOrd and Ord are in the prelude, it makes sense (to me) for Ordering and (perhaps) its variants to be in the prelude. If it weren’t in the prelude, then you’d have to import Ordering every time you implemented PartialOrd or Ord manually (or use the fully-qualified path), so the advantages of the Ord traits being in the prelude would be diminished somewhat.

The Guide could be adjusted to demonstrate enums using a type that doesn’t already exist in the standard library (or at least in the prelude). It seems (to me) strange to change a very commonly-used part of the standard library based on a specific piece of documentation.

@Gankra
Copy link
Contributor

Gankra commented Dec 6, 2014

Do the trait-based operators work without the trait in scope?

Edit: actually that shouldn't matter. It's pretty common to have a T: Ord or T: Eq bound in generic code without actually using the associated enum (because you can just do <= or whatever).

@steveklabnik
Copy link
Member Author

@P1start I see it as more 'the guide uncovering something that shouldn't always be there' than driven soley by the guide.

@steveklabnik
Copy link
Member Author

If it weren’t in the prelude, then you’d have to import Ordering every time you implemented PartialOrd or Ord manually

I don't think this is very common compared to deriving it. And similar traits aren't, for example, Show, which needs a fmt::Show when implementing.

@steveklabnik
Copy link
Member Author

It's pretty common to have a T: Ord or T: Eq bound in generic code without actually using the associated enum

I'm unsure how common this actually is, but it's true that I used Greater in my search.

@quantheory
Copy link
Contributor

Ordering and its variants were in the prelude before my time, and so I'm not 100% sure why they're in the prelude. I would imagine, like most things, it comes down to convenience: importing four things to match on == is kind of a hassle.

I find it a little jarring to see first person pronouns in RFCs, because if you are just looking in the RFC repo itself, you may not know (or remember) who "I" refers to. But to address the question itself, I suspect that at first this was due to the influence of other languages, e.g. Haskell has Ord in its Prelude.

Having to branch specifically on Greater/Less/Equal is a very search-algorithm thing.

Well, I've used it for searches, merging sorted data structures while weeding out duplicates, custom Ord implementations, and special code for boundaries of ranges. (E.g. there's data that you normally iterate through until reaching a special "end" value, but random access is also supported. So the three cases are normal accesses, the end-of-data value, and out-of-bounds errors.)

Still, I admit that I don't see Ordering being used overwhelmingly often. The main reasons I can think of to keep it in would be:

  • Ord::cmp is in the prelude, so it's counterintuitive if its return type isn't. I have a similar objection to range being in the prelude without Range, but ops reform will change that. At the same time, there are probably many other cases of this, so maybe I should just get over it.
  • IMO, Ordering is still important enough to the language that it's not a good idea to use names that would conflict with it or its variants. More to the point, the guide definitely shouldn't be written in a way that (even accidentally) encourages people to re-implement Ordering, or anything else in the standard library, and that would still be true if Ordering had never been in the prelude at all.

@quantheory
Copy link
Contributor

I have a similar objection to range being in the prelude without Range, but ops reform will change that.

Actually, thinking about it now, I'm not 100% sure what ops reform will do the the prelude, but I suppose that that's not relevant to this RFC...

@alexcrichton
Copy link
Member

cc #503, an RFC for the entire prelude as well!

@steveklabnik
Copy link
Member Author

Closing becuase #503 is better. 🎊

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.

5 participants