TotalOrder trait for floating point numbers#295
Conversation
Define an orthogonal trait which corresponds to the `totalOrder` predicate the IEEE 754 (2008 revision) floating point standard. In order to maintain coherence, the bounds on `TotalOrder` should most likely be `TotalOrder: Float` (or `TotalOrder: FloatCore`). Without type constraints, `TotalOrder` could be defined on, well, anything. Though slightly ugly, one way to deal with this is to define two traits, `TotalOrderCore: FloatCore` and `TotalOrder: Float`. On the other hand, `Inv` has no such constraints (due to the possibility of a meaningful implementation on rational numbers).
| impl TotalOrder for $T { | ||
| #[inline] | ||
| fn total_cmp(&self, other: &Self) -> std::cmp::Ordering { | ||
| Self::total_cmp(&self, other) |
There was a problem hiding this comment.
We're going to need a fallback implementation for Rust < 1.62, or else this errors as unconditional recursion. I can add that myself if you're not sure how to go about that...
There was a problem hiding this comment.
I don't know what sort of cfg tricks could be used to conditionally provide a fallback based on language version. Or maybe cfg is not the answer? I will learn from you.
There was a problem hiding this comment.
Ok, I've pushed that and a few other fixups -- please review in return!
There was a problem hiding this comment.
Ah, the answer is autocfg... no surprise it has 200 million downloads. Very interesting.
Excellent idea to replace std::cmp::Ordering with core::cmp::Ordering, so that this PR is compatible with no_std by default.
Looks good to me.
Tangentially, thank you for the opportunity to contribute; I learned several useful things.
|
Thanks! bors r+ |
295: `TotalOrder` trait for floating point numbers r=cuviper a=andrewjradcliffe Define an orthogonal trait which corresponds to the `totalOrder` predicate the IEEE 754 (2008 revision) floating point standard. In order to maintain coherence, the bounds on `TotalOrder` should most likely be `TotalOrder: Float` (or `TotalOrder: FloatCore`). Without type constraints, `TotalOrder` could be defined on, well, anything. Though slightly ugly, one way to deal with this is to define two traits, `TotalOrderCore: FloatCore` and `TotalOrder: Float`. On the other hand, `Inv` has no such constraints (due to the possibility of a meaningful implementation on rational numbers). If the crate designers could weigh in on whether to introduce constraints, that would be helpful. Resolves: [issue#256](#256) Co-authored-by: Andrew Radcliffe <andrewjradcliffe@gmail.com> Co-authored-by: Josh Stone <cuviper@gmail.com>
|
Build failed: |
|
bors r+ |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Define an orthogonal trait which corresponds to the
totalOrderpredicate the IEEE 754 (2008 revision) floating point standard.In order to maintain coherence, the bounds on
TotalOrdershould most likely beTotalOrder: Float(orTotalOrder: FloatCore). Without type constraints,TotalOrdercould be defined on, well, anything. Though slightly ugly, one way to deal with this is to define two traits,TotalOrderCore: FloatCoreandTotalOrder: Float. On the other hand,Invhas no such constraints (due to the possibility of a meaningful implementation on rational numbers). If the crate designers could weigh in on whether to introduce constraints, that would be helpful.Resolves: issue#256