const-num-traits: const-trait port + modern op coverage + typestates#7
const-num-traits: const-trait port + modern op coverage + typestates#7kaidokert wants to merge 10 commits into
Conversation
Squashed working state on top of the merged fork base, for PR review. Ports the num-traits surface to const traits via `c0nst` (byte-identical on stable, `const trait`/`impl const` on nightly), and modernizes it: - by-value receivers + associated `Output` (implementable by both Copy and non-Copy types) - capability splits (`PrimBits`, `Signum`, `RingOps`, `FromStrRadix`) + prelude - complete nightly-std integer/float op coverage (checked/wrapping/saturating/ overflowing/strict variants, rounding, ilog/isqrt/pow2, bits, mixed-sign, conversions, float_ops, from_ascii, format_into) - `ops/carrying.rs` (std `bigint_helper_methods`: carrying/borrowing/widening) split from `ops/clmul.rs` (carry-less GF(2) multiplication) - opt-in `typestate` feature (`PowerOfTwo`) + pure-core `personality` marker (`Ct`/`Nct`) - `ct` feature (subtle-backed masked ops); `libm`/`nightly`/`nightly-std` - MSRV 1.86 Docs: DESIGN.md, MIGRATION.md, API_BREAKS.md, COVERAGE.md, CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ntax) rust PR #158009 (2026-06-17, oli-obk) replaced `impl const Trait` with `const impl Trait`. The c0nst 0.2.1 macro still emits the old form, so any nightly >= 2026-06-19 fails the `nightly`/`nightly-std` features with "expected a trait, found type". Pin to the last good nightly (2026-06-18, rustc c1b22f44c) as a temporary stopgap. NIGHTLY_PIN.md documents the symptom, root cause, bisect, why c0nst is the unaccounted-for case, and the two unpin paths (wait for c0nst, or migrate our source `impl c0nst` -> `c0nst impl`). Known follow-up (NOT addressed): this rust-toolchain.toml overrides CI toolchains too, so the stable/MSRV jobs would run on the pinned nightly. See the "CI note" in NIGHTLY_PIN.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ghtly pin rust PR #158009 (in nightly >= 2026-06-19) replaced `impl const Trait` with `const impl Trait`. Because c0nst 0.2.1 does positional `c0nst`->`const` substitution, moving the keyword before `impl` in our source makes it emit the new `const impl` on nightly and plain `impl` on stable — the durable fix for the breakage the 2026-06-18 pin was working around. - 160 `impl c0nst Trait` sites -> `c0nst impl Trait`, including the macro metavariable forms (`impl c0nst $trait_name`/`$name`). `pub c0nst trait` declarations and `[c0nst]` bounds are unchanged (#158009 only moved the keyword on impls). - Remove rust-toolchain.toml; the source now requires nightly >= 2026-06-19. - NIGHTLY_PIN.md marked resolved; HOW_TO_FIX.md records the procedure. Verified: stable (114 lib tests) and the previously-broken nightly f428d123a (2026-06-19) both build `--features nightly` green; const_nightly canary 7/7. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…f_pow2, ct ctors; Parity-for-&T Completes the resident typestate families per the final-shape synthesis — all behind the `typestate` feature (default-feature surface unchanged): - D1: blanket `impl<T: Parity + Copy> Parity for &T`, so `from_ref`-style borrowed constructors borrow-check for primitives (parity.rs). - PowerOfTwoOps gains `next_multiple_of_pow2` (branch-free align-up) and `checked_next_multiple_of_pow2`. - NonZero bridge: `HasNonZero` (assoc `type NonZero = core::num::NonZero<Self>`; no `Into<Self>` bound — non-const, would poison const consumers — value via a const `nonzero_get` accessor) + `DivNonZero` (`div_nonzero`/`rem_nonzero`, fresh `Output`, unsigned only; doc notes primitive codegen win ~0). - Sign typestates `NonNegative`/`Positive` (signed-only): infallible unsigned cast, branch-free abs, total isqrt, + const-fn refinement narrowings (`Positive -> NonNegative`, `Positive -> NonZero`). - `Odd<T>` (+ sibling `Even<T>`): bare proofs (no consuming op here; Odd's consumer is modmath). Even was cut by the synthesis; re-added bare to round out the ct-constructor set — drop if the cut should stand. - `ct` feature: 6 `CtOption`-returning masked constructors — `new_ct` on PowerOfTwo / NonNegative / Positive / Odd / Even + `CtNonZero::into_nonzero_ct` — over the existing CtIsPowerOfTwo / CtParity / CtIsZero primitives. Verified: default 114, typestate 122, typestate+ct 126, nightly+typestate 122 (const traits), typestate doctest green, zero warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
typestate is pure-core and zero-cost when unused, so — unlike `ct`, which gates the real `subtle` dependency — there's no reason to feature-gate it. Gating only kept the names out of the default surface, not worth the opt-in friction for a crate that's already a heavy num-traits fork. Make it always available. - drop `#[cfg(feature = "typestate")]` from ops/mod.rs, the root re-exports, and the const_nightly canary; the `CtNonZero` re-export now gates on `ct` only. - the prelude exports the typestate *traits* (PowerOfTwoOps / HasNonZero / DivNonZero, + ct `CtNonZero`) for method resolution but NOT the wrapper *types* — keeps generic names (Odd/Even/Positive/NonNegative) out of `prelude::*`. - remove the now-pointless vestigial `typestate = []` and `i128 = []` no-op features (no legacy users to preserve; nothing gates on either). Verified: default 122 lib tests, ct 126, nightly green, zero warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the four families previously deferred from the typestate layer. - `BitIndex<T>` (all 12 ints, u32 rep like `PowerOfTwo`) + `BitIndexOps` (c0nst): `shl_index`/`shr_index` by an amount proven `< BITS`, so the shift carries no overflow-check branch and needs no `unbounded_*` masking. No `new_ct` — shift amounts are public parameters per the crate CT convention. - `NonMin<T>` (signed, repr(transparent)): proof that a value is `!= MIN`. Spends it on total `neg`/`abs` (the sole overflow is at `MIN`) and, as the dividend, total signed `div_nonzero`/`rem_nonzero` by a `core::num::NonZero` divisor — the only signed-division overflow is `MIN / -1`, excluded by construction. This is the co-proof unsigned `DivNonZero` can do without. `Positive`/`NonNegative` narrow here for free via `into_nonmin`; adds a `new_ct` masked constructor. - `Finite<T>` (f32/f64, repr(transparent)): const `new` via an exponent-bit test (sidesteps the non-const `is_finite`). Spent by a total `Ord`/`Eq` — with NaN excluded `partial_cmp().unwrap()` can't panic — so finite floats become sortable / `BTreeMap`-keyable, which bare `f32`/`f64` aren't. No `new_ct`: floats are outside the constant-time model. Updates the `DivNonZero` doc (the NonMin co-proof is no longer "deferred"), documents the whole typestate module in CLAUDE.md, and adds unit + doctest + nightly-const-canary coverage. Verified: stable 126 lib / 20 integration / 257 doctests, ct 130, nightly 126 + 9 const-canary, no_std builds, zero warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`Odd::new` / `Even::new` gain a `where T: [const] Parity` refinement, so on nightly they are `const`-callable for any backend whose `Parity` impl is const (plain on stable, signature unchanged). This is the panic-free-north-star path from modmath's roundup: a compile-time-constant modulus can be proven odd in a `const` block, so a downstream `Field::new(p).unwrap()` becomes a compile error instead of a surviving `panic_fmt` symbol. The `[const]` bound rides on the fn's where-clause (an inherent impl header can't carry it); `Copy` covers the drop, so no `Destruct` bound is needed. Also corrects a stale CLAUDE.md line: the `Overflowing*` family IS root-exported (has been since the squash), not module-path-only — that re-export is what resolves modmath's `use const_num_traits::OverflowingAdd` on nightly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Untracks the design/process notes that shouldn't live in the fork's history (kept on disk locally): NIGHTLY_PIN.md, HOW_TO_FIX.md, CLAUDE.md, DESIGN.md, COVERAGE.md, API_BREAKS.md. MIGRATION.md (downstream-facing) and README.md stay. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @kaidokert, your pull request is larger than the review limit of 300000 diff characters
|
Warning Review limit reached
More reviews will be available in 23 minutes and 30 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR significantly expands Changesconst-num-traits API expansion and restructure
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the num-traits fork to const-num-traits, introducing a comprehensive suite of const-compatible numeric traits, carry-less and carry-propagating arithmetic, and zero-cost typestate proofs. Key changes include migrating calling conventions to pass arguments by value, splitting aggregate traits like PrimInt and Signed into capability-pure components, and adding constant-time operations. The review feedback identifies several timing side-channel vulnerabilities in the carry-less multiplication, bit-manipulation, and constant-time typestate constructor implementations, where branching on secret-derived data violates the documented constant-time (Tier A) guarantees. Actionable branchless alternatives are provided to eliminate these timing leaks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (rhs >> i) & 1 == 1 { | ||
| result ^= self << i; | ||
| } |
There was a problem hiding this comment.
The current implementation of carryless_mul branches on the secret-derived bits of rhs (if (rhs >> i) & 1 == 1). Since CarrylessMul is documented as a Tier A (CT-implementable) trait, its implementation should be completely branchless to prevent timing side-channel leaks.
We can achieve this branchlessly by using wrapping_neg() on the extracted bit to create a mask of all zeros or all ones, and then bitwise-ANDing it with the shifted value.
let mask = ((rhs >> i) & 1).wrapping_neg();
result ^= (self << i) & mask;| if (rhs >> i) & 1 == 1 { | ||
| result ^= wide << i; | ||
| } |
There was a problem hiding this comment.
The implementation of widening_carryless_mul branches on the secret-derived bits of rhs (if (rhs >> i) & 1 == 1). To maintain the constant-time (Tier A) guarantees of this module, this should be implemented branchlessly.
let mask = (((rhs >> i) & 1) as $w).wrapping_neg();
result ^= (wide << i) & mask;| if (rhs >> i) & 1 == 1 { | ||
| p ^= a << i; | ||
| } |
There was a problem hiding this comment.
The implementation of carrying_carryless_mul branches on the secret-derived bits of rhs (if (rhs >> i) & 1 == 1). To maintain the constant-time (Tier A) guarantees of this module, this should be implemented branchlessly.
let mask = (((rhs >> i) & 1) as $w).wrapping_neg();
p ^= (a << i) & mask;| if (b >> i) & 1 == 1 { | ||
| p ^= a << i; | ||
| } |
There was a problem hiding this comment.
The helper function wide_clmul_u64 branches on the bits of b (if (b >> i) & 1 == 1). To ensure constant-time execution for carry-less multiplication, this should be implemented branchlessly.
let mask = (((b >> i) & 1) as u128).wrapping_neg();
p ^= (a << i) & mask;| if self & bb != 0 { | ||
| result |= lowest; | ||
| } |
There was a problem hiding this comment.
The implementation of deposit_bits branches on the secret-derived value self & bb (if self & bb != 0). Since DepositBits is documented as a Tier A constant-time trait, this should be implemented branchlessly to prevent timing side-channel leaks.
let bit_mask = (((self & bb) != 0) as $t).wrapping_neg();
result |= lowest & bit_mask;| if self & lowest != 0 { | ||
| result |= bb; | ||
| } |
There was a problem hiding this comment.
The implementation of extract_bits branches on the secret-derived value self & lowest (if self & lowest != 0). Since ExtractBits is documented as a Tier A constant-time trait, this should be implemented branchlessly to prevent timing side-channel leaks.
let bit_mask = (((self & lowest) != 0) as $t).wrapping_neg();
result |= bb & bit_mask;| let exp = match value.checked_ilog2() { | ||
| Some(e) => e, | ||
| None => 0, | ||
| }; |
There was a problem hiding this comment.
The constant-time constructor new_ct for PowerOfTwo uses value.checked_ilog2(), which contains internal branching on whether the value is zero. Since this is a constant-time constructor handling potentially secret values, we should avoid branching.
We can compute the exponent completely branchlessly and safely by using value.trailing_zeros() % <$t>::BITS. If value is 0, trailing_zeros() returns BITS, and BITS % BITS wraps safely to 0 (which is < BITS), avoiding any out-of-bounds invariant violation.
// exp is unused when masked; trailing_zeros % BITS is branchless and safe even for 0.
let exp = value.trailing_zeros() % <$t>::BITS;![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a3bc7e004
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (rhs >> i) & 1 == 1 { | ||
| result ^= self << i; |
There was a problem hiding this comment.
When CarrylessMul is used with secret operands under the documented Tier-A/branchless guarantee, this branch leaks each bit of rhs through data-dependent control flow; the same pattern is repeated in the widening/carrying carry-less variants. Please compute a mask from the bit and XOR the shifted partial product conditionally rather than branching on operand data.
Useful? React with 👍 / 👎.
| /// Convert from a string and radix (typically `2..=36`); see | ||
| /// [`Num::from_str_radix`] for the conventions around supported | ||
| /// radices. | ||
| fn from_str_radix(str: &str, radix: u32) -> Result<Self, Self::Err>; |
There was a problem hiding this comment.
Adding another public root trait method named from_str_radix conflicts with Num::from_str_radix for existing users who do use const_num_traits::*; and call f32::from_str_radix(...): floats have no inherent method, so both trait associated functions are in scope and the call becomes ambiguous. Keep this atom out of the crate-root glob path or rename the method so existing glob-imported parsing code continues to compile.
Useful? React with 👍 / 👎.
| # `core::fmt::NumBuffer`). Kept separate from `nightly` so const-traits-only | ||
| # builds aren't exposed to churn in the `int_format_into` lang feature. | ||
| nightly-std = ["nightly"] | ||
| ct = ["dep:subtle"] |
There was a problem hiding this comment.
The previous manifest kept i128 = [] as a vestigial compatibility feature, and removing it makes any downstream crate that still declares features = ["i128"] fail during Cargo dependency resolution even though the feature is supposed to be always-on. Please keep the empty feature in this list while adding the new ct/nightly-std features.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We are aiming for an explicit API break here
… tripwire The proofs' only *safe* constructor was the per-primitive macro `new`; a non-primitive carrier (fixed big-integer, CT word) could build them only via the `unsafe` `from_*_unchecked`. That forces `unsafe` downstream and blocks e.g. `PowerOfTwoOps for FixedUInt` — a typestate that can't be constructed safely for the crate's own carrier types is upstream-broken. - `PowerOfTwo::new_checked` (`T: IsPowerOfTwo + PrimBits`): recovers the exponent as `BITS-1-leading_zeros` (width = `T::ZERO.count_zeros()`); the `unsafe` stays inside the crate. Per-primitive `const new` stays the fast path. - `BitIndex::new_checked` (`T: PrimBits`): `index < T::BITS` via the same width. - Both are `[c0nst]`-bounded, so const-callable on nightly for const carriers. Tripwire: tests/typestate_generic_carrier.rs constructs every carrier-generic proof (PowerOfTwo/BitIndex/Odd) from a non-primitive `Carrier` with no `unsafe`, via capability-bound-only helpers. If a future change re-narrows construction to primitives, it fails to compile — the regression guard mirroring prim_bits.rs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ops/wrapping.rs (1)
170-185:⚠️ Potential issue | 🟠 MajorChange shift trait bounds from
usizetou32to match parameter types and return projections.The
WrappingShlandWrappingShrtraits acceptrhs: u32but declare trait bounds and return type projections usingShl<usize>andShr<usize>. For custom types whereShl<u32>::Outputdiffers fromShl<usize>::Output, this mismatch advertises an incorrect return contract.Update trait bounds, return projections, and
Wrapping<T>where clauses fromusizetou32:Changes required
-pub c0nst trait WrappingShl: Sized + [c0nst] Shl<usize> { +pub c0nst trait WrappingShl: Sized + [c0nst] Shl<u32> { - fn wrapping_shl(self, rhs: u32) -> <Self as Shl<usize>>::Output; + fn wrapping_shl(self, rhs: u32) -> <Self as Shl<u32>>::Output; -pub c0nst trait WrappingShr: Sized + [c0nst] Shr<usize> { +pub c0nst trait WrappingShr: Sized + [c0nst] Shr<u32> { - fn wrapping_shr(self, rhs: u32) -> <Self as Shr<usize>>::Output; + fn wrapping_shr(self, rhs: u32) -> <Self as Shr<u32>>::Output; - Wrapping<T>: Shl<usize, Output = Wrapping<T>>, + Wrapping<T>: Shl<u32, Output = Wrapping<T>>, - Wrapping<T>: Shr<usize, Output = Wrapping<T>>, + Wrapping<T>: Shr<u32, Output = Wrapping<T>>,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ops/wrapping.rs` around lines 170 - 185, The WrappingShl trait definition declares trait bounds using Shl<usize> but the wrapping_shl method accepts rhs: u32, creating a type mismatch. Change the trait bounds in the WrappingShl trait declaration from Shl<usize> to Shl<u32>, and update the return type projection in the wrapping_shl method signature from <Self as Shl<usize>>::Output to <Self as Shl<u32>>::Output. Apply the same changes to the WrappingShr trait and any where clauses in Wrapping<T> implementations that reference usize shift operators, changing them all to u32 to match the parameter types.
🧹 Nitpick comments (1)
src/ops/overflowing.rs (1)
8-9: 🧹 Nitpick | 🔵 TrivialRemove legacy numeric constant imports.
The primitive types (
i8,i16,i32,i64,i128,isize,u8,u16,u32,u64,u128,usize) and their associated constants are available directly without importing thecore::*modules. The code only uses primitive associated constants likei8::MINandi16::MAX, which resolve without these imports.♻️ Suggested cleanup
-use core::{i128, i16, i32, i64, i8, isize}; -use core::{u128, u16, u32, u64, u8, usize};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ops/overflowing.rs` around lines 8 - 9, Remove the two import statements that import numeric primitive types from core module (the lines importing i8, i16, i32, i64, i128, isize and u8, u16, u32, u64, u128, usize). These imports are redundant because primitive type constants like i8::MIN and i16::MAX are available directly in Rust without explicit imports from the core module. Delete both legacy import lines entirely.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MIGRATION.md`:
- Around line 7-8: The MIGRATION.md file contains references to documentation
files that do not exist in the repository: API_BREAKS.md, COVERAGE.md,
DESIGN.md, and CLAUDE.md are referenced on lines 8, 27, and 75-76 respectively.
Either create these missing documentation files in the repository root with
appropriate content for each, or update all the references in MIGRATION.md to
point to actual documentation files that exist in the repository. Ensure all
links are relative paths and that the documentation content aligns with what the
MIGRATION.md guide is trying to reference.
In `@README.md`:
- Around line 1-32: The README has rebranded the crate as const-num-traits but
the dependency installation snippets still reference the old num-traits crate
name, which will misdirect users. Update both dependency configuration examples
in the file: replace num-traits with const-num-traits in the first inline
dependency snippet, and in the second detailed example update the
[dependencies.num-traits] section header to [dependencies.const-num-traits]
while also changing the version from "0.2" to "0.1" in both places to match the
actual package version.
In `@src/cast.rs`:
- Around line 91-103: The rustdoc comments for both the to_f32 and to_f64
functions contain a misspelling of the word "infinity" as "inifinity". Fix this
typo by correcting "inifinity" to "infinity" in both function documentation
comments.
In `@src/int.rs`:
- Around line 20-30: The PrimBits trait currently inherits from ConstZero and
ConstOne, which transitively require Add and Mul traits respectively,
undermining the design intent of PrimBits as a capability-pure bit layer without
full arithmetic trait contracts. Instead of inheriting ConstZero and ConstOne in
the PrimBits trait definition, create a minimal ConstBitIdentities trait that
provides only the ZERO and ONE constants needed for bit-twiddling operations
without exposing arithmetic trait requirements, or alternatively inline these
constants directly in PrimBits. Apply the same separation principle to the
PrimInt trait (which inherits from PrimBits) to ensure consistency across the
trait hierarchy.
In `@src/ops/bits.rs`:
- Around line 12-16: The documentation comment classifies DepositBits and
ExtractBits as Tier A constant-time operations, but the fallback implementations
at lines 464-475 and 485-496 contain mask-dependent loop counts and
data-dependent branches in the deposit_bits and extract_bits functions. Either
update the documentation comment to clarify that these fallback implementations
require both operands to be public to maintain constant-time guarantees, or
refactor the deposit_bits and extract_bits implementations to use fixed-width
masked logic without data-dependent branches to achieve true Tier A
constant-time behavior for secret operands.
In `@src/ops/checked.rs`:
- Around line 3-5: The module-level documentation claim that operations are
branchless overstates the constant-time guarantee because the file includes
checked_div, checked_rem, and checked_pow which have data-dependent (for
division and remainder) or exponent-dependent (for exponentiation) behavior.
Update the module-level comment at the top of the file to narrow the
constant-time claim by clarifying that only certain operations are branchless,
and explicitly exclude or note that division, remainder, and exponentiation
operations are not constant-time for secret-derived inputs. Also review and
update the individual documentation comments for the checked_div, checked_rem,
and checked_pow functions (around lines 94-138 and 320-338) to make clear these
operations are not branchless.
In `@src/ops/clmul.rs`:
- Line 10: The CT tier documentation on line 10 claims branchless
implementation, but the shift/XOR fallback loops at lines 43-49, 88-95, 143-150,
and 170-177 branch on operand bits during polynomial multiplication, leaking
timing information about secret operands. Replace each fallback's bit-branching
multiplication loop with a mask-select XOR accumulation pattern where instead of
conditionally branching on each bit, use bitwise operations to create masks
(typically by sign-extending or arithmetic-shifting the bit value) and
unconditionally XOR the partial products selected by these masks, ensuring all
operand-dependent iterations execute without branches.
In `@src/ops/euclid.rs`:
- Around line 397-405: The floating-point Euclid assertions are not using
absolute values to check reconstruction errors, allowing large negative errors
to pass. For each of the four assert statements checking div_euclid and
rem_euclid reconstruction, wrap the arithmetic expression on the left-hand side
of the <= comparison (the expressions computing Euclid::div_euclid(...) * ... +
Euclid::rem_euclid(...) combined with x) in `.abs()` to verify closeness in both
positive and negative directions against the epsilon tolerance.
In `@src/ops/overflowing.rs`:
- Around line 3-5: The file-level CT tier A comment at the beginning of the
module makes a blanket statement about all operations being CT-implementable,
but this is inaccurate since OverflowingDiv, OverflowingRem, and OverflowingPow
are data-dependent or exponent-dependent operations that cannot be implemented
in constant-time. Modify the file-level comment to scope the CT tier A
designation to only the operations that are actually CT-implementable,
explicitly noting that the blanket statement does not apply to division,
remainder, or power operations. Ensure the comment at lines 3-5 clearly reflects
this limited scope.
In `@src/ops/saturating.rs`:
- Around line 1-4: The module-level documentation at the top of saturating.rs
currently classifies all saturating operations as CT tier A with the branchless
select explanation, but this does not apply to the SaturatingDiv trait since
division is value-dependent and the trait panics on zero. Split the tier wording
in the module documentation to clearly separate the CT tier A classification
from the SaturatingDiv operation, ensuring callers understand that div-based
saturation is not constant-time-safe while other saturating operations remain CT
tier A compliant.
In `@src/ops/typestate.rs`:
- Around line 543-545: The safety documentation comment for the
NonMin::new_unchecked method contains an incorrect precondition. Change the
safety contract text from stating that value must not be less than MIN to
correctly stating that value must not be equal to MIN (value != MIN), since this
is the actual invariant required by the neg, abs, and div_nonzero operations
that depend on this type's constraint.
- Around line 807-817: In the PowerOfTwo::new_ct method, the match expression
using value.checked_ilog2() introduces data-dependent control-flow branching on
the condition value == 0, which leaks secret information in a constant-time
constructor. Replace the entire match statement that assigns the exp variable
with a single branchless call to value.trailing_zeros() (which returns the bit
position directly without branching), since the result is masked out by the
choice CtChoice anyway and does not require special handling for zero values.
In `@src/ops/wrapping.rs`:
- Around line 3-4: The module-level documentation comment claims all builtin
wrapping operations are branchless and universally constant-time safe, but this
is incorrect because WrappingDiv, WrappingRem, and WrappingPow have
data-dependent branching patterns. Update the documentation comment at the top
of the module to clarify that only certain wrapping operations are universally
CT-safe, and explicitly note that WrappingDiv, WrappingRem, and WrappingPow are
NOT universally constant-time safe due to their data-dependent behavior. Also
update the individual documentation comments for WrappingDiv, WrappingRem, and
WrappingPow to clearly indicate they should not be treated as safe for secret
data.
In `@src/sign.rs`:
- Around line 77-79: The abs method is using direct negation (-self) which
overflows when called on signed integer MIN values, causing panics in debug mode
and compilation errors in const contexts. Replace the negation operation with
wrapping_neg() in the abs method implementation (within the c0nst::c0nst! macro
block) to handle overflow correctly and return MIN as expected per the
documentation. Verify that wrapping_neg() is available as a stable const fn
under MSRV 1.86 for all target signed integer types; if not available in that
version, an alternative approach using conditional logic or bit manipulation may
be needed.
---
Outside diff comments:
In `@src/ops/wrapping.rs`:
- Around line 170-185: The WrappingShl trait definition declares trait bounds
using Shl<usize> but the wrapping_shl method accepts rhs: u32, creating a type
mismatch. Change the trait bounds in the WrappingShl trait declaration from
Shl<usize> to Shl<u32>, and update the return type projection in the
wrapping_shl method signature from <Self as Shl<usize>>::Output to <Self as
Shl<u32>>::Output. Apply the same changes to the WrappingShr trait and any where
clauses in Wrapping<T> implementations that reference usize shift operators,
changing them all to u32 to match the parameter types.
---
Nitpick comments:
In `@src/ops/overflowing.rs`:
- Around line 8-9: Remove the two import statements that import numeric
primitive types from core module (the lines importing i8, i16, i32, i64, i128,
isize and u8, u16, u32, u64, u128, usize). These imports are redundant because
primitive type constants like i8::MIN and i16::MAX are available directly in
Rust without explicit imports from the core module. Delete both legacy import
lines entirely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27b64830-0bf1-4e11-a8f6-9254a358e880
📒 Files selected for processing (38)
Cargo.tomlMIGRATION.mdREADME.mdsrc/bounds.rssrc/cast.rssrc/identities.rssrc/int.rssrc/lib.rssrc/ops/bits.rssrc/ops/bytes.rssrc/ops/carrying.rssrc/ops/checked.rssrc/ops/clmul.rssrc/ops/convert.rssrc/ops/ct.rssrc/ops/euclid.rssrc/ops/float_ops.rssrc/ops/format_into.rssrc/ops/from_ascii.rssrc/ops/inv.rssrc/ops/log.rssrc/ops/mixed.rssrc/ops/mod.rssrc/ops/mul_add.rssrc/ops/overflowing.rssrc/ops/parity.rssrc/ops/pow2.rssrc/ops/rounding.rssrc/ops/saturating.rssrc/ops/sqrt.rssrc/ops/strict.rssrc/ops/typestate.rssrc/ops/wrapping.rssrc/personality.rssrc/pow.rssrc/sign.rstests/const_nightly.rstests/prim_bits.rs
| # const-num-traits | ||
|
|
||
| A fork of [`num-traits`](https://github.com/rust-num/num-traits) with two goals: | ||
|
|
||
| 1. **Const traits**: the integer trait surface is `const trait` / `impl const` | ||
| on nightly (via the [`c0nst`](https://crates.io/crates/c0nst) macro crate), | ||
| while compiling to byte-identical plain traits on stable. | ||
| 2. **Complete coverage of modern Rust's integer operations**: every inherent | ||
| numeric method on the primitive integers in current nightly std — stable or | ||
| unstable — has a trait here, including the full | ||
| checked/wrapping/saturating/overflowing/strict variant matrix (`pow`, | ||
| `abs`, division, shifts, Euclidean division), bigint helpers | ||
| (`carrying_add`, `borrowing_sub`, `carrying_mul[_add]`, `widening_mul`, | ||
| carry-less multiplication), rounding (`div_ceil`/`div_floor`/`div_exact`, | ||
| `next_multiple_of`, `midpoint`), `ilog*`/`isqrt`/power-of-two ops, | ||
| bit manipulation (unbounded/funnel/exact shifts, bit isolation, | ||
| `bit_width`, PDEP/PEXT), mixed-signedness arithmetic, and conversions | ||
| (`cast_signed`/`cast_unsigned`, `widen`/`truncate`, generic | ||
| checked/wrapping/saturating/strict casts, `abs_diff`, `unsigned_abs`, | ||
| `clamp_magnitude`). | ||
|
|
||
| Operations whose std counterparts are newer than the MSRV (1.86) or still | ||
| unstable are hand-rolled with the same algorithms and panic messages as core, | ||
| so they work on stable Rust and inside `const` on nightly. The `unsafe` | ||
| `unchecked_*` family is deliberately not exposed. | ||
|
|
||
| Enable const traits with the `nightly` feature on a nightly compiler: | ||
|
|
||
| [](https://crates.io/crates/num-traits) | ||
| [](https://docs.rs/num-traits) | ||
| [](https://rust-lang.github.io/rfcs/2495-min-rust-version.html) | ||
| [](https://github.com/rust-num/num-traits/actions) | ||
| ```toml | ||
| [dependencies] | ||
| const-num-traits = { version = "0.1", features = ["nightly"] } | ||
| ``` |
There was a problem hiding this comment.
Fix dependency snippets to match the renamed crate.
The README now brands this as const-num-traits, but dependency instructions in the same file still reference num-traits, which will misdirect users during setup.
Suggested patch
## Usage
Add this to your `Cargo.toml`:
```toml
[dependencies]
-num-traits = "0.2"
+const-num-traits = "0.1"Features
This crate can be used without the standard library (#![no_std]) by disabling
the default std feature. Use this in Cargo.toml:
-[dependencies.num-traits]
-version = "0.2"
+[dependencies.const-num-traits]
+version = "0.1"
default-features = false
# features = ["libm"] # <--- Uncomment if you wish to use `Float` and `Real` without `std`</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @README.md around lines 1 - 32, The README has rebranded the crate as
const-num-traits but the dependency installation snippets still reference the
old num-traits crate name, which will misdirect users. Update both dependency
configuration examples in the file: replace num-traits with const-num-traits in
the first inline dependency snippet, and in the second detailed example update
the [dependencies.num-traits] section header to [dependencies.const-num-traits]
while also changing the version from "0.2" to "0.1" in both places to match the
actual package version.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- cr-comment:v1:e028e1a62ea598c1d74f382e -->
<!-- This is an auto-generated comment by CodeRabbit -->
| /// Converts the value of `self` to an `f32`. Overflows may map to positive | ||
| /// or negative inifinity, otherwise `None` is returned if the value cannot | ||
| /// be represented by an `f32`. | ||
| fn to_f32(&self) -> Option<f32>; | ||
| fn to_f32(&self) -> Option<f32> { | ||
| match self.to_f64() { | ||
| Some(v) => if v.is_finite() && (v < f32::MIN as f64 || v > f32::MAX as f64) { None } else { Some(v as f32) }, | ||
| None => None, | ||
| } | ||
| } | ||
|
|
||
| /// Converts the value of `self` to an `f64`. Overflows may map to positive | ||
| /// or negative inifinity, otherwise `None` is returned if the value cannot | ||
| /// be represented by an `f64`. |
There was a problem hiding this comment.
Correct typo in conversion docs (inifinity → infinity).
Two rustdoc comments contain the same misspelling, which leaks into generated documentation.
Suggested patch
- /// or negative inifinity, otherwise `None` is returned if the value cannot
+ /// or negative infinity, otherwise `None` is returned if the value cannot
@@
- /// or negative inifinity, otherwise `None` is returned if the value cannot
+ /// or negative infinity, otherwise `None` is returned if the value cannot📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Converts the value of `self` to an `f32`. Overflows may map to positive | |
| /// or negative inifinity, otherwise `None` is returned if the value cannot | |
| /// be represented by an `f32`. | |
| fn to_f32(&self) -> Option<f32>; | |
| fn to_f32(&self) -> Option<f32> { | |
| match self.to_f64() { | |
| Some(v) => if v.is_finite() && (v < f32::MIN as f64 || v > f32::MAX as f64) { None } else { Some(v as f32) }, | |
| None => None, | |
| } | |
| } | |
| /// Converts the value of `self` to an `f64`. Overflows may map to positive | |
| /// or negative inifinity, otherwise `None` is returned if the value cannot | |
| /// be represented by an `f64`. | |
| /// Converts the value of `self` to an `f32`. Overflows may map to positive | |
| /// or negative infinity, otherwise `None` is returned if the value cannot | |
| /// be represented by an `f32`. | |
| fn to_f32(&self) -> Option<f32> { | |
| match self.to_f64() { | |
| Some(v) => if v.is_finite() && (v < f32::MIN as f64 || v > f32::MAX as f64) { None } else { Some(v as f32) }, | |
| None => None, | |
| } | |
| } | |
| /// Converts the value of `self` to an `f64`. Overflows may map to positive | |
| /// or negative infinity, otherwise `None` is returned if the value cannot | |
| /// be represented by an `f64`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cast.rs` around lines 91 - 103, The rustdoc comments for both the to_f32
and to_f64 functions contain a misspelling of the word "infinity" as
"inifinity". Fix this typo by correcting "inifinity" to "infinity" in both
function documentation comments.
| /// The [`ConstZero`]/[`ConstOne`] supertraits provide the `ZERO`/`ONE` | ||
| /// constants the bit-twiddling defaults need without requiring callable | ||
| /// constructors in `const` context. | ||
| /// | ||
| /// This trait and many of the method names originate in the unstable `core::num::Int` trait from | ||
| /// the rust standard library. The original trait was never stabilized and thus removed from the | ||
| /// standard library. | ||
| pub c0nst trait PrimInt: | ||
| /// [`ConstZero`]: crate::ConstZero | ||
| /// [`ConstOne`]: crate::ConstOne | ||
| pub c0nst trait PrimBits: | ||
| Sized | ||
| + Copy | ||
| + [c0nst] Num | ||
| + [c0nst] NumCast | ||
| + [c0nst] Bounded | ||
| + [c0nst] PartialOrd | ||
| + [c0nst] Ord | ||
| + [c0nst] Eq | ||
| + ConstZero | ||
| + ConstOne |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the identity traits that PrimBits inherits through
# ConstZero/ConstOne. Expect to see whether Zero/One expose comparison or
# arithmetic APIs beyond the ZERO/ONE constants needed for bit defaults.
rg -n -C4 'pub (c0nst )?trait (Zero|One|ConstZero|ConstOne)\b' src/identities.rs
rg -n -C8 'pub c0nst trait PrimBits' src/int.rsRepository: kaidokert/num-traits
Length of output: 1722
🏁 Script executed:
sed -n '360,385p' src/int.rsRepository: kaidokert/num-traits
Length of output: 845
Refactor PrimBits to avoid inheriting full arithmetic trait contracts.
PrimBits is documented as a capability-pure bit layer for types that cannot expose comparison or arithmetic operations. However, inheriting ConstZero and ConstOne forces transitive inheritance of Zero and One, which require Add and Mul respectively. This undermines the design intent and blocks constant-time/capability-restricted types.
Define a minimal ConstBitIdentities trait (or inline constants directly) instead:
Suggested direction
+pub trait ConstBitIdentities {
+ const ZERO: Self;
+ const ONE: Self;
+}
+
pub c0nst trait PrimBits:
Sized
+ Copy
- + ConstZero
- + ConstOne
+ + ConstBitIdentities
+ [c0nst] Not<Output = Self>
+ [c0nst] BitAnd<Output = Self>
+ [c0nst] BitOr<Output = Self>Also applies to: 365-380 (PrimInt inherits PrimBits and will benefit from this separation).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/int.rs` around lines 20 - 30, The PrimBits trait currently inherits from
ConstZero and ConstOne, which transitively require Add and Mul traits
respectively, undermining the design intent of PrimBits as a capability-pure bit
layer without full arithmetic trait contracts. Instead of inheriting ConstZero
and ConstOne in the PrimBits trait definition, create a minimal
ConstBitIdentities trait that provides only the ZERO and ONE constants needed
for bit-twiddling operations without exposing arithmetic trait requirements, or
alternatively inline these constants directly in PrimBits. Apply the same
separation principle to the PrimInt trait (which inherits from PrimBits) to
ensure consistency across the trait hierarchy.
There was a problem hiding this comment.
How do you figure that Zero and One require Add and Mul ?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C10 'pub (c0nst )?trait (ConstZero|ConstOne)\b' src/identities.rsLength of output: 2641
🏁 Script executed:
#!/bin/bash
rg -n -C5 'pub (c0nst )?trait (Zero|One)\b' src/identities.rsLength of output: 918
@kaidokert, from the actual definitions in src/identities.rs:
// Line 16
pub c0nst trait Zero: Sized + [c0nst] Add<Self, Output = Self> { ... }
// Line 146
pub c0nst trait One: Sized + [c0nst] Mul<Self, Output = Self> { ... }And then:
pub trait ConstZero: Zero { const ZERO: Self; }
pub trait ConstOne: One { const ONE: Self; }So the transitive chain is:
PrimBits: ConstZero: Zero: Add<Self, Output = Self>
PrimBits: ConstOne: One: Mul<Self, Output = Self>
That's how — ConstZero is a supertrait refinement of Zero, which itself requires Add, and ConstOne is a supertrait refinement of One, which requires Mul. So any type implementing PrimBits must also implement Add and Mul, which conflicts with the stated design goal of supporting constant-time types that "cannot expose those" operations.
🧬 Code Graph Analysis Results
Relevant snippets for understanding why Zero/One imply Add/Mul identities:
src/identities.rs(lines 36-39)
pub trait ConstZero: Zero {
/// The additive identity element of `Self`, `0`.
const ZERO: Self;
}(Shows that ConstZero depends on a Zero trait and provides the additive identity constant ZERO.)
src/identities.rs(lines 167-170)
pub trait ConstOne: One {
/// The multiplicative identity element of `Self`, `1`.
const ONE: Self;
}(Shows that ConstOne depends on a One trait and provides the multiplicative identity constant ONE.)
src/int.rs(for context: definition ofPrimBitsbounds; excerpt around where it requiresConstZero/ConstOne)
pub c0nst trait PrimBits:
Sized
+ Copy
+ ConstZero
+ ConstOne
+ [c0nst] Not<Output = Self>
+ [c0nst] BitAnd<Output = Self>
+ [c0nst] BitOr<Output = Self>
+ [c0nst] BitXor<Output = Self>
+ [c0nst] Shl<usize, Output = Self>
+ [c0nst] Shr<usize, Output = Self>
{
// ...
}(Indicates PrimBits requires ConstZero and ConstOne, which in turn require Zero and One.)
| //! **CT tiers**: [`IsolateHighestOne`]/[`IsolateLowestOne`], [`BitWidth`], | ||
| //! [`DepositBits`]/[`ExtractBits`] and the funnel/unbounded shifts (under | ||
| //! the public-parameter convention for shift amounts) are Tier A; | ||
| //! [`HighestOne`]/[`LowestOne`] and [`ShlExact`]/[`ShrExact`] are Tier B | ||
| //! (`Option` returns). |
There was a problem hiding this comment.
Don’t classify PDEP/PEXT fallbacks as Tier A while they branch on operands.
deposit_bits/extract_bits have mask-dependent loop counts and data-dependent branches. Either document that both operands must be public for these fallbacks, or switch to a fixed-width masked implementation before advertising them as Tier A.
📝 Minimal documentation fix if these are not intended to be CT for secret operands
-//! [`IsolateHighestOne`]/[`IsolateLowestOne`], [`BitWidth`],
-//! [`DepositBits`]/[`ExtractBits`] and the funnel/unbounded shifts (under
-//! the public-parameter convention for shift amounts) are Tier A;
+//! [`IsolateHighestOne`]/[`IsolateLowestOne`], [`BitWidth`] and the
+//! funnel/unbounded shifts (under the public-parameter convention for shift
+//! amounts) are Tier A;
+//! [`DepositBits`]/[`ExtractBits`] are only CT for public operands in this
+//! portable fallback; the loop count and branches depend on the mask/input.Also applies to: 464-475, 485-496
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ops/bits.rs` around lines 12 - 16, The documentation comment classifies
DepositBits and ExtractBits as Tier A constant-time operations, but the fallback
implementations at lines 464-475 and 485-496 contain mask-dependent loop counts
and data-dependent branches in the deposit_bits and extract_bits functions.
Either update the documentation comment to clarify that these fallback
implementations require both operands to be public to maintain constant-time
guarantees, or refactor the deposit_bits and extract_bits implementations to use
fixed-width masked logic without data-dependent branches to achieve true Tier A
constant-time behavior for secret operands.
| //! Saturating arithmetic. | ||
| //! | ||
| //! **CT tier A (CT-implementable)**: saturation is expressible as a | ||
| //! branchless select on the overflow mask. |
There was a problem hiding this comment.
Don’t classify SaturatingDiv as CT tier A.
The module-level CT note now covers the new SaturatingDiv, but division is value-dependent and this trait also panics on zero. Split the tier wording so callers don’t treat div-based saturation as constant-time-safe.
Suggested wording
-//! **CT tier A (CT-implementable)**: saturation is expressible as a
-//! branchless select on the overflow mask.
+//! **CT tier A/C**: add/sub/mul/neg/abs/pow-style saturation is expressible as
+//! a branchless select on an overflow mask; `SaturatingDiv` remains CT-hostile
+//! because it performs division and panics on a zero divisor.Also applies to: 125-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ops/saturating.rs` around lines 1 - 4, The module-level documentation at
the top of saturating.rs currently classifies all saturating operations as CT
tier A with the branchless select explanation, but this does not apply to the
SaturatingDiv trait since division is value-dependent and the trait panics on
zero. Split the tier wording in the module documentation to clearly separate the
CT tier A classification from the SaturatingDiv operation, ensuring callers
understand that div-based saturation is not constant-time-safe while other
saturating operations remain CT tier A compliant.
| /// # Safety | ||
| /// `value` must not be `<$t>::MIN`. | ||
| #[inline] |
There was a problem hiding this comment.
Fix the NonMin::new_unchecked safety contract text.
The safety precondition is currently vacuous (value is never < MIN). It should state value != MIN, which is the invariant consumed by neg/abs/div_nonzero.
Suggested patch
/// # Safety
- /// `value` must not be `<$t>::MIN`.
+ /// `value` must be `!= <$t>::MIN`.
#[inline]
pub const unsafe fn new_unchecked(value: $t) -> Self { NonMin(value) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// # Safety | |
| /// `value` must not be `<$t>::MIN`. | |
| #[inline] | |
| /// # Safety | |
| /// `value` must be `!= <$t>::MIN`. | |
| #[inline] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ops/typestate.rs` around lines 543 - 545, The safety documentation
comment for the NonMin::new_unchecked method contains an incorrect precondition.
Change the safety contract text from stating that value must not be less than
MIN to correctly stating that value must not be equal to MIN (value != MIN),
since this is the actual invariant required by the neg, abs, and div_nonzero
operations that depend on this type's constraint.
| pub fn new_ct(value: $t) -> CtOption<PowerOfTwo<$t>> { | ||
| let choice = value.ct_is_power_of_two(); | ||
| // exp is unused when masked; checked_ilog2 dodges ilog2(0). | ||
| let exp = match value.checked_ilog2() { | ||
| Some(e) => e, | ||
| None => 0, | ||
| }; | ||
| // SAFETY: meaningful only when `choice` is set (value is a | ||
| // power of two, so `exp < BITS`); masked out otherwise. | ||
| let proof = unsafe { PowerOfTwo::from_exp_unchecked(exp) }; | ||
| CtOption::new(proof, choice) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the constructor currently branches on checked_ilog2
rg -n -C3 'fn new_ct|checked_ilog2|match value\.checked_ilog2' src/ops/typestate.rsRepository: kaidokert/num-traits
Length of output: 2519
🏁 Script executed:
# Check if this file has crypto/constant-time context
head -50 src/ops/typestate.rs | grep -i "const\|time\|crypto\|secret"Repository: kaidokert/num-traits
Length of output: 658
🌐 Web query:
Rust checked_ilog2 implementation const-time timing-safe
💡 Result:
In Rust, the checked_ilog2 method is not strictly guaranteed to be constant-time [1][2]. While the standard library implementations are optimized for performance, they are primarily designed for general-purpose utility rather than for cryptographic applications requiring constant-time, timing-safe execution [3][4][5]. The implementation of checked_ilog2 relies on underlying CPU instructions like leading_zeros, which are typically very fast and often constant-time on modern hardware [6]. However, because the standard library methods do not explicitly guarantee constant-time execution as a security requirement, you should not assume they are immune to timing attacks. Key points regarding the implementation: 1. Performance Optimization: Recent improvements to the integer logarithm implementations in the Rust standard library have focused on efficiency, moving from iterative division to multiplication-based approaches for generic bases [4][5]. 2. Not Cryptographic: If you require a strictly constant-time, timing-safe logarithm calculation for cryptographic operations, the standard library methods should not be relied upon. Instead, you should implement or use a specialized library designed for constant-time arithmetic that avoids branching and variable-time CPU instructions based on secret input values. 3. Compiler/Optimizer Behavior: Even if an implementation appears constant-time in source code, the Rust compiler (via LLVM) may optimize the code in ways that introduce variable execution timing, depending on the architecture and optimization levels [7][4][8]. For security-critical contexts, always perform a side-channel analysis of the compiled machine code for your specific target architecture.
Citations:
- 1: https://stdrs.dev/nightly/x86_64-pc-windows-gnu/core/primitive.u64.html
- 2: https://doc.rust-lang.org/std/primitive.u32.html
- 3: Improve performance of
ilogandchecked_ilogfor primitive integer types rust-lang/rust#115874 - 4: Unroll first iteration of checked_ilog loop rust-lang/rust#124294
- 5: checked_ilog: improve performance rust-lang/rust#115913
- 6: https://github.com/rust-lang/rust/blob/024757fe/library/core/src/num/int_macros.rs
- 7: Optimize
checked_ilogandpowwhenbaseis a power of two rust-lang/rust#147250 - 8: https://rust.googlesource.com/rust/+/58652d5a4312b2d2549d2f631bc301df8e4d7aae/tests/codegen-llvm/checked_ilog.rs
🌐 Web query:
Rust trailing_zeros implementation constant-time timing-safe
💡 Result:
The Rust trailing_zeros method is implemented using the cttz (count trailing zeros) compiler intrinsic [1][2], which maps directly to hardware instructions (like TZCNT or BSF on x86) where available [3][4]. Regarding constant-time and timing-safety: 1. Performance Characteristics: While trailing_zeros is generally highly efficient and often maps to a single CPU instruction [3], the specific assembly generated depends on the target architecture, compiler optimizations, and the specific CPU instruction used [4]. Hardware instructions like BSF (Bit Scan Forward) on some older processors had variable execution times depending on the input, and some implementations included conditional logic to handle the zero-input case (where the result is defined as the bit-width of the type) [4][5]. 2. Compiler Optimizations: Modern LLVM (the backend used by the Rust compiler) is highly effective at optimizing these calls [4][6]. When trailing_zeros is used, the compiler often generates efficient branchless code [4]. However, because the exact instruction sequence can vary based on the target and optimization level, it is not strictly guaranteed to be constant-time across all possible hardware and compiler versions [4]. 3. Timing-Safety: If your application requires strict constant-time execution to prevent timing-based side-channel attacks (e.g., in cryptographic contexts), you should not rely solely on the standard library's trailing_zeros without verifying the generated assembly for your specific target [4]. In such critical scenarios, it is standard practice to audit the machine code output or use specialized, constant-time primitives if available for your platform. 4. Non-Zero Optimization: For cases where the input is guaranteed to be non-zero, using NonZeroU* types (e.g., NonZeroU32) can allow the compiler to skip zero-check logic, potentially resulting in more predictable or efficient code paths [3][5]. In summary, while trailing_zeros is safe to call and implemented efficiently, its timing characteristics are subject to the underlying hardware and compiler behavior, and it should be treated as potentially variable-time if strictly constant-time execution is required for security [4][5].
Citations:
- 1: https://github.com/rust-lang/rust/blob/f375177a/library/core/src/num/uint_macros.rs
- 2: https://github.com/rust-lang/rust/blob/81799cd8fd841e23b52876ae5e22faeb3ad04eb5/library/core/src/num/int_macros.rs
- 3: Tracking Issue for feature(nonzero_leading_trailing_zeros) rust-lang/rust#79143
- 4: https://stackoverflow.com/questions/74620023/why-does-u64trailing-zeros-generate-branched-assembly-when-branchless-works
- 5: https://internals.rust-lang.org/t/leading-zeros-has-ub-again/23882
- 6:
x.trailing_zeros() > nis not optimized as well asx & ((1 << n) - 1) == 0on x86 rust-lang/rust#43024
Remove data-dependent branching from PowerOfTwo::new_ct.
The match value.checked_ilog2() introduces control-flow branching on value == 0, leaking a secret-derived bit in a CT constructor. Replace with trailing_zeros() to keep exponent derivation branchless (the result is masked out by choice anyway).
Suggested patch
pub fn new_ct(value: $t) -> CtOption<PowerOfTwo<$t>> {
let choice = value.ct_is_power_of_two();
- // exp is unused when masked; checked_ilog2 dodges ilog2(0).
- let exp = match value.checked_ilog2() {
- Some(e) => e,
- None => 0,
- };
+ // Branchless exponent derivation (unused when masked by `choice`):
+ // For power-of-two N: trailing_zeros(N) == ilog2(N)
+ let exp = value.trailing_zeros() & (<$t>::BITS - 1);
// SAFETY: meaningful only when `choice` is set (value is a
// power of two, so `exp < BITS`); masked out otherwise.
let proof = unsafe { PowerOfTwo::from_exp_unchecked(exp) };
CtOption::new(proof, choice)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn new_ct(value: $t) -> CtOption<PowerOfTwo<$t>> { | |
| let choice = value.ct_is_power_of_two(); | |
| // exp is unused when masked; checked_ilog2 dodges ilog2(0). | |
| let exp = match value.checked_ilog2() { | |
| Some(e) => e, | |
| None => 0, | |
| }; | |
| // SAFETY: meaningful only when `choice` is set (value is a | |
| // power of two, so `exp < BITS`); masked out otherwise. | |
| let proof = unsafe { PowerOfTwo::from_exp_unchecked(exp) }; | |
| CtOption::new(proof, choice) | |
| pub fn new_ct(value: $t) -> CtOption<PowerOfTwo<$t>> { | |
| let choice = value.ct_is_power_of_two(); | |
| // Branchless exponent derivation (unused when masked by `choice`): | |
| // For power-of-two N: trailing_zeros(N) == ilog2(N) | |
| let exp = value.trailing_zeros() & (<$t>::BITS - 1); | |
| // SAFETY: meaningful only when `choice` is set (value is a | |
| // power of two, so `exp < BITS`); masked out otherwise. | |
| let proof = unsafe { PowerOfTwo::from_exp_unchecked(exp) }; | |
| CtOption::new(proof, choice) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ops/typestate.rs` around lines 807 - 817, In the PowerOfTwo::new_ct
method, the match expression using value.checked_ilog2() introduces
data-dependent control-flow branching on the condition value == 0, which leaks
secret information in a constant-time constructor. Replace the entire match
statement that assigns the exp variable with a single branchless call to
value.trailing_zeros() (which returns the bit position directly without
branching), since the result is masked out by the choice CtChoice anyway and
does not require special handling for zero values.
| //! **CT tier A (CT-implementable)**: branchless on the data for all the | ||
| //! builtin integers. |
There was a problem hiding this comment.
Do not classify wrapping div/rem/pow as universally CT-safe.
The module now includes WrappingDiv, WrappingRem, and WrappingPow, but the top-level CT tier says all builtin wrapping operations are branchless. Division/remainder are data-dependent, and pow is exponent-dependent, so callers could incorrectly treat these as safe for secrets.
🛡️ Suggested wording adjustment
-//! **CT tier A (CT-implementable)**: branchless on the data for all the
-//! builtin integers.
+//! **CT tier A (CT-implementable)** for modular add/sub/mul/neg/shifts/abs
+//! on builtin integers. Division/remainder are CT-hostile, and
+//! exponentiation is exponent-dependent.Also applies to: 238-297, 339-351
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ops/wrapping.rs` around lines 3 - 4, The module-level documentation
comment claims all builtin wrapping operations are branchless and universally
constant-time safe, but this is incorrect because WrappingDiv, WrappingRem, and
WrappingPow have data-dependent branching patterns. Update the documentation
comment at the top of the module to clarify that only certain wrapping
operations are universally CT-safe, and explicitly note that WrappingDiv,
WrappingRem, and WrappingPow are NOT universally constant-time safe due to their
data-dependent behavior. Also update the individual documentation comments for
WrappingDiv, WrappingRem, and WrappingPow to clearly indicate they should not be
treated as safe for secret data.
| fn abs(self) -> $t { | ||
| if self.is_negative() { -self } else { self } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Rust signed integer unary negation overflow i32::MIN debug const eval wrapping_neg const stable
💡 Result:
In Rust, applying unary negation to the most negative value of a signed integer type (such as i32::MIN) results in overflow [1]. The behavior of this operation in different contexts is as follows: 1. Runtime (Non-const): In debug mode, this operation will cause a panic [1]. In release mode, it will wrap (using two's complement arithmetic), returning the original i32::MIN value [2][3][1]. 2. Const Evaluation: In a const context, attempting to negate i32::MIN will result in a compile-time error [4]. The compiler does not allow operations that would cause undefined behavior or overflow in a const context, and unary negation of i32::MIN is explicitly flagged as a constant evaluation error [4]. 3. Wrapping Negation: The method wrapping_neg is the standard way to perform modular (wrapping) negation [2][3]. It is a stable, const-compatible method [2][3]. Unlike the standard negation operator (-), wrapping_neg is guaranteed to return i32::MIN when called on i32::MIN, regardless of compiler flags or context [2][3]. Since it is a const fn, it can be used in const contexts to safely perform this operation without triggering a panic or compile-time error [2][3].
Citations:
- 1: https://doc.rust-lang.org/reference/expressions/operator-expr.html
- 2: https://doc.rust-lang.org/stable/std/primitive.i32.html
- 3: https://doc.rust-lang.org/stable/core/primitive.i32.html
- 4: https://fuchsia.googlesource.com/third_party/rust/+/e804a3cf256106c097d44f6e0212cd183122da07/src/test/compile-fail/const-eval-overflow.rs
🏁 Script executed:
head -n 90 src/sign.rs | tail -n 30Repository: kaidokert/num-traits
Length of output: 853
🏁 Script executed:
head -n 50 src/sign.rsRepository: kaidokert/num-traits
Length of output: 1716
🏁 Script executed:
cat -n src/sign.rs | sed -n '60,90p'Repository: kaidokert/num-traits
Length of output: 1094
🏁 Script executed:
grep -r "wrapping_neg" src/ || echo "Not found in src"Repository: kaidokert/num-traits
Length of output: 1295
🏁 Script executed:
grep -i "msrv\|rust-version" Cargo.toml README.md 2>/dev/null | head -20Repository: kaidokert/num-traits
Length of output: 181
🏁 Script executed:
head -n 1 src/sign.rs && git log --oneline src/sign.rs 2>/dev/null | head -5 || echo "Git info not available"Repository: kaidokert/num-traits
Length of output: 176
🏁 Script executed:
grep -A 5 "fn wrapping_neg" src/ops/wrapping.rs | head -20Repository: kaidokert/num-traits
Length of output: 469
🏁 Script executed:
cat src/ops/wrapping.rs | grep -A 2 "const fn"Repository: kaidokert/num-traits
Length of output: 46
🏁 Script executed:
grep -B 5 "fn abs(self)" src/sign.rs | head -15Repository: kaidokert/num-traits
Length of output: 525
🏁 Script executed:
cat -n src/sign.rs | sed -n '35,50p'Repository: kaidokert/num-traits
Length of output: 936
🏁 Script executed:
rg "c0nst::|const fn" src/sign.rs | head -10Repository: kaidokert/num-traits
Length of output: 232
🏁 Script executed:
cat -n src/sign.rs | sed -n '1,10p'Repository: kaidokert/num-traits
Length of output: 352
🏁 Script executed:
# Check if this is using the c0nst crate and what it does
grep -A 10 "c0nst::c0nst!" src/lib.rs 2>/dev/null || rg "c0nst" src/lib.rs 2>/dev/null | head -5Repository: kaidokert/num-traits
Length of output: 6223
🏁 Script executed:
# Verify wrapping_neg availability in stable Rust and const contexts
web_search Rust wrapping_neg const fn stable 1.86Repository: kaidokert/num-traits
Length of output: 115
🌐 Web query:
Rust wrapping_neg const fn stable availability Rust 1.86
💡 Result:
As of June 22, 2026, the wrapping_neg method on integer types is stable as a const fn in Rust. While the wrapping_neg method itself has been stable since Rust 1.10.0, its status as a const fn was tracked under the unstable feature const_ops (issue #143802) for an extended period [1][2][3]. It is currently available as a stable const fn in the language. Regarding your specific mention of Rust 1.86 (which was released on April 3, 2025) [4][5], the release notes for that version do not mention the stabilization of wrapping_neg as a const fn [6]. The stabilization occurred after that release. The const availability of wrapping_neg is a separate development from the general stable availability of the method itself [7][8][2].
Citations:
- 1: https://rust.docs.kernel.org/src/core/num/wrapping.rs.html
- 2: https://github.com/rust-lang/rust/blob/master/library/core/src/num/wrapping.rs
- 3: https://doc.rust-lang.org/stable/src/core/num/wrapping.rs.html
- 4: https://github.com/rust-lang/rust/releases/tag/1.86.0
- 5: https://blog.rust-lang.org/2025/04/03/Rust-1.86.0/
- 6: https://releases.rs/docs/1.86.0/
- 7: https://doc.rust-lang.org/std/primitive.u32.html
- 8: Make overflowing and wrapping negation const rust-lang/rust#58044
Avoid overflow when abs is called on signed MIN.
The docs say signed integer MIN returns MIN, but -self overflows for that input, causing panics in debug mode and compilation errors in const contexts. The code is wrapped in c0nst::c0nst! to support both const and non-const contexts. Using wrapping_neg() fixes the overflow, but verify const fn availability under MSRV 1.86; if wrapping_neg was not a stable const fn in that version, an alternative approach may be needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sign.rs` around lines 77 - 79, The abs method is using direct negation
(-self) which overflows when called on signed integer MIN values, causing panics
in debug mode and compilation errors in const contexts. Replace the negation
operation with wrapping_neg() in the abs method implementation (within the
c0nst::c0nst! macro block) to handle overflow correctly and return MIN as
expected per the documentation. Verify that wrapping_neg() is available as a
stable const fn under MSRV 1.86 for all target signed integer types; if not
available in that version, an alternative approach using conditional logic or
bit manipulation may be needed.
MSRV 1.86 already clears edition 2024's 1.85 floor; no source migration was needed (stable/no_std/ct/nightly + all doctests build unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Forks
num-traitsinto a const-trait crate via thec0nstmacro: byte-identical to upstream on stable, realconst trait/impl conston nightly with--features nightly.What's here
Zero/One/Num/PrimInt/Checked*/Wrapping*/Saturating*/Overflowing*/Euclid/Signed/ToPrimitive/…).strict, bigint (carrying+clmul),rounding,log/sqrt/pow2,bits,mixed-sign,convert(casts/widen/truncate),float_ops,from_ascii,format_into.Copyreuse:PrimBits,Signum,RingOps,FromStrRadix; apreludemodule.core, always available, zero-cost unused):PowerOfTwo,BitIndex, theNonZerobridge +DivNonZero,NonNegative/Positive/NonMin,Odd/Even,Finite.ctfeature (optionalsubtledep): masked-return constant-time atoms.Breaking vs
num-traits(deliberate — seeMIGRATION.md)x.checked_add(y), notx.checked_add(&y).Output; cross-type ops useSigned/Unsigned/Wide.PrimInt→PrimBits+pow,Signed→Signum, …).This is not a drop-in for
num-traits; downstream call sites need the mechanical changes inMIGRATION.md.Toolchain / features
nightly/nightly-stdare experimental — they ride bleeding-edge const-trait syntax throughc0nstand can break on rustc-nightly churn (e.g. Rejectimpl const Traitsince the right syntax isconst impl Traitnow rust-lang/rust#158009).default = ["std"],libm,std,nightly,nightly-std,ct.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
ctfeature using thesubtlecrate)Documentation
Refactor
Selfreturn values