Skip to content

Commit

Permalink
Auto merge of #79134 - ohadravid:nzint-div, r=dtolnay
Browse files Browse the repository at this point in the history
Add `impl Div<NonZeroU{0}> for u{0}` which cannot panic

Dividing an unsigned int by a `NonZeroUxx` requires a user to write (for example, in [this SO question](https://stackoverflow.com/questions/64855738/how-to-inform-the-optimizer-that-nonzerou32get-will-never-return-zero)):

```
pub fn safe_div(x: u32, y: std::num::NonZeroU32) -> u32 {
    x / y.get()
}
```

which generates a panicking-checked-div [assembly](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(fontScale:14,j:1,lang:rust,selection:(endColumn:2,endLineNumber:6,positionColumn:2,positionLineNumber:6,selectionStartColumn:2,selectionStartLineNumber:6,startColumn:2,startLineNumber:6),source:%27pub+fn+div(x:+u32,+y:+u32)+-%3E+u32+%7B%0A++++x+/+y%0A%7D%0Apub+fn+safe_div(x:+u32,+y:+std::num::NonZeroU32)+-%3E+u32+%7B%0A++++x+/+y.get()+//+an+unchecked+division+expected%0A%7D%27),l:%275%27,n:%270%27,o:%27Rust+source+%231%27,t:%270%27)),k:50,l:%274%27,n:%270%27,o:%27%27,s:0,t:%270%27),(g:!((h:compiler,i:(compiler:r1470,filters:(b:%270%27,binary:%271%27,commentOnly:%270%27,demangle:%270%27,directives:%270%27,execute:%271%27,intel:%270%27,libraryCode:%271%27,trim:%271%27),fontScale:14,j:1,lang:rust,libs:!(),options:%27-O%27,selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:%275%27,n:%270%27,o:%27rustc+1.47.0+(Editor+%231,+Compiler+%231)+Rust%27,t:%270%27)),k:50,l:%274%27,n:%270%27,o:%27%27,s:0,t:%270%27)),l:%272%27,n:%270%27,o:%27%27,t:%270%27)),version:4).
Avoiding the `panic` currently requires `unsafe` code.

This PR adds an `impl Div<NonZeroU{0}> for u{0}` (and `impl Rem<NonZeroU{0}> for u{0}`) which calls the `unchecked_div` (and `unchecked_rem`) intrinsic without any additional checks,
making the following code compile:

```
pub fn safe_div(x: u32, y: std::num::NonZeroU32) -> u32 {
    x / y
}

pub fn safe_rem(x: u32, y: std::num::NonZeroU32) -> u32 {
    x % y
}
```

The doc is set to match the regular div impl [docs](https://doc.rust-lang.org/beta/src/core/ops/arith.rs.html#460).

I've marked these as stable because (as I understand it) trait impls are automatically stable. I'm happy to change it to unstable if needed.

Following `@dtolnay` template from a similar issue:
this adds the following **stable** impls, which rely on dividing unsigned integers by nonzero integers being well defined and previously would have involved unsafe code to encode that knowledge:
```
impl Div<NonZeroU8> for u8 {
    type Output = u8;
}

impl Rem<NonZeroU8> for u8 {
    type Output = u8;
}
```
and equivalent for u16, u32, u64, u128, usize, but **not** for i8, i16, i32, i64, i128, isize (since -1/MIN is undefined).

r? `@dtolnay`
  • Loading branch information
bors committed Dec 27, 2020
2 parents dc6121c + 9586643 commit 2fab321
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
42 changes: 41 additions & 1 deletion library/core/src/num/nonzero.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Definitions of integer that is known not to equal zero.
use crate::fmt;
use crate::ops::{BitOr, BitOrAssign};
use crate::ops::{BitOr, BitOrAssign, Div, Rem};
use crate::str::FromStr;

use super::from_str_radix;
Expand Down Expand Up @@ -263,3 +263,43 @@ nonzero_leading_trailing_zeros! {
NonZeroI128(u128), -1i128;
NonZeroIsize(usize), -1isize;
}

macro_rules! nonzero_integers_div {
( $( $Ty: ident($Int: ty); )+ ) => {
$(
#[stable(feature = "nonzero_div", since = "1.51.0")]
impl Div<$Ty> for $Int {
type Output = $Int;
/// This operation rounds towards zero,
/// truncating any fractional part of the exact result, and cannot panic.
#[inline]
fn div(self, other: $Ty) -> $Int {
// SAFETY: div by zero is checked because `other` is a nonzero,
// and MIN/-1 is checked because `self` is an unsigned int.
unsafe { crate::intrinsics::unchecked_div(self, other.get()) }
}
}

#[stable(feature = "nonzero_div", since = "1.51.0")]
impl Rem<$Ty> for $Int {
type Output = $Int;
/// This operation satisfies `n % d == n - (n / d) * d`, and cannot panic.
#[inline]
fn rem(self, other: $Ty) -> $Int {
// SAFETY: rem by zero is checked because `other` is a nonzero,
// and MIN/-1 is checked because `self` is an unsigned int.
unsafe { crate::intrinsics::unchecked_rem(self, other.get()) }
}
}
)+
}
}

nonzero_integers_div! {
NonZeroU8(u8);
NonZeroU16(u16);
NonZeroU32(u32);
NonZeroU64(u64);
NonZeroU128(u128);
NonZeroUsize(usize);
}
16 changes: 16 additions & 0 deletions library/core/tests/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,19 @@ fn nonzero_trailing_zeros() {
const TRAILING_ZEROS: u32 = NonZeroU16::new(1 << 2).unwrap().trailing_zeros();
assert_eq!(TRAILING_ZEROS, 2);
}

#[test]
fn test_nonzero_uint_div() {
let nz = NonZeroU32::new(1).unwrap();

let x: u32 = 42u32 / nz;
assert_eq!(x, 42u32);
}

#[test]
fn test_nonzero_uint_rem() {
let nz = NonZeroU32::new(10).unwrap();

let x: u32 = 42u32 % nz;
assert_eq!(x, 2u32);
}

0 comments on commit 2fab321

Please sign in to comment.