derive more traits for core::ops::ControlFlow#96416
derive more traits for core::ops::ControlFlow#96416canndrew wants to merge 1 commit intorust-lang:masterfrom
Conversation
Implement Hash, Eq, PartialOrd, Ord for ControlFlow.
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
|
r? @yaahc (rust-highfive has picked a reviewer for you, use r? to override) |
|
r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs |
| /// [`Continue`]: ControlFlow::Continue | ||
| #[stable(feature = "control_flow_enum_type", since = "1.55.0")] | ||
| #[derive(Debug, Clone, Copy, PartialEq)] | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] |
There was a problem hiding this comment.
Ord/PartialOrd were intentionally left out as described in https://rust-lang.github.io/rfcs/3058-try-trait-v2.html#traits-for-controlflow
I'd be inclined to leave it like that, unless you can give a good reason why one variant is less than the other. (Personally I might make it PartialOrd + !Ord, where partial_cmp gives None for mixing the variants, because I think having the order of variants matter is a misfeature, but I understand that's an unpopular position so don't actually propose it here.)
Eq/Hash seem fine to add, though.
|
Going to move this back to waiting-on-author, since the PartialOrd/Ord probably should be dropped. It would be good to add a comment to this type with the explanation @scottmcm provided, so we don't accidentally do this in the future once that context has been paged out. |
|
r? rust-lang/libs I've been taking a break from the review rotation for a while and didn't realize this one was still assigned to me, sorry about that. |
I wanted to put a
ControlFlowin a hash set but couldn't because it lacks the necessary impls. Is there any reason not to implement these?