Skip to content

Commit c8bafe0

Browse files
committed
auto merge of #17248 : jbcrail/rust/fix-rational-rounding, r=alexcrichton
When I fixed the previous issue with rational rounding, I had introduced a regression. There was also an overflow bug introduced for fixed-precision rationals. This patch corrects both bugs.
2 parents c669411 + 363c264 commit c8bafe0

File tree

1 file changed

+44
-5
lines changed

1 file changed

+44
-5
lines changed

src/libnum/rational.rs

+44-5
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,29 @@ impl<T: Clone + Integer + PartialOrd>
139139
/// Rounds to the nearest integer. Rounds half-way cases away from zero.
140140
#[inline]
141141
pub fn round(&self) -> Ratio<T> {
142-
if *self < Zero::zero() {
143-
// a/b - 1/2 = (2*a - b)/(2*b)
144-
Ratio::from_integer((self.numer + self.numer - self.denom) / (self.denom + self.denom))
142+
let one: T = One::one();
143+
let two: T = one + one;
144+
145+
// Find unsigned fractional part of rational number
146+
let fractional = self.fract().abs();
147+
148+
// The algorithm compares the unsigned fractional part with 1/2, that
149+
// is, a/b >= 1/2, or a >= b/2. For odd denominators, we use
150+
// a >= (b/2)+1. This avoids overflow issues.
151+
let half_or_larger = if fractional.denom().is_even() {
152+
*fractional.numer() >= *fractional.denom() / two
145153
} else {
146-
// a/b + 1/2 = (2*a + b)/(2*b)
147-
Ratio::from_integer((self.numer + self.numer + self.denom) / (self.denom + self.denom))
154+
*fractional.numer() >= (*fractional.denom() / two) + one
155+
};
156+
157+
if half_or_larger {
158+
if *self >= Zero::zero() {
159+
self.trunc() + One::one()
160+
} else {
161+
self.trunc() - One::one()
162+
}
163+
} else {
164+
self.trunc()
148165
}
149166
}
150167

@@ -382,6 +399,7 @@ mod test {
382399
use std::from_str::FromStr;
383400
use std::hash::hash;
384401
use std::num;
402+
use std::i32;
385403

386404
pub static _0 : Rational = Ratio { numer: 0, denom: 1};
387405
pub static _1 : Rational = Ratio { numer: 1, denom: 1};
@@ -616,6 +634,27 @@ mod test {
616634
assert_eq!(_1.floor(), _1);
617635
assert_eq!(_1.round(), _1);
618636
assert_eq!(_1.trunc(), _1);
637+
638+
// Overflow checks
639+
640+
let _neg1 = Ratio::from_integer(-1);
641+
let _large_rat1 = Ratio::new(i32::MAX, i32::MAX-1);
642+
let _large_rat2 = Ratio::new(i32::MAX-1, i32::MAX);
643+
let _large_rat3 = Ratio::new(i32::MIN+2, i32::MIN+1);
644+
let _large_rat4 = Ratio::new(i32::MIN+1, i32::MIN+2);
645+
let _large_rat5 = Ratio::new(i32::MIN+2, i32::MAX);
646+
let _large_rat6 = Ratio::new(i32::MAX, i32::MIN+2);
647+
let _large_rat7 = Ratio::new(1, i32::MIN+1);
648+
let _large_rat8 = Ratio::new(1, i32::MAX);
649+
650+
assert_eq!(_large_rat1.round(), One::one());
651+
assert_eq!(_large_rat2.round(), One::one());
652+
assert_eq!(_large_rat3.round(), One::one());
653+
assert_eq!(_large_rat4.round(), One::one());
654+
assert_eq!(_large_rat5.round(), _neg1);
655+
assert_eq!(_large_rat6.round(), _neg1);
656+
assert_eq!(_large_rat7.round(), Zero::zero());
657+
assert_eq!(_large_rat8.round(), Zero::zero());
619658
}
620659

621660
#[test]

0 commit comments

Comments
 (0)