Add min/max/clamp to Ord

[pongad]

#12061 removed Orderable, so now we need to add min/max/clamp methods back to Ord. Adding them right now creates a lot of ambiguous method error. This is probably best left until UFCS lands.

[thestinger]

There are already min and max functions in std::cmp. I don't think these should be methods because reserving the min and max method names for every type implementing Ord to too broad.

[emberian]

The benefit being that floats could use fmin/fmax.

[emberian]

...but we could probably just expose those directly

[thestinger]

I don't think floating point numbers should implement Ord. They can't satisfy any sane definition of Ord with the hardware comparison operations.

[pongad]

@thestinger Could you elaborate? I thought floating point comparison is quite well defined.

[thestinger]

The hardware comparison operations do not provide a total ordering. IEEE754 does provide a separate total ordering but hardware doesn't implement it and Rust doesn't use it for the comparison operators.

If you can't say that x == x is always true or that !(a < b) && !(b < a) implies a == b then what do the Eq and Ord traits actually mean? They're not usable by sort, HashMap or TreeMap...

[pongad]

I see your point now. There are obviously problems when you do floating points, though I'm not sure if it's wise to not implement the traits entirely. Users with experience with floating points ought to know of these drawbacks. My current (not too informed) opinion is that implementing Eq and Ord anyway is more pragmatic.

[thestinger]

Rust has TotalEq and TotalOrd to work around this issue... so implementing it anyway just means no one can use Eq and Ord (TreeMap and sort need TotalOrd). I think the only sane choices are using the IEEE754 total ordering for floats or not implementing the comparison traits and operators for them. SML does the right thing here.

[brendanzab]

I think the only sane choices are using the IEEE754 total ordering for floats or not implementing the comparison traits and operators for them. SML does the right thing here.

Agreed. Should we have a separate fmin and fmax for floats?

[thestinger]

Yes, I think the floating point modules should define min and max functions or they can be defined on a Float trait.

[thestinger]

I think we have enough issues open about this already. There's a strong consensus that the current situation is bad but no consensus on how to deal with floating point. However, it's clear that it's unable to provide the requirements expected of Eq and Ord while still using the usual comparison semantics so there's not a strong reason to change min, max and clamp to methods. The min and max method names should really not be reserved by such a common trait.