bpo-35959: Fix division by 0 when checking for overflow by pablogsal · Pull Request #11808 · python/cpython (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This fix the issue, do you think we still need to consume the whole iterable thought?
Maybe we could break early?
This fix the issue, do you think we still need to consume the whole iterable thought? Maybe we could break early?
If something like ('nan') appear afterwards, the result need to be ('nan') so we cannot break earlier.
| @@ -1756,6 +1756,10 @@ def test_prod(self): |
|---|
| with self.assertRaises(TypeError): |
| prod([10, 20], [30, 40]) # start is a keyword-only argument |
| self.assertEqual(prod([0, 1, 2, 3]), 0) |
| self.assertEqual(prod([1, 0, 2, 3]), 0) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[1, 2, 3, 0] was already working fine without this fix so is it worth adding this as a test case? I have less knowledge with C so please ignore if this is not needed.
self.assertEqual(prod([1, 2, 3, 0]), 0)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added this test and removed the range base one. Even if that case worked fine because it takes a different code path, I like to have the 0 in the start/middle/end.
Thanks for the catch!
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR fixes my case. Thanks for the explanation on the zero by division code path.