Implement clamp_to #150075
Implement clamp_to #150075
Conversation
rustbot
added
S-waiting-on-review
|
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
| fn clamp(self, value: $t) -> $t { | ||
| let (start, end) = self.into_inner(); | ||
| // Deliberately avoid using `clamp` to handle NaN consistently | ||
| value.max(start).min(end) |
There was a problem hiding this comment.
I would be very surprised if x.clamp_to(a..=b) were to behave differently at all from x.clamp(a, b), regardless of the various values for x, a, and b.
Isn't clamp moving toward making its NaN behaviour consistent, anyway?
There was a problem hiding this comment.
Yeah but imo it's worse if x.clamp_to(a..) and x.clamp_to(a..=b) handle nan differently. Making all of them panic on nan would be a plausible choice though. It's listed as an unresolved question on the tracking issue.
There was a problem hiding this comment.
Looking at the current code, it does appear that clamp simply panics on NaN, so, I would say that it's reasonable to always panic on NaN for these methods to match that behaviour. But I guess we can discuss that in the tracking issue instead of blocking this.
5 participants
Implements the revised version of #147781. Supersedes #147786.
Currently I restrict the ClampBounds trait using a second, perma-unstable feature. I don't know if that's the usual way to deal with this kind of traits, I'd be happy to change it if not.
I currently define NaN as equal to no bound. This is consistent with
maxandmin, but is inconsistent withclamp, which panics.