cmp: Use default methods in trait Ord, only require Ord::lt #7765
Conversation
| * (cf. IEEE 754-2008 section 5.11). | ||
| */ | ||
| #[allow(default_methods)] |
There was a problem hiding this comment.
You shouldn't need this any more, a very recently landed commit now allows these by default.
There was a problem hiding this comment.
yes, it's needed to get past stage0
There was a problem hiding this comment.
Oh good point... Could you put in a NOTE to remove it?
|
Fwiw, the minimum implementation is often |
|
Oh I didn't realize that |
|
Right huonw, however we can't provide a default for all four methods -- realistically, because we fail so hilariously if the user does I chose le because that's what the comment already in place stated. |
|
Of course don't provide defaults for all of them, but it seems more natural to define |
|
I agree about |
|
I find two cases of using "impl Ord" in a pattern of only implementing one method and then reusing it, and those use |
It will be simpler to implement only one method for Ord, while we also allow implementing all four Ord methods for semantics or performance reasons. We only supply three default methods (and not four), because don't have any nice error reporting for the case where at least one method must be implemented, but it's arbitrary which.
|
Well, it seems that I was wrong, Haskell uses And, fwiw, Haskell actually offers default implementations for all the functions, it is up to the programmer to check that it is coherent. (That said, it should be entirely possible to tell the compiler "at least one of these must be implemented", I opened #7771 about that.) |
|
I still think Ord is broken and ought to be removed or reformed, as it doesn't adequately express partial order, aiui... there needs to be a third value, for "no comparison." |
|
(I'd still r+ though) |
Rust will allow to supply default methods for all four methods, but we don't have any nice error reporting for the case where at least one method must be implemented, but it's arbitrary which. So in this case, we require `lt`, but allow implementing the others if needed.
try reading rust-version from Cargo.toml Cargo.toml can contain a field `rust-version`, that acts like a MSRV of clippy.toml file: https://doc.rust-lang.org/cargo/reference/manifest.html#the-rust-version-field This will try to read that field and use it, if the clippy.toml config has no `msrv` entry changelog: respect `rust-version` from `Cargo.toml` closes rust-lang#8746 closes rust-lang#7765
Rust will allow to supply default methods for all four methods, but we
don't have any nice error reporting for the case where at least one
method must be implemented, but it's arbitrary which.
So in this case, we require
lt, but allow implementing the others if needed.