std::cmp::max vs std::num::Float vs NaN

[kornelski]

It's werid that std::cmp::max can't compare f64 numbers. I know that strictly speaking IEEE floats don't have total order this function expects, but still it's surprising (and partial_max is awkward to use).

And there's std::num::Float::max which works with f64 just fine (the docs don't even say how NaN is handled).

It bugs me that the two versions of max are not consistent in their strictness, and that the first-and-most-obvious max function in the stdlib "doesn't work" with a basic type in the language.

My suggestion:

• Rename the max version that only allows Ord to something else, like total_max or strict_max.
• Implement std::cmp::max for floating point numbers, so that a.max(b) is consistent with max(a,b).

[bombless]

I personally think max can just return a NaN when there's no order.

[kennytm]

This issue should be moved to https://github.com/rust-lang/rfcs?

---

Regarding your second suggestion, how would you implement max so that max(2.0f64, 3.0f64) and max(2u64, 3u64) both compiles? It is just as weird if max only works for floats but not integers.

---

Alternative: we could introduce an additional trait, implemented for Ord and f32, f64:

trait Lattice : PartialOrd {
    fn max(&self, other: &Self) -> Self;
    fn min(&self, other: &Self) -> Self;
}
impl<T: Ord> Lattice for T { ... }
impl Lattice for f32 { ... }
impl Lattice for f64 { ... }
fn max<T: Lattice>(x: T, y: T) -> T { x.max(&y) }
fn min<T: Lattice>(x: T, y: T) -> T { x.min(&y) }

The drawback is that, we need to introduce yet another comparison trait.

---

@bombless Note that IEEE 754 defines max(y, NaN) == min(y, NaN) == y (§5.3.1). That's also the current behavior of Float::min, max.

[bombless]

We have Float::max anyway, so I don't think IEEE 754 could be a real problem.

[steveklabnik]

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

As these functions are stable, changing them would require going through the RFC process.

This issue has been moved to the RFCs repo: rust-lang/rfcs#852