Add some core::cmp::Ordering helpers #79656
Conversation
|
r? @sfackler (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
If |
|
Bikeshedding: Common word syntax for such matchers is |
|
I agree that |
jnqnfe
force-pushed
the
ordering
branch
3 times, most recently
from
d6fcc7c
to
b03d04b
Compare
|
Ok, I've revised things, including addressing the feedback.
I force-pushed the changes to keep things tidy, since it's such a trivial one to review. |
|
@jnqnfe Looks like this is still failing CI? |
|
You have a bunch of these: It'll probably have to use |
scottmcm
added
S-waiting-on-author
Ah, thanks, fixed! (Sorry about that, it's been a while since I last contributed and I haven't been able to get things setup for compiling this time, partly because I'm using the stable compiler version from my repo :/ ) I've force-pushed that fix, fingers crossed CI will work this time :) Oh, result came in before I could post, it still can't get the log... |
|
@scottmcm Ohhhhh, I've just noticed that a github domain was being blocked by noscript in my browser and as a consequence the errors you saw were being hidden from me. I was under the impression that there was some issue with CI itself because all I saw was the 'could not download log' error line. Sorry about that, I would have fixed the errors days ago if I'd realised. I've just fixed the typo in the last push, so should be good now. Okay, so now it's complaining about missing stability attributes... |
|
@scottmcm Looking into what stability attribute to assign, I'm not at all certain.
|
|
See https://rustc-dev-guide.rust-lang.org/stability.html#unstable Yes, you need a feature name and a tracking issue. Just invent a reasonable name; you don't need to put it anywhere other than in the attributes. Whenever possible -- as it is here -- things go in unstable. That allows them to go in with just a PR sign-off, whereas stabilization will require a full libs team FCP. |
jnqnfe
force-pushed
the
ordering
branch
3 times, most recently
from
b4f9169
to
1016655
Compare
|
@scottmcm Ok. I've:
Outstanding:
I merged the changes into a single commit. I'll split things up if preferred. Thanks for your patience. |
|
Sorry, I guess I wasn't clear here. The "you don't need to put it anywhere other than in the attributes" I said above is because you don't need the compiler or unstable_book changes or the feature gate UI test. See https://github.com/rust-lang/rust/pull/79673/files for an example of adding unstable methods. |
|
Oh ok, thanks, updated. CI has passed now finally :D |
...to allow easier greater-than-or-equal-to and less-than-or-equal-to
comparisons, and variant checking without needing to import the enum,
similar to `Option::is_none()` / `Option::is_some()`, in situations where
you are dealing with an `Ordering` value. (Simple `PartialOrd` / `Ord`
based evaluation may not be suitable for all situations).
Prior to Rust 1.42 a greater-than-or-equal-to comparison might be written
either as a match block, or a traditional conditional check like this:
```rust
if cmp == Ordering::Equal || cmp == Ordering::Greater {
// Do something
}
```
Which requires two instances of `cmp`. Don't forget that while `cmp` here
is very short, it could be something much longer in real use cases.
From Rust 1.42 a nicer alternative is possible:
```rust
if matches!(cmp, Ordering::Equal | Ordering::Greater) {
// Do something
}
```
The commit adds another alternative which may be even better in some cases:
```rust
if cmp.is_ge() {
// Do something
}
```
The earlier examples could be cleaner than they are if the variants of
`Ordering` are imported such that `Equal`, `Greater` and `Less` can be
referred to directly, but not everyone will want to do that.
The new solution can shorten lines, help avoid logic mistakes, and avoids
having to import `Ordering` / `Ordering::*`.
|
@bors r+ |
|
📌 Commit 169c59f has been approved by |
bors
added
S-waiting-on-bors
Add some core::cmp::Ordering helpers
...to allow easier equal-to-or-greater-than and less-than-or-equal-to
comparisons.
Prior to Rust 1.42 a greater-than-or-equal-to comparison might be written
either as a match block, or a traditional conditional check like this:
```rust
if cmp == Ordering::Equal || cmp == Ordering::Greater {
// Do something
}
```
Which requires two instances of `cmp`. Don't forget that while `cmp` here
is very short, it could be something much longer in real use cases.
From Rust 1.42 a nicer alternative is possible:
```rust
if matches!(cmp, Ordering::Equal | Ordering::Greater) {
// Do something
}
```
The commit adds another alternative which may be even better in some cases:
```rust
if cmp.is_equal_or_greater() {
// Do something
}
```
The earlier examples could be cleaner than they are if the variants of
`Ordering` are imported such that `Equal`, `Greater` and `Less` can be
referred to directly, but not everyone will want to do that.
The new solution can shorten lines, help avoid logic mistakes, and avoids
having to import `Ordering` / `Ordering::*`.
|
☀️ Test successful - checks-actions |
...to allow easier equal-to-or-greater-than and less-than-or-equal-to
comparisons.
Prior to Rust 1.42 a greater-than-or-equal-to comparison might be written
either as a match block, or a traditional conditional check like this:
Which requires two instances of
cmp. Don't forget that whilecmphereis very short, it could be something much longer in real use cases.
From Rust 1.42 a nicer alternative is possible:
The commit adds another alternative which may be even better in some cases:
The earlier examples could be cleaner than they are if the variants of
Orderingare imported such thatEqual,GreaterandLesscan bereferred to directly, but not everyone will want to do that.
The new solution can shorten lines, help avoid logic mistakes, and avoids
having to import
Ordering/Ordering::*.