rust-lang
ACP: `try_exact_div` method on `NonZero<{integer}>`
Proposal
Problem statement
Given x: NonZero<{unsigned_integer}> and y: NonZero<{unsigned_integer}> if x % y is zero then x / y is guaranteed to not be zero.
In some computations one needs to compute modulo and then divide if it's zero, so it'd be helpful to preserve this property. Today, the optimizer doesn't understand this property so unsafe might be needed to optimize it rather than relying on unwrap. It'd be helpful to share the burden of reviewing the correctness among all crates that need to do this.
Motivating examples or use cases
In decimal formatting, to display the number after decimal point it's useful to normalize it by removing the trailing zeros. This can be done by dividing by 10 in a loop while the remainder is 0. While NonZero doesn't need to be used at all in the code, it's still helpful to express this property because the number after decimal point being zero is special and needs to be handled differently (not adding the dot). And it's also nice to express that the non-zero number stayed non-zero.
Solution sketch
impl<T: Div<Output=T> + Mod<Output=T>> NonZero<T> {
/// Divides the number if it's exactly divisible.
///
/// In that case the result is guaranteed to be non-zero. In case of error, the remainder is returned.
fn try_exact_div(self, rhs: Self) -> Result<Self, Self> {
match NonZero::new(self % rhs) {
None => unsafe { Ok(NonZero::new_unchecked(self / rhs)) },
Some(remainder) => Err(remainder),
}
}
}
Alternatives
- Ignore this - it may be legitimately too niche. (But the function is also very simple, is it worth it?)
- Teach the optimizer about this pattern instead, so people could use
unwrapwithout perf hit.
Links and related work
https://internals.rust-lang.org/t/div-mod-for-nonzero-t-is-it-worth-acp
What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
- We think this problem seems worth solving, and the standard library might be the right place to solve it.
- We think that this probably doesn't belong in the standard library.
Second, if there's a concrete solution:
- We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
- We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
i would call it checked_exact_div, see: https://github.com/rust-lang/rust/issues/139911
also, you need to check for signed overflow when doing i32::MIN / -1
Yeah, that could make sense. Signed is problematic since the remainder is always zero and then the method doesn't make sense. So it should be only for unsigned integers.
Signed is problematic since the remainder is always zero
it's still plenty useful for signed integers, just that checked_exact_div would return None in cases of overflow too. so i32::MIN.checked_exact_div(-1) == None, but there are non-zero remainders in signed division: -23i32.checked_exact_div(-10) == None and -30i32.checked_exact_div(-10) == Some(3)
The proposed API specifically returns Result so for Signed it'd have to be Result<NonZero<T>, Option<NonZero<T>>` or some kind of error type.
I think this is way too niche for the standard library, even though it could in theory avoid a NonZero::unchecked_new. In particular the signature doesn't match the ones for the proposed exact-div methods proposed in #337, especially the Err return type where nothing in the method name indicates that this is the remainder.
I'm fine with "this is too niche" (I even expected it) but signatures not matching is very usual and reasonable thing to do in Rust. E.g. Option::map and Iterator::map have different signatures because it makes sense. In the internals thread I proposed a separate enum which was more readable but it increased the API urface even more.
We've discussed this in the @rust-lang/libs-api meeting on 2025-06-03. We have two requests:
We'd like this method to be called exact_div_or_rem instead of try_exact_div. That clarifies the what it does (especially that the remainder can be returned) and doesn't get into the potentially confusing signatures as discussed above.
Second, this method could be useful for more than just unsigned NonZero integers and we want to see the whole picture between the various integer types and make sure it's consistent.
Can you please put together a table showing the main integer types, NonZero and both signed and unsigned and what would the result of this method call be?
Is this method supposed to panic or to return an Err when an overflow or division-by-zero occurs? It seems to me that neither is a particularly good option.
The original naming of try_exact_div would preclude panicking, and exact_div_or_rem doesn't quite cover all the possibilities. Whereas it might be acceptable to return the value of 0 for a remainder with overflow (e.g. i32::MIN / -1 mathematically has a remainder of 0), for division by zero the remainder is undefined.
If I had to choose a remainder, I would pick the original dividend a = any * 0 + r therefore r = a as the remainder, but it remains an undefined operation.
~~The solution sketch panics on div-by-zero (via calling self % rhs).~~ With the name exact_div_or_rem it is more clear that it should panic, and the non-panicking variant could be named checked_exact_div_or_rem or try_exact_div_or_rem.
If the return type Option<Result<Self, Self>> is too strange one could introduce an explicit type enum DivOrRem<T> { Div(NonZero<T>), Rem(NonZero<T>) } and make the functions return DivOrRem<T> and Option<DivOrRem<T>> instead.
The original proposal requires unsigned non-zero, so errors are impossible. But yes, the request to include the other combinations is a significant reason why I didn't have much time to respond to this yet as it complicates things.
@kennytm it doesn't because rhs is guaranteed to be non-zero.
@kennytm I like the Option<Result<A,B>> for a non-panicking variant. The only drawback I can see is not being able to distinguish the error in the None case immediately, but testing for that should be simple e.g. whether RHS is 0 or -1.
// May not panic
match a.try_exact_div_or_rem(b) {
Some(Ok(quot)) => ..,
Some(Err(rem)) => ..,
None => /* unknown cause.. */
}
As for the combination of types, I would suggest the following for fixed N.
| Type | Panicking Behaviour | Return Type |
|---|---|---|
uN / uN |
Division By Zero | Result<uN, uN> |
uN / NonZero<uN> |
N/A | Result<uN, NonZero<uN>> |
NonZero<uN> / uN |
Division By Zero | Result<NonZero<uN>, NonZero<uN>> |
NonZero<uN> / NonZero<uN> |
N/A | Result<NonZero<uN>, NonZero<uN>> |
iN / iN |
Division By Zero, Overflow | Result<iN, iN> |
iN / NonZero<iN> |
Overflow | Result<iN, NonZero<iN>> |
NonZero<iN> / iN |
Division By Zero, Overflow | Result<NonZero<iN>, NonZero<iN>> |
NonZero<iN> / NonZero<iN> |
Overflow | Result<NonZero<iN>, NonZero<iN>> |
Even though the remainder is never 0 for any of the combinations, I still suggest only returning a NonZero when either of the operands is NonZero.
@wmstack thanks for the table! I guess I agree with it mostly.
I still suggest only returning a
NonZerowhen either of the operands isNonZero.
Why? It's easy to do NonZero<T> -> T conversion but the reverse is more annoying. Although, one can try converting upfront, handling the error and pass it to the non-zero method instead, so maybe it's not a big deal.
I am happy if the remainder is wrapped in a NonZero in all cases, but @tomassedovic do you think this table adequately describes the desired combinations? This is no longer waiting on author.
Error: The feature shortcut is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.
Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.
@wmstack thank you! I don't have the rights to remove waiting-on-author but I've nominated it for the next libs-api discussion.
We talked about this in the libs-api call today. We wanted to compare what the code that's motivating this would look like both with and without this method, but we were struggling to simulate that in our heads during the call.
If you could please post playground-runnable examples of motivating code using this method, and comparable code not using this method, that would be helpful to us.
