Conversation
The naive product-then-divide binomial helper overflowed usize (C(27, 7) on 32-bit, C(21, 19) even on 64-bit). Use the same overflow-safe algorithm as checked_binomial.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1131 +/- ##
==========================================
- Coverage 94.38% 93.83% -0.56%
==========================================
Files 48 52 +4
Lines 6665 6491 -174
==========================================
- Hits 6291 6091 -200
- Misses 374 400 +26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| if n < k { | ||
| return 0; | ||
| } | ||
| // Same algorithm as `checked_binomial` in `src/adaptors/mod.rs`. |
There was a problem hiding this comment.
If it's the same algorithm, can't we just call checked_binomial?
There was a problem hiding this comment.
Because checked_binomial is pub(crate) inside the private adaptors module, it isn't visible to the integration test crates in tests/.
If you'd prefer to share the implementation across tests rather than keeping the local helper, I can expose it with #[doc(hidden)] pub use crate::adaptors::checked_binomial; in lib.rs and call it here as itertools::checked_binomial(n, k).unwrap_or(0). Would you like me to do that?
This branch has not been deployed
2 participants
Fixes #995.
combinations_inexact_size_hintspanics on 32-bit because the test helper computes binomial coefficients with a naive product-then-divide. That overflowsusizeeven when the result fits (C(27, 7)on 32-bit;C(21, 19)even on 64-bit). Use the same overflow-safe algorithm aschecked_binomial.Test plan
cargo test --test test_std binomial_avoids_intermediate_overflow— failed (overflow panic) before the helper change, passes aftercargo test --test test_std combinations—combinations_inexact_size_hintsandcombinations_range_countpasscargo test --tests— all integration tests passcargo fmt --all