Implement RFC#28: Add PartialOrd::partial_cmp #15030
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Test compilation failures: |
|
JSON objects are unordered key-value maps, and I don't really like that we've extended it with ordering semantics based on key comparisons. I've only ever seen JSON objects mapped to hash tables, not ordered maps (except in C++98, where there was no standard hash table). I wouldn't worry much about trying to get that ordering right, because I don't think it's a good idea to be doing it in the first place. |
|
r=me when coretest is passing, thanks! |
|
It seems like the right way forward here is to extract libcore's tests to a separate crate. I'm working on that now, and this can go in on top of that. |
| @@ -154,20 +160,57 @@ pub fn lexical_ordering(o1: Ordering, o2: Ordering) -> Ordering { | |||
| /// 5.11). | |||
| #[lang="ord"] | |||
| pub trait PartialOrd: PartialEq { | |||
| /// This method returns an ordering between `self` and `other` values | |||
| /// if one exists. | |||
| #[cfg(stage0)] | |||
There was a problem hiding this comment.
The snapshot's deriving won't generate partial_cmp so there needs to be a default impl.
I ended up altering the semantics of Json's PartialOrd implementation. It used to be the case that Null < Null, but I can't think of any reason for an ordering other than the default one so I just switched it over to using the derived implementation. This also fixes broken `PartialOrd` implementations for `Vec` and `TreeMap`. RFC: 0028-partial-cmp
|
@huonw updated |
bors
added a commit
that referenced
this pull request
Jun 30, 2014
I ended up altering the semantics of Json's PartialOrd implementation. It used to be the case that Null < Null, but I can't think of any reason for an ordering other than the default one so I just switched it over to using the derived implementation. This also fixes broken `PartialOrd` implementations for `Vec` and `TreeMap`. # Note This isn't ready to merge yet since libcore tests are broken as you end up with 2 versions of `Option`. The rest should be reviewable though. RFC: 0028-partial-cmp
cadencemarseille
added a commit
to cadencemarseille/rust-pcre
that referenced
this pull request
Jul 1, 2014
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 17, 2023
ci(metrics): Run measurement functions in parallel Resolves rust-lang#14853
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Jun 26, 2025
…5030) Optimize `needless_doctest_main`, make it short-circuit, make sure that we don't spin up a new compiler on EVERY code block. --- The old implementation was creating a new compiler, new parser, new thread, new SessionGlobals, new everything for each code block. No matter if they actually didn't even contain `fn main()` or anything relevant. On callgrind, seems that we're reducing about a 6.7242% de cycle count (which turns out to be a 38 million instruction difference, great!). Benchmarked in `bumpalo-3.16.0`. Also on bumpalo we spawn 78 less threads. This moves `SessionGlobals::new` from the top time-consuming function by itself in some benchmarks, into one not even in the top 500. Also, populate the test files. changelog:[`needless_doctest_main`]: Avoid spawning so many threads in unnecessary circumstances
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I ended up altering the semantics of Json's PartialOrd implementation.
It used to be the case that Null < Null, but I can't think of any reason
for an ordering other than the default one so I just switched it over to
using the derived implementation.
This also fixes broken
PartialOrdimplementations forVecandTreeMap.Note
This isn't ready to merge yet since libcore tests are broken as you end up with 2 versions of
Option. The rest should be reviewable though.RFC: 0028-partial-cmp