Skip to content

Commit 73598ce

Browse files
committed
Fix incorrect overflow handling in binomial
In cases where `k` was 2 or 3, no guard was used, so this function could return `None`, when the general algorithm can compute the actual Some(_). Also, checked arithmetic was used when it is not needed, since the initial guards are enough to assure no overflows will occur. Tests where changed to include these cases.
1 parent 0b9fa05 commit 73598ce

File tree

1 file changed

+11
-41
lines changed

1 file changed

+11
-41
lines changed

src/iter/combinations.rs

Lines changed: 11 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use std::marker::PhantomData;
2-
use std::ops::Div;
32
use std::sync::Arc;
43

54
use super::plumbing::*;
@@ -424,32 +423,12 @@ fn checked_binomial(n: usize, k: usize) -> Option<usize> {
424423
// Fast paths x2 ~ x6 speedup
425424
match k {
426425
1 => return Some(n),
427-
2 => return n.checked_mul(n - 1).map(|r| r / 2),
428-
3 => {
429-
return n
430-
.checked_mul(n - 1)?
431-
.div(2)
432-
.checked_mul(n - 2)
433-
.map(|r| r / 3)
434-
}
435-
4 if n <= 77_937 /* prevent unnecessary overflow */ => {
436-
return n
437-
.checked_mul(n - 1)?
438-
.div(2)
439-
.checked_mul(n - 2)?
440-
.checked_mul(n - 3)
441-
.map(|r| r / 12)
442-
}
443-
5 if n <= 10_811 /* prevent unnecessary overflow */ => {
444-
return n
445-
.checked_mul(n - 1)?
446-
.div(2)
447-
.checked_mul(n - 2)?
448-
.checked_mul(n - 3)?
449-
.div(4)
450-
.checked_mul(n - 4)
451-
.map(|r| r / 15)
452-
}
426+
// The guards prevent unnecessary overflows,
427+
// those cases can be handled by the general checked algorithm
428+
2 if n <= 4_294_967_296 => return Some(n * (n - 1) / 2),
429+
3 if n <= 3_329_022 => return Some(n * (n - 1) / 2 * (n - 2) / 3),
430+
4 if n <= 77_937 => return Some(n * (n - 1) / 2 * (n - 2) * (n - 3) / 12),
431+
5 if n <= 10_811 => return Some(n * (n - 1) / 2 * (n - 2) * (n - 3) / 4 * (n - 4) / 15),
453432
_ => {}
454433
}
455434

@@ -653,20 +632,11 @@ mod tests {
653632
#[test]
654633
#[cfg(target_pointer_width = "64")]
655634
fn checked_binomial_not_overflows() {
656-
assert_eq!(
657-
checked_binomial(4_294_967_296, 2),
658-
Some(9_223_372_034_707_292_160)
659-
);
660-
assert_eq!(
661-
checked_binomial(3_329_022, 3),
662-
Some(6_148_913_079_097_324_540)
663-
);
664-
assert_eq!(
665-
checked_binomial(102_570, 4),
666-
Some(4_611_527_207_790_103_770)
667-
);
668-
assert_eq!(checked_binomial(13_467, 5), Some(3_688_506_309_678_005_178));
669-
assert_eq!(checked_binomial(109, 15), Some(1_015_428_940_004_440_080));
635+
assert!(checked_binomial(4_294_967_297, 2).is_some());
636+
assert!(checked_binomial(3_329_023, 3).is_some());
637+
assert!(checked_binomial(102_570, 4).is_some());
638+
assert!(checked_binomial(13_467, 5).is_some());
639+
assert!(checked_binomial(109, 15).is_some());
670640
}
671641

672642
#[test]

0 commit comments

Comments
 (0)