Infallible div_ceil for NonZero<unsigned>

Proposal

Problem statement

The NonZero<T> types currently doesn't have any division functions. If you need to perform divisions on these types, you need to convert it to its underlying type, do the division and convert it back. It would make sense to provide these methods directly on the types.

Motivating examples or use cases

I recently needed this when calculating the number of sheets needed for printing a document using duplex printing.
As a document can never have 0 pages, I represent the page count as a NonZero<u32>. The number of sheets required can then be calculated with (assuming that div_ceil exists on NonZero<u32>):

fn duplex_sheets(pages: NonZero<u32>) -> NonZero<u32> {
    // SAFETY: 2 is not zero
    const TWO: NonZero<u32> = unsafe { NonZero::<u32>::new_unchecked(2) };
    pages.div_ceil(TWO)
}

Solution sketch

NonZero<unsigned> should implement div_ceil as it would be panic-free and could never produce a zero:

impl NonZero<u8> { // similarly for u16, u32, u64, u128 & usize
    pub const fn div_ceil(self, other: Self) -> Self;
}

I think it would be weird to just add just `div_ceil` without also adding a checked version of "regular" division. The signed `NonZero` types should probably also have the checked "regular" division function.
So in conclusion I think the following functions should be added to core:

impl NonZero<u8> { // similarly for u16, u32, u64, u128 & usize pub const fn checked_div(self, other: Self) -> Option<Self>; pub const fn div_ceil(self, other: Self) -> Self; } impl NonZero<i8> { // similarly for i16, i32, i64, i128 & isize pub const fn checked_div(self, other: Self) -> Option<Self>; }

Alternatives

Alternatively users can define these themselves using an extension trait or using a separate function.

For example:

use std::num::NonZero;

pub(crate) trait NonZeroUnsignedExt {
    fn div_ceil(self, divisor: Self) -> Self;
}

impl NonZeroUnsignedExt for NonZero<u32> {
    fn div_ceil(self, divisor: Self) -> Self {
        let v = self.get().div_ceil(divisor.get());
        // SAFETY: `v` can never be zero
        unsafe { Self::new_unchecked(v) }
    }
}

The user will either need to use unsafe or unwrap to create the result, which they could avoid if div_ceil was implemented in core.

Links and related work

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 think it would be weird to just add just div_ceil without also adding a checked version of "regular" division. The signed NonZero types should probably also have the checked "regular" division function.

I disagree. The existing API surface of these types is pretty much only functions that can't already be written without unwrap or unsafe code. Normal division as you propose is just NonZero::new(x.get() / y.get())

I think it would be weird to just add just div_ceil without also adding a checked version of "regular" division. The signed NonZero types should probably also have the checked "regular" division function.

I disagree. The existing API surface of these types is pretty much only functions that can't already be written without unwrap or unsafe code. Normal division as you propose is just NonZero::new(x.get() / y.get())

Note that NonZero::new(x.get() / y.get()) only works for unsigned numbers.

For signed numbers you would need x.get().checked_div(y.get()).and_then(NonZero::new), as i8::MIN / -1 overflows.

For signed numbers you would need x.get().checked_div(y.get()).and_then(NonZero::new), as i8::MIN / -1 overflows.

That's not an equivalent operation to "regular division" which will panic on overflow.

The existing API surface of these types is pretty much only functions that can't already be written without unwrap or unsafe code.

Yeah, this. That's why there's no NonZeroU32::wrapping_add but there is NonZeroU32::checked_add, for example.

So I agree with skipping something that's just .and_then(NonZero::new) after an operation on the normal integers.

EDIT later for clarity: they're useful when they're .map(|x| unsafe { NonZero::new_unchecked(x) }) to avoid unsafe in the caller, just not when they need an additional check to be safe that could be done with the normal version anyway.

For signed numbers you would need x.get().checked_div(y.get()).and_then(NonZero::new), as i8::MIN / -1 overflows.

That's not an equivalent operation to "regular division" which will panic on overflow.

Sure, but it would be insane to add a checked_div function returning Option<Self> that could panic.

The existing API surface of these types is pretty much only functions that can't already be written without unwrap or unsafe code.

Yeah, this. That's why there's no NonZeroU32::wrapping_add but there is NonZeroU32::checked_add, for example.

So I agree with skipping something that's just .and_then(NonZero::new) after an operation on the normal integers.

But there is a NonZero<T>::checked_mul, which could as well be implemented as x.get().checked_mul(y.get()).and_then(NonZero::new).

Not exactly. We know that n*m>0 iff n>0 and m>0. So we actually don't need to check if the result of the operation is zero, we just need to check that it doesn't overflow. The and_then(NonZero::new) would be redundant.

For div, you have to check if the result is zero regardless, you can't make any assumptions about it based on the fact that both inputs are nonzero. So in the division case, the and_then(NonZero::new) is necessary.

Fair enough. I've updated the proposal to only propose the div_ceil for NonZero<unsigned>.

Seems reasonable; let's accept this.

Rollup merge of rust-lang#132665 - tyilo:nonzero-u-div-ceil, r=joboet

Unrolled build for rust-lang#132665

Rollup merge of #132665 - tyilo:nonzero-u-div-ceil, r=joboet

Navigation Menu

Repository navigation

Infallible `div_ceil` for `NonZero<unsigned>` #471

Proposal

Problem statement

Motivating examples or use cases

Solution sketch

Alternatives

Links and related work

What happens now?

Possible responses

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Infallible div_ceil for NonZero<unsigned> #471

Description

Proposal

Problem statement

Motivating examples or use cases

Solution sketch

Alternatives

Links and related work

What happens now?

Possible responses

Activity

pitaj commented on Oct 28, 2024

tyilo commented on Oct 28, 2024

pitaj commented on Oct 28, 2024

scottmcm commented on Oct 29, 2024

tyilo commented on Oct 29, 2024

tyilo commented on Oct 29, 2024

pitaj commented on Oct 29, 2024

tyilo commented on Oct 29, 2024

joshtriplett commented on Nov 5, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Infallible `div_ceil` for `NonZero<unsigned>` #471