Implement clamp_to by Kyuuhachi · Pull Request #150075 · rust-lang/rust

Kyuuhachi · 2025-12-16T22:31:41Z

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 max and min, but is inconsistent with clamp, which panics.~~

Changed so that the float versions panic if any bound is NaN, just like clamp does.

rustbot · 2025-12-16T22:31:48Z

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

clarfonthey · 2025-12-17T03:23:53Z

+        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)


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?

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.

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.

(Side note: after a lot of poking around the changes to clamp and its original RFC, it appears that the panicking behaviour was explicitly decided as the best, and it was not an accident. So, I do think that panicking on NaN bounds like clamp does is the best behaviour.

You're right, if it's named like clamp it should behave like clamp. I'm leaving the question as unresolved, though it's pretty likely that it'll end up this way.

I don't currently have any tests that it does panic on nan; should I add that, and if so where?

I honestly would just look at whatever clamp's tests are and try to match them. Could even use some macros to just run the same tests on both functions to make sure the output matches.

Looks like there's some clamp tests in coretests, but I don't know how that system works. Should I just duplicate all the tests for clamp and adjust them for the different shapes of clamp_to? Or just not do that and leave as-is?

@Kyuuhachi Please do add tests for clamp_to that match the tests for clamp.

clarfonthey · 2025-12-18T23:09:46Z

+            assert!(!self.start.is_nan(), "min was NaN");
+            value.max(self.start)
+        }
+    }
+
+    #[unstable(feature = "clamp_bounds", issue = "147781")]
+    #[rustc_const_unstable(feature = "clamp_bounds", issue = "147781")]
+    impl const ClampBounds<$t> for RangeToInclusive<$t> {
+        fn clamp(self, value: $t) -> $t {
+            assert!(!self.end.is_nan(), "max was NaN");


Minor nit, but since these are ranges, you should probably use the start/end naming instead.

Note that std::range::RangeToInclusive uses the name last to represent an inclusive end instead of end, but since these are just for the regular ops ranges, we should match the actual field name.

These messages are consistent with clamp though. What is more important, consistency with clamp or with ranges?

@Kyuuhachi I'd suggest being consistent with ranges, and later we could potentially modify clamp to be consistent with this.

reddevilmidzy · 2026-03-24T08:25:11Z

@rustbot author

rustbot · 2026-03-24T08:25:17Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Kyuuhachi · 2026-03-25T18:35:45Z

Oops, didn't realize this was still waiting for action from me. Do all the comments need to be resolved or what?

joshtriplett · 2026-05-08T02:46:50Z

@Kyuuhachi I responded in the two threads that remain open. With those two addressed, r=me.

Kyuuhachi · 2026-05-12T21:39:41Z

@bors r=joshtriplett

rust-bors · 2026-05-12T21:39:46Z

@Kyuuhachi: 🔑 Insufficient privileges: not in review users

Kyuuhachi · 2026-05-12T21:42:13Z

@joshtriplett Did I do that wrong?

conradludgate · 2026-05-27T13:42:38Z

Did I do that wrong?

I think you need to:

@rustbot ready

Kyuuhachi · 2026-05-28T07:13:04Z

@rustbot ready

ChrisDenton · 2026-05-28T07:31:31Z

Maybe squish your commits? I'm uncertain if one or two commits would be better it'd be nice to merge some of them at least.

ChrisDenton · 2026-06-26T20:22:59Z

I don't think the reviewer is currently active. I'll add this to my review queue so if nobody reviews this first I'll do it (I think it was more or less approved before, I just need to take a minute to double check).

r? @ChrisDenton

tgross35 · 2026-08-27T07:06:03Z

Diff is just adding the #[expect],

@bors r=ChrisDenton

rust-bors · 2026-08-27T07:06:06Z

📌 Commit bd174e1 has been approved by ChrisDenton

It is now in the queue for this repository.

Implement clamp_to Implements the revised version of rust-lang#147781. Supersedes rust-lang#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 `max` and `min`, but is inconsistent with `clamp`, which panics.~~ Changed so that the float versions panic if any bound is NaN, just like `clamp` does.

Rollup of 7 pull requests Successful merges: - #150075 (Implement clamp_to) - #157145 (Codegen Overloaded LLVM intrinsics based on their name) - #161866 (delegation: add tests fixating behavior of delegating to default trait implementations) - #161456 (reduce perf impact of scalar size checks) - #161730 (Improve type mismatch annotation for lets with block-wrapped initializers) - #161828 (Never type after-stabilization cleanup) - #161860 (atomicptr.rs test: remove unused import)

Implement clamp_to Implements the revised version of rust-lang#147781. Supersedes rust-lang#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 `max` and `min`, but is inconsistent with `clamp`, which panics.~~ Changed so that the float versions panic if any bound is NaN, just like `clamp` does.

Rollup of 14 pull requests Successful merges: - #150075 (Implement clamp_to) - #159103 (fix(reborrow): recursive implementation) - #160848 (std: avoid aliasing violations when wrapping opaque C types) - #161421 (Include startup crt objects on WASI for more outputs) - #161805 (Prefer ambiguous candidates when deduplicating traits in scope, so `ambiguous_glob_imported_traits` doesn't depend on import order) - #161862 (Put data segment in specified section with link_section on wasm) - #161866 (delegation: add tests fixating behavior of delegating to default trait implementations) - #161456 (reduce perf impact of scalar size checks) - #161666 (Print vendor instructions in `x vendor`) - #161730 (Improve type mismatch annotation for lets with block-wrapped initializers) - #161828 (Never type after-stabilization cleanup) - #161860 (atomicptr.rs test: remove unused import) - #161870 (bind to [::1] instead of 127.0.0.1 in documentation examples for v6 UDP methods) - #161876 (rustdoc: Correctly handle when a macro generates multiple items in `--generate-macro-expansion`)

Implement clamp_to Implements the revised version of rust-lang#147781. Supersedes rust-lang#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 `max` and `min`, but is inconsistent with `clamp`, which panics.~~ Changed so that the float versions panic if any bound is NaN, just like `clamp` does.

…uwer Rollup of 21 pull requests Successful merges: - #158609 (Update sccache to 0.16.0) - #150075 (Implement clamp_to) - #159103 (fix(reborrow): recursive implementation) - #160562 (add target feature ABI checks for SPARC) - #160848 (std: avoid aliasing violations when wrapping opaque C types) - #161421 (Include startup crt objects on WASI for more outputs) - #161805 (Prefer ambiguous candidates when deduplicating traits in scope, so `ambiguous_glob_imported_traits` doesn't depend on import order) - #161862 (Put data segment in specified section with link_section on wasm) - #161866 (delegation: add tests fixating behavior of delegating to default trait implementations) - #161456 (reduce perf impact of scalar size checks) - #161528 (Add regression test to ensure optimal compilation) - #161666 (Print vendor instructions in `x vendor`) - #161730 (Improve type mismatch annotation for lets with block-wrapped initializers) - #161828 (Never type after-stabilization cleanup) - #161859 (Do not optimize MIR for comptime ConstFns) - #161860 (atomicptr.rs test: remove unused import) - #161870 (bind to [::1] instead of 127.0.0.1 in documentation examples for v6 UDP methods) - #161876 (rustdoc: Correctly handle when a macro generates multiple items in `--generate-macro-expansion`) - #161889 (Add link to ownership section in ptr::read docs) - #161890 (rustdoc: some clarifying comments) - #161891 (Mark `extern_item_impls` feature as incomplete) Failed merges: - #161702 (Use `drop_guard` in some places in {core,alloc,std})

Implement clamp_to Implements the revised version of rust-lang#147781. Supersedes rust-lang#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 `max` and `min`, but is inconsistent with `clamp`, which panics.~~ Changed so that the float versions panic if any bound is NaN, just like `clamp` does.

rustbot assigned ibraheemdev Dec 16, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 16, 2025

This was referenced Dec 16, 2025

Implement clamp_min and clamp_max #147786

Closed

Tracking Issue for clamp_to #147781

Open

clarfonthey reviewed Dec 17, 2025

View reviewed changes

danielhenrymantilla reviewed Dec 18, 2025

View reviewed changes

Comment thread library/core/src/num/f64.rs Outdated

Comment thread library/core/src/num/f32.rs Outdated

Comment thread library/core/src/num/f16.rs Outdated

Comment thread library/core/src/num/f128.rs Outdated

clarfonthey reviewed Dec 18, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2026

This comment has been minimized.

Sign in to view

Kyuuhachi force-pushed the limit_to branch from 2229986 to 8786b0d Compare May 12, 2026 21:14

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 27, 2026

Kyuuhachi force-pushed the limit_to branch from 8786b0d to f31860e Compare June 26, 2026 17:08

This comment has been minimized.

Sign in to view

rustbot assigned ChrisDenton Jun 26, 2026

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2026

JonathanBrouwer mentioned this pull request Aug 27, 2026

Rollup of 4 pull requests #161854

Closed

matthiaskrgr mentioned this pull request Aug 27, 2026

Rollup of 4 pull requests #161863

Closed

matthiaskrgr mentioned this pull request Aug 27, 2026

Rollup of 5 pull requests #161869

Closed

matthiaskrgr mentioned this pull request Aug 27, 2026

Rollup of 7 pull requests #161871

Closed

matthiaskrgr mentioned this pull request Aug 27, 2026

Rollup of 12 pull requests #161874

Closed

JonathanBrouwer mentioned this pull request Aug 27, 2026

Rollup of 11 pull requests #161875

Closed

JonathanBrouwer mentioned this pull request Aug 27, 2026

Rollup of 11 pull requests #161878

Closed

GuillaumeGomez mentioned this pull request Aug 27, 2026

Rollup of 14 pull requests #161884

Closed

JonathanBrouwer mentioned this pull request Aug 28, 2026

Rollup of 18 pull requests #161901

Closed

JonathanBrouwer mentioned this pull request Aug 28, 2026

Rollup of 21 pull requests #161905

Closed

JonathanBrouwer mentioned this pull request Aug 28, 2026

Rollup of 21 pull requests #161906

Open

Uh oh!

Uh oh!

Conversation

Kyuuhachi commented Dec 16, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clarfonthey Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reddevilmidzy commented Mar 24, 2026

Uh oh!

rustbot commented Mar 24, 2026

Uh oh!

Kyuuhachi commented Mar 25, 2026

Uh oh!

joshtriplett commented May 8, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Kyuuhachi commented May 12, 2026

Uh oh!

rust-bors Bot commented May 12, 2026

Uh oh!

Kyuuhachi commented May 12, 2026

Uh oh!

conradludgate commented May 27, 2026

Uh oh!

Kyuuhachi commented May 28, 2026

Uh oh!

ChrisDenton commented May 28, 2026

Uh oh!

This comment has been minimized.

ChrisDenton commented Jun 26, 2026

Uh oh!

tgross35 commented Aug 27, 2026

Uh oh!

rust-bors Bot commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Kyuuhachi commented Dec 16, 2025 •

edited by rustbot

Loading

clarfonthey Dec 18, 2025 •

edited

Loading