ACP: add `{integer}::unchecked_div` and `{integer}::unchecked_rem`

[tgross35]

Proposal

Problem statement

core::intrinsics provides unchecked_add, unchecked_div, unchecked_mul, unchecked_rem, unchecked_shl, unchecked_shr, and unchecked_sub. The add, div, mul, shl, shr, and sub versions are available as i32::unchecked_mul or similar, but there is no i32::unchecked_div or i32::unchecked_rem.

Motivating examples or use cases

This behavior can already be achieved today with x.checked_div(y).unwrap_unchecked(), so the main motivation is consistency with the other unchecked_* methods. A side benefit is lighter codegen in debug mode compared to unwrap_unchecked, but this may not always be true (if we choose to add an assert_unsafe_precondition at some point in the future).

Solution sketch

// in core

impl {i8, u8, i16, u16, i32, u32, i64, u64, i128, u128} {
    // Same preconditions as the versions in intrinsics

    /// Performs an unchecked division, resulting in undefined behavior where y == 0 or x == T::MIN && y == -1.
    pub fn unchecked_div(self, rhs: Self) -> Self;

    /// Returns the remainder of an unchecked division, resulting in undefined behavior when y == 0 or x == T::MIN && y == -1.
    pub fn unchecked_rem(self, rhs: Self) -> Self;
}

Alternatives

Continue using checked_div with unwrap_unchecked.

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.

[scottmcm]

Can you say more about where you wanted to use this on signed numbers?

A side benefit is lighter codegen in debug mode compared to unwrap_unchecked, but this may not always be true

We should absolutely not treat this as a reason, and if these are added, they should have the precondition check from day one. All the others (or at the very least nearly all the others) have them as of rust-lang/rust#121571

but there is no i32::unchecked_div

Notes that there's https://doc.rust-lang.org/std/primitive.u32.html#impl-Div%3CNonZero%3Cu32%3E%3E-for-u32 but there's not i32: Div<NonZero<i32>>, because that's not enough to satisfy the safety preconditions.

One reason I really like delegating the unsafety through NonZero here is that it helps treat these things separately. If an i32::unchecked_div were to be added, I'd be tempted to have it still take NonZero<i32> as the RHS, to emphasize that there's another precondition.

Otherwise it's really easy to write

    // SAFETY: z isn't zero
    unsafe { a.unchecked_div(z) }

and have a soundness bug.

[tgross35]

Can you say more about where you wanted to use this on signed numbers?

Most of my initial usecase came from investigating removal of the intrinsics call added at rust-lang/libm@fe396e0, and being surprised that there isn't a stable equivalent. This is used with signed arithmetic to calculate LUT offsets, but there isn't any technical reason that it couldn't be casted to unsigned for division (though signed -> unsigned -> NonZero -> div -> signed is a bit of an awkward dance).

A side benefit is lighter codegen in debug mode compared to unwrap_unchecked, but this may not always be true

We should absolutely not treat this as a reason, and if these are added, they should have the precondition check from day one. All the others (or at the very least nearly all the others) have them as of rust-lang/rust#121571

I don't disagree. The only reason I mentioned it is because the code linked above cites using the intrinsic as a workaround for rust-lang/rust#72751, but I don't find the motivation overly strong.

Notes that there's https://doc.rust-lang.org/std/primitive.u32.html#impl-Div%3CNonZero%3Cu32%3E%3E-for-u32 but there's not i32: Div<NonZero<i32>>, because that's not enough to satisfy the safety preconditions.

One reason I really like delegating the unsafety through NonZero here is that it helps treat these things separately. If an i32::unchecked_div were to be added, I'd be tempted to have it still take NonZero<i32> as the RHS, to emphasize that there's another precondition.

Otherwise it's really easy to write

// SAFETY: z isn't zero
unsafe { a.unchecked_div(z) }

and have a soundness bug.

For what it's worth, the existing unchecked_* methods are all the same as their checked_* methods:

Calling x.unchecked_mul(y) is semantically equivalent to calling x.checked_mul(y).unwrap_unchecked().

but there isn't any specific reason for this to be the same. That being said, anybody using these needs to be reading the precondition docs anyway...

[Amanieu]

We discussed this in the libs-api meeting and decided to accept this. The main argument in favor was consistency with the other unchecked_* operations. Additionally, as these methods are unsafe, so their preconditions are well documented.

[tgross35]

Opened rust-lang/rust#136716 to track this