const and NonZero impl for clamp_magnitude() by Eleocraft · Pull Request #163279 · rust-lang/rust

Eleocraft · 2026-09-24T17:14:35Z

This PR is implementing the latest proposed changes in Issue #148519, making the clamp_magnitude() functions const for all floating point and integer types, as well as adding a clamp_magnitude() to NonZero<$Int>, taking a NonZero<$UInt> as limit parameter.

…nZero<T>

rustbot · 2026-09-24T17:14:40Z

Thanks for the pull request, and welcome! The Rust Project has assigned @Darksonn (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 12 candidates
Random selection from 6 candidates

Copilot

Copilot review overview

🟡 Changes recommended

Address the missing const-stability gates, documentation issue, and required compile-time/runtime test coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 · 2

Open (5)

Add explicit const-stability gate · New
Add explicit const-stability gate · New
Test const behavior and NonZero overloads · New
Capitalize sentence starts in documentation · New
Fix invalid NonZero clamp equivalence · New

What changed in this PR

This PR makes clamp_magnitude const across numeric types and adds signed NonZero support.

Changes:

Makes integer and floating-point implementations const.
Adds NonZero<signed>::clamp_magnitude.
Updates floating-point panic-message tests.

File	Summary
`library/coretests/tests/num/clamp_magnitude.rs`	Updates panic expectations; add compile-time coverage for const and `NonZero` APIs.
`library/core/src/num/nonzero.rs`	Adds signed `NonZero` support and documentation; fix the equivalence example and add coverage.
`library/core/src/num/int_macros.rs`	Makes signed integer methods const; add compile-time assertions.
`library/core/src/num/f64.rs`	Makes the method const; add an explicit const-stability gate.
`library/core/src/num/f32.rs`	Makes the method const; add an explicit const-stability gate.
`library/core/src/num/f16.rs`	Makes the method const.
`library/core/src/num/f128.rs`	Makes the method const.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

View changes since this review

Adding tests addressing the changes in coretests/tests/num/clamp_magnitude.rs

Eleocraft · 2026-09-24T23:43:02Z

Currently the clamp_magnitude.rs file in coretests is not included in mod.rs, therefore the (additional and existing) tests are not run with the rest of the suite. Since this has to be deliberate: what is the reason?

Darksonn · 2026-09-25T07:07:14Z

+        #[must_use = "method returns a new number and does not mutate the original value"]
+        #[rustc_const_unstable(feature = "clamp_magnitude", issue = "148519")]
+        #[unstable(feature = "clamp_magnitude", issue = "148519")]
+        pub const fn clamp_magnitude(self, limit: $UnsignedT) -> Self {
            if let Ok(limit) = core::convert::TryInto::<$SelfT>::try_into(limit) {


You're making this const. Note that this implementation depends on traits, so stabilization as const may be an issue.

View changes since the review

You are right. const impl Ord is gated behind #[rustc_const_unstable(feature = "const_cmp", issue = "143800")], so const fn clamp_magnitude should be too. I will address this.

Gating it behind const_cmp seems wrong/weird to me ... most likely it should just be implemented without using the const trait, or it should wait for said const trait to stabilize.

Clamp is part of the Ord trait. Generally const functions that depend on functions in the ord trait seem to have rustc_const_unstable gated behind "const_cmp". Look for example to the total_cmp() functions in the float types that are stable, but gated rustc_const_unstable behind "const_cmp" since they depend on cmp(). The only reason why clamp_magnitude() is not itself part of Ord is that it requires Neg (and therefore is only applicable to signed types). I also don't see how I would implement clamp_magnitude() without Ord, that seems like it would result in a lot of redundant code. If that does not convince you I am fine removing the const in int_macros.rs and nonzero.rs for now, however that partially defeats the purpose of this PR.

rustbot · 2026-09-25T07:07:25Z

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

Darksonn

Some of these missing tests are pre-existing problems, which you don't necessarily have to implement if you don't want to, but I think this could use some more test cases.

View changes since this review

Darksonn · 2026-09-25T07:30:07Z

You removed this for NonZero, but I think it could make sense to translate it to cases using 1 as the limit.

Not sure if that would really make sense. The special case in zero for signed integers is that a value clamped to the magnitude of zero should always be zero, regardless of whether that value was positive or negative. For NonZero values that would not be the case, therefore I fail to see how cases using 1 as limit provide any additional coverage compared to the tests using 50 as limit.

teor2345

Some hints for changes

View changes since this review

Adding mod clamp_magnitude in coretests/num/mod.rs Activating feature(clamp_magnitude) in coretests/lib.rs Adding test coverage for f16, f128, and NaN as well as zero cases Modifying nonzero and int_macros documentation for better readablity

Eleocraft · 2026-09-28T09:12:47Z

@rustbot ready

Eleocraft · 2026-09-28T12:04:26Z

@rustbot ready

rust-bors · 2026-09-29T21:03:54Z

☀️ Try build successful (CI)
Build commit: ea7be8b (ea7be8b9acc14424599ff684b6da80bdef3b4dfc)
Base parent: ea6bb45 (ea6bb45b74c24a026dab4187b1550e97b6985c8a)

Eleocraft · 2026-09-29T21:10:20Z

The failure was (presumably) due to MinGW not having real f16 and f128 support. Gating the relevant tests behind target_has_reliable should fix the issues.

@rustbot ready

maxdexh · 2026-09-29T21:11:12Z

You could have just linked to zulip, so that your reviewer doesn't just have to take your word for it ^^

Darksonn · 2026-09-30T05:44:28Z

@bors try jobs=dist-various-1,test-various,test-x86_64-gnu-aux,test-x86_64-gnu-llvm-21-3,test-x86_64-msvc-1,test-aarch64-apple-1,test-aarch64-apple-2,test-x86_64-mingw-1,test-i686-msvc,test-armhf-gnu

…try> const and NonZero impl for clamp_magnitude() try-job: dist-various-1 try-job: test-various try-job: test-x86_64-gnu-aux try-job: test-x86_64-gnu-llvm-21-3 try-job: test-x86_64-msvc-1 try-job: test-aarch64-apple-1 try-job: test-aarch64-apple-2 try-job: test-x86_64-mingw-1 try-job: test-i686-msvc try-job: test-armhf-gnu

rust-bors · 2026-09-30T08:49:18Z

☀️ Try build successful (CI)
Build commit: 3fe1d17 (3fe1d17c5be6e38bc62e204cdf0e1689dc4ceba6)
Base parent: 5c543b0 (5c543b0b8c73c7b72bc8284ced4fb22ead15734d)

Darksonn · 2026-09-30T09:22:53Z

@bors r+ rollup=iffy

rust-bors · 2026-09-30T09:22:56Z

📌 Commit 36358d0 has been approved by Darksonn

It is now in the queue for this repository.

…-limit, r=Darksonn const and NonZero impl for clamp_magnitude() This PR is implementing the latest proposed changes in Issue rust-lang#148519, making the clamp_magnitude() functions const for all floating point and integer types, as well as adding a clamp_magnitude() to NonZero<$Int>, taking a NonZero<$UInt> as limit parameter.

…uwer Rollup of 18 pull requests Successful merges: - #163532 (rustc_codegen_cranelift subtree update) - #163534 (miri subtree update) - #163279 (const and NonZero impl for clamp_magnitude()) - #162900 (Some refactorings around metadata encoding) - #163455 (Don't use the metadata based crate_hash for rustdoc runs) - #163483 (Bump bootstrap compiler to 1.100.0 beta) - #159798 (Attribute documentation for cfg_attr) - #163368 (Don't build format string suggestions from `concat!` offsets) - #163375 (Update expect messages in library/std/src/os/unix/net/ following Rust's `expect` guidance) - #163405 (Remove some #[linkage] options) - #163470 (Miscellaneous attr error stuff) - #163489 (expose Rc::is_unique) - #163492 (x86 and x86_64: cleanup some callconv code) - #163496 (cycle handling: mirror old solver) - #163509 (Use more default field values in `Resolver`) - #163519 (Forbid `Reborrow` impls for types with destructors) - #163520 (Cast cleanups) - #163524 (Document `Result` case for the `arena_cache` query modifier) Failed merges: - #163547 ([beta] rustfmt backport)

…-limit, r=Darksonn const and NonZero impl for clamp_magnitude() This PR is implementing the latest proposed changes in Issue rust-lang#148519, making the clamp_magnitude() functions const for all floating point and integer types, as well as adding a clamp_magnitude() to NonZero<$Int>, taking a NonZero<$UInt> as limit parameter.

…uwer Rollup of 18 pull requests Successful merges: - #163532 (rustc_codegen_cranelift subtree update) - #163534 (miri subtree update) - #163279 (const and NonZero impl for clamp_magnitude()) - #162900 (Some refactorings around metadata encoding) - #163455 (Don't use the metadata based crate_hash for rustdoc runs) - #159798 (Attribute documentation for cfg_attr) - #162921 (make mips64 `Complex` GCC-compatible) - #163368 (Don't build format string suggestions from `concat!` offsets) - #163375 (Update expect messages in library/std/src/os/unix/net/ following Rust's `expect` guidance) - #163405 (Remove some #[linkage] options) - #163470 (Miscellaneous attr error stuff) - #163489 (expose Rc::is_unique) - #163492 (x86 and x86_64: cleanup some callconv code) - #163496 (cycle handling: mirror old solver) - #163509 (Use more default field values in `Resolver`) - #163519 (Forbid `Reborrow` impls for types with destructors) - #163520 (Cast cleanups) - #163524 (Document `Result` case for the `arena_cache` query modifier)

Rollup of 20 pull requests Successful merges: - #163279 (const and NonZero impl for clamp_magnitude()) - #163081 (Provide more context on "not general enough" error) - #163455 (Don't use the metadata based crate_hash for rustdoc runs) - #159798 (Attribute documentation for cfg_attr) - #162921 (make mips64 `Complex` GCC-compatible) - #163368 (Don't build format string suggestions from `concat!` offsets) - #163375 (Update expect messages in library/std/src/os/unix/net/ following Rust's `expect` guidance) - #163470 (Miscellaneous attr error stuff) - #163489 (expose Rc::is_unique) - #163492 (x86 and x86_64: cleanup some callconv code) - #163496 (cycle handling: mirror old solver) - #163509 (Use more default field values in `Resolver`) - #163519 (Forbid `Reborrow` impls for types with destructors) - #163520 (Cast cleanups) - #163524 (Document `Result` case for the `arena_cache` query modifier) - #163546 (PassWrapper: adapt for new PassPlugin load method) - #163551 (Add libs-nominated triagebot config) - #163559 (Suggest `#[unsafe(no_mangle)]` for entry points in `no_std` binaries) - #163564 (rustc-dev-guide subtree update) - #163568 (remove dead cfg_select! arm)

Rollup merge of #163279 - Eleocraft:clamp-magnitude-unsigned-limit, r=Darksonn const and NonZero impl for clamp_magnitude() This PR is implementing the latest proposed changes in Issue #148519, making the clamp_magnitude() functions const for all floating point and integer types, as well as adding a clamp_magnitude() to NonZero<$Int>, taking a NonZero<$UInt> as limit parameter.

Rollup of 20 pull requests Successful merges: - rust-lang/rust#163279 (const and NonZero impl for clamp_magnitude()) - rust-lang/rust#163081 (Provide more context on "not general enough" error) - rust-lang/rust#163455 (Don't use the metadata based crate_hash for rustdoc runs) - rust-lang/rust#159798 (Attribute documentation for cfg_attr) - rust-lang/rust#162921 (make mips64 `Complex` GCC-compatible) - rust-lang/rust#163368 (Don't build format string suggestions from `concat!` offsets) - rust-lang/rust#163375 (Update expect messages in library/std/src/os/unix/net/ following Rust's `expect` guidance) - rust-lang/rust#163470 (Miscellaneous attr error stuff) - rust-lang/rust#163489 (expose Rc::is_unique) - rust-lang/rust#163492 (x86 and x86_64: cleanup some callconv code) - rust-lang/rust#163496 (cycle handling: mirror old solver) - rust-lang/rust#163509 (Use more default field values in `Resolver`) - rust-lang/rust#163519 (Forbid `Reborrow` impls for types with destructors) - rust-lang/rust#163520 (Cast cleanups) - rust-lang/rust#163524 (Document `Result` case for the `arena_cache` query modifier) - rust-lang/rust#163546 (PassWrapper: adapt for new PassPlugin load method) - rust-lang/rust#163551 (Add libs-nominated triagebot config) - rust-lang/rust#163559 (Suggest `#[unsafe(no_mangle)]` for entry points in `no_std` binaries) - rust-lang/rust#163564 (rustc-dev-guide subtree update) - rust-lang/rust#163568 (remove dead cfg_select! arm)

making clamp_magnitude functions const + adding clamp_magnitude to No…

dc39f01

…nZero<T>

Copilot AI lite review requested due to automatic review settings September 24, 2026 17:14

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 Sep 24, 2026

rustbot assigned Darksonn Sep 24, 2026

Copilot started reviewing on behalf of Eleocraft September 24, 2026 17:15 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

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

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

Comment thread library/core/src/num/nonzero.rs

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

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

Eleocraft changed the title ~~making clamp_magnitude functions const + adding clamp_magnitude to No…~~ const and NonZero impl for clamp_magnitude() Sep 24, 2026

adding coretests and fixing docs

d6d250c

Adding tests addressing the changes in coretests/tests/num/clamp_magnitude.rs

Darksonn requested changes Sep 25, 2026

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 Sep 25, 2026

Darksonn reviewed Sep 25, 2026

View reviewed changes

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

Darksonn reviewed Sep 25, 2026

View reviewed changes

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

teor2345 reviewed Sep 25, 2026

View reviewed changes

Applying requested changes

3f2e9ee

Adding mod clamp_magnitude in coretests/num/mod.rs Activating feature(clamp_magnitude) in coretests/lib.rs Adding test coverage for f16, f128, and NaN as well as zero cases Modifying nonzero and int_macros documentation for better readablity

Eleocraft requested a review from Darksonn September 25, 2026 13:30

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 Sep 25, 2026

Darksonn requested changes Sep 28, 2026

View reviewed changes

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

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 Sep 28, 2026

nonzero doc: replacing tabs with spaces

b9e7e71

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 28, 2026

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 Sep 29, 2026

This comment has been minimized.

Sign in to view

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 Sep 30, 2026

JonathanBrouwer mentioned this pull request Sep 30, 2026

Rollup of 14 pull requests #163540

Closed

JonathanBrouwer mentioned this pull request Sep 30, 2026

Rollup of 13 pull requests #163542

Closed

JonathanBrouwer mentioned this pull request Sep 30, 2026

Rollup of 18 pull requests #163560

Closed

JonathanBrouwer mentioned this pull request Sep 30, 2026

Rollup of 18 pull requests #163566

Closed

jhpratt mentioned this pull request Oct 1, 2026

Rollup of 20 pull requests #163583

Merged

rust-bors Bot merged commit a2e82d0 into rust-lang:main Oct 1, 2026
14 checks passed

rustbot added this to the 1.101.0 milestone Oct 1, 2026

Uh oh!

Conversation

Eleocraft commented Sep 24, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 24, 2026

Uh oh!

Copilot AI left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Eleocraft commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Darksonn Sep 25, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eleocraft Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Darksonn Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

Eleocraft Sep 28, 2026 • 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!

rustbot commented Sep 25, 2026

Uh oh!

Uh oh!

Darksonn left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Darksonn Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Eleocraft Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

teor2345 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eleocraft commented Sep 28, 2026

Uh oh!

Uh oh!

Eleocraft commented Sep 28, 2026

Uh oh!

rust-bors Bot commented Sep 29, 2026

Uh oh!

Eleocraft commented Sep 29, 2026

Uh oh!

maxdexh commented Sep 29, 2026

Uh oh!

Darksonn commented Sep 30, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Sep 30, 2026

Uh oh!

Darksonn commented Sep 30, 2026

Uh oh!

rust-bors Bot commented Sep 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Eleocraft commented Sep 24, 2026 •

edited by rustbot

Loading

Copilot AI left a comment •

edited by rustbot

Loading

Eleocraft commented Sep 24, 2026 •

edited

Loading

Darksonn Sep 25, 2026 •

edited by rustbot

Loading

Eleocraft Sep 28, 2026 •

edited

Loading

Darksonn left a comment •

edited by rustbot

Loading

teor2345 left a comment •

edited by rustbot

Loading