mbutrovich commented on code in PR #11425:
URL: https://github.com/apache/arrow-rs/pull/11425#discussion_r4221432062
##########
arrow-buffer/src/bigint/mod.rs:
##########
@@ -572,6 +572,48 @@ impl i256 {
self.div_rem(other).map(|(_, v)| v).ok()
}
+ /// Performs wrapping division and remainder, returning `(quotient,
remainder)`
+ ///
+ /// # Panics
+ ///
+ /// Panics if `other` is zero
+ ///
+ /// # Example
+ /// ```
+ /// # use arrow_buffer::i256;
+ /// let (q, r) = i256::from_i128(-7).wrapping_div_rem(i256::from_i128(2));
+ /// assert_eq!((q, r), (i256::from_i128(-3), i256::from_i128(-1)));
+ ///
+ /// let (q, r) = i256::MIN.wrapping_div_rem(i256::MINUS_ONE);
+ /// assert_eq!((q, r), (i256::MIN, i256::ZERO));
+ /// ```
+ #[inline]
+ pub fn wrapping_div_rem(self, other: Self) -> (Self, Self) {
+ match self.div_rem(other) {
+ Ok(v) => v,
+ Err(DivRemError::DivideByZero) => panic!("attempt to divide by
zero"),
+ Err(_) => (Self::MIN, Self::ZERO),
+ }
+ }
Review Comment:
> This is now the third copy of the zero-divisor panic and the `MIN / -1`
fallback, after `wrapping_div` and `wrapping_rem` above. Those two could become
`self.wrapping_div_rem(other).0` and `.1`, which keeps the error handling in
one place and lets `test_wrapping_div_rem_zero_panics` cover the panic for all
three.
Thanks, `wrapping_div` and `wrapping_rem` now call `wrapping_div_rem` and
take `.0` and `.1`.
> Matching `Err(DivRemError::DivideOverflow)` instead of `Err(_)` would also
turn a future `DivRemError` variant into a compile error here instead of a
silent `(MIN, 0)`.
The match now names `DivRemError::DivideOverflow`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
