Tracking Issue for clamp_min and clamp_max

[Kyuuhachi]

Feature gate: #![feature(clamp_to)], as well as possibly perma-unstable #[feature(clamp_bounds)].

This is a tracking issue for rust-lang/libs-team#665, which adds less confusable alternatives for x.min(y) and x.max(y).

Public API

// Perma-unstable
trait ClampBounds<T> {
    fn clamp(self, x: T) -> T;
}

impl ClampBounds<T> for RangeInclusive<T> where T: Ord {}
impl ClampBounds<f32> for RangeInclusive<f32> {}
// likewise for RangeFrom, RangeToInclusive, and RangeFull, and for f16/f64/f128

trait Ord {
    fn clamp_to(self, range: impl ClampBounds<Self>) -> Self;
}

// For each floating-point type:
impl f32 {
    fn clamp_to(self, range: impl ClampBounds<Self>) -> Self;
}

Steps / History

(Remember to update the S-tracking-* label when checking boxes.)

• ACP: ACP: Intuitive alternative for .min() and .max() libs-team#665
• Implementation 1: Implement clamp_min and clamp_max #147786
• Implementation 2: Implement clamp_to #150075
• Final comment period (FCP)¹
• Stabilization PR

Unresolved Questions

• Is having these functions, where clamp_min == max, actually less confusing than the status quo?
  • It is not. Switched to clamp_to with ranges.
• Should there be a lint that suggests replacing x.max(y) with x.clamp_to(y..)?
• What semantics should the float versions have regarding NaN? NAN.max(0.0) is 0, while NAN.clamp(0.0, 0.0) is NaN, and 0.0.clamp(NAN, NAN) panics.

Footnotes

1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩

[Kyuuhachi]

I'm adding NaN semantics to unresolved questions. Current implementation has same as .max(), but it would also make sense to have the same semantics as .clamp().

[ChrisDenton]

On the original zulip thread, simulacrum suggested naming these at_most and at_least, which would avoid the slightly confusing naming where clamp_max does the opposite to max.

[clarfonthey]

I don't really like the idea of having two methods that do exactly the same thing with different names. I think it's reasonable to deprecate a name in favour of a new one, but not just have two concurrent names at the same time.

I agree that at_most and at_least are more understandable than the current names and might actually be an improvement, but I would much rather have something like clamp_into(range) than a pair of methods, so there's actually the additional benefit of showing the range instead, although you would have to be careful to explicitly disallow exclusive ranges.

[joboet]

I too favour the originally proposed clamp_range, especially as it corresponds nicely with the notion of a "valid value range".

[ChrisDenton]

Using range types was discussed In the libs-api meeting but team members weren't generally favourable. From memory I think the concern was the added complexity especially considering the fact that most people would be looking to clamp only one side.

I don't really like the idea of having two methods that do exactly the same thing with different names.

This was also a concern raised in the meeting. But experimenting with it in nightly was deemed acceptable.

[boomshroom]

Using range types was discussed In the libs-api meeting but team members weren't generally favourable. From memory I think the concern was the added complexity especially considering the fact that most people would be looking to clamp only one side.

Oh, so I guess we should get rid of clamp then, since it clamps from both sides?

Using a range type makes it more obvious that 1-sided clamps from either side and 2-sided clamps are fundamentally the same operation.

[ChrisDenton]

Oh, so I guess we should get rid of clamp then, since it clamps from both sides?

No, the fact that clamp exists and nobody has taken issue with it suggests it doesn't need a range replacement.

[clarfonthey]

I think that having different names for the versions with different calling conventions is fine, but we can't limit the calling conventions of methods at the moment. Perhaps a good path forward if we do choose to pursue this path is to have a clippy lint (I only propose clippy because it has MSRV support, which rustc lints do not), which suggests reformatting x.min/max(y) into x.clamp_max/min(y) and Ord::clamp_min/max(x, y) into Ord::max/min(x, y). I think this shouldn't be machine-applicable because we want to verify that the user was actually using the right version, and so they should have to manually fix it.