Skip to content

Commit

Permalink
Merge #745
Browse files Browse the repository at this point in the history
745: Do not sum options r=phimuemue a=Philippe-Cholet

A mistake on my part, discovered in time as code is still commented out. Note that it passed tests.
It only returns None if the iterator yields None. But it would overflow whenever an addition does.

Do not product options either, as multiplications could overflow the same way. That's how I found out.

I guess `checked_sum` and `checked_product` methods would be nice.
EDIT: ~Would you be interested?~ Related: rust-lang/rust#95485

Co-authored-by: Philippe-Cholet <phcholet@orange.fr>
  • Loading branch information
bors[bot] and Philippe-Cholet authored Sep 6, 2023
2 parents c7a038e + 79a1b15 commit 557cb89
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 6 deletions.
3 changes: 0 additions & 3 deletions src/combinations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,6 @@ fn remaining_for(n: usize, first: bool, indices: &[usize]) -> Option<usize> {
indices
.iter()
.enumerate()
// TODO: Once the MSRV hits 1.37.0, we can sum options instead:
// .map(|(i, n0)| checked_binomial(n - 1 - *n0, k - i))
// .sum()
.fold(Some(0), |sum, (i, n0)| {
sum.and_then(|s| s.checked_add(checked_binomial(n - 1 - *n0, k - i)?))
})
Expand Down
3 changes: 0 additions & 3 deletions src/combinations_with_replacement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,6 @@ fn remaining_for(n: usize, first: bool, indices: &[usize]) -> Option<usize> {
indices
.iter()
.enumerate()
// TODO: Once the MSRV hits 1.37.0, we can sum options instead:
// .map(|(i, n0)| count(n - 1 - *n0, k - i))
// .sum()
.fold(Some(0), |sum, (i, n0)| {
sum.and_then(|s| s.checked_add(count(n - 1 - *n0, k - i)?))
})
Expand Down

0 comments on commit 557cb89

Please sign in to comment.