Removed num::Orderable #12061
Removed num::Orderable #12061
Conversation
| // These should be methods on `Ord`, with overridable default implementations. We don't want | ||
| // to encumber all implementors of Ord by requiring them to implement these functions, but at | ||
| // the same time we want to be able to take advantage of the speed of the specific numeric | ||
| // functions (like the `fmin` and `fmax` intrinsics). |
There was a problem hiding this comment.
It looks like we didn't even use fmin/fmax 😦
Can you open an issue for adding min/max/clamp default methods to Ord? Alternatively, just do it in this PR?
|
r+ with above comment addressed. death to num! |
|
Opened min/max/clamp issue. |
|
Where has the fmin and fmax functionality gone? D: Why the r+? |
|
I think it would be best to have some alternative for floats in place before we r+ this. I'm all for killing Orderable, (I wrote that original FIXME at line 37), but it would be good to have something in place before hand. |
|
@bjz: This doesn't seem to be using |
|
@thestinger I'm wondering if we should have an |
|
@thestinger Are you ok with r+-ing this for now? |
|
Yes, I'm okay with removing this trait. |
|
I did not expect people to have strong opinions about this. Since some tests are failing anyway, I'll wait for the dust to settle over the weekend, just to make sure we're doing what we want to be doing. My university is also trying to get the last pint of my blood before I graduate, so this small break might prove useful. |
|
From the lack of response, I assume everyone is ok with r+-ing this? The compile log looks like the compilation was manually interrupted. Is there a failed test I didn't see? |
|
Yeah this should be ok. |
|
r=cmr |
|
Needs a rebase. |
|
Should be fixed now. |
#12057