[issue27073] redundant checks in long_add and long_sub

Roundup Robot added the comment: New changeset c21bf38a9d07 by Serhiy Storchaka in branch 'default': Issue #27073: Removed redundant checks in long_add and long_sub. https://hg.python.org/cpython/rev/c21bf38a9d07 -- nosy: +python-dev ___ Python tracke

[issue27073] redundant checks in long_add and long_sub

Changes by Serhiy Storchaka : -- type: behavior -> performance ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue27073] redundant checks in long_add and long_sub

Serhiy Storchaka added the comment: LGTM (except a trailing space in a comment). Thank you for your contribution Oren. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue27073] redundant checks in long_add and long_sub

Changes by Oren Milman : Added file: http://bugs.python.org/file43164/patchedCPythonTestOutput.txt ___ Python tracker ___ ___ Python-bugs-list

[issue27073] redundant checks in long_add and long_sub

Oren Milman added the comment: I considered doing that, but I had already opened another issue (http://bugs.python.org/issue27145) in which I had proposed to replace that in-place negate in long_sub with a call to _PyLong_Negate. But I guess I shouldn't worry about my patches colliding. Anyway

[issue27073] redundant checks in long_add and long_sub

Serhiy Storchaka added the comment: Maybe add an assert for the second size negation? -- ___ Python tracker ___ ___ Python-bugs-list m

[issue27073] redundant checks in long_add and long_sub

Oren Milman added the comment: All right. The updated diff file is attached. I compiled and ran the tests again. They are quite the same. The test output is attached. -- Added file: http://bugs.python.org/file43144/issue27073.diff ___ Python tracker

[issue27073] redundant checks in long_add and long_sub

Changes by Oren Milman : Added file: http://bugs.python.org/file43145/patchedCPythonTestOutput2.txt ___ Python tracker ___ ___ Python-bugs-lis

[issue27073] redundant checks in long_add and long_sub

Serhiy Storchaka added the comment: It would be nice. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https:

[issue27073] redundant checks in long_add and long_sub

Oren Milman added the comment: I agree. This assert only indirectly verifies that something bad doesn't happen. The bad thing that might happen is an in-place negating of an element of small_ints, so the most direct assert should be 'assert(Py_REFCNT(z) == 1);'. This is exactly what Victor di

[issue27073] redundant checks in long_add and long_sub

Serhiy Storchaka added the comment: I don't think this assert is needed. Nothing bad happens if the asserted condition is false. On other side, additional assert can slow down debug build (that is already slower than release build). -- ___ Python tr

[issue27073] redundant checks in long_add and long_sub

Oren Milman added the comment: And after quadruple checking myself, I found a foolish mistake - in that flow, x_add received at least one multiple-digit int (not necessarily two :(). I fixed that mistake in the comment. The updated diff file is attached. -- Added file: http://bugs.pyth

[issue27073] redundant checks in long_add and long_sub

Oren Milman added the comment: After giving it some more thought (while working on another, somewhat related issue - http://bugs.python.org/issue27145), I realized that that assert in long_add could further verify that the int x_add returned is a multiple-digit int (as x_add had received two m

[issue27073] redundant checks in long_add and long_sub

Oren Milman added the comment: Thanks for the reviews! I added an assert in long_add (in long_sub it might be that the result is 0). The updated diff file is attached. -- Added file: http://bugs.python.org/file42926/issue27073.diff ___ Python tracker

[issue27073] redundant checks in long_add and long_sub

STINNER Victor added the comment: Sorry, I didn't check if the change is valid or not, but: > issue.diff Please keep the check but as an assertion (Put it in the if block). -- ___ Python tracker ___

[issue27073] redundant checks in long_add and long_sub

Mark Dickinson added the comment: Your analysis and patch look good to me. -- nosy: +mark.dickinson ___ Python tracker ___ ___ Python-

[issue27073] redundant checks in long_add and long_sub

Changes by SilentGhost : -- nosy: +haypo, serhiy.storchaka, yselivanov stage: -> patch review type: -> behavior versions: +Python 3.6 ___ Python tracker ___ ___

[issue27073] redundant checks in long_add and long_sub

Changes by Oren Milman : Added file: http://bugs.python.org/file42922/patchedCPythonTestOutput.txt ___ Python tracker ___ ___ Python-bugs-list

[issue27073] redundant checks in long_add and long_sub

Changes by Oren Milman : Added file: http://bugs.python.org/file42921/CPythonTestOutput.txt ___ Python tracker ___ ___ Python-bugs-list mailin

[issue27073] redundant checks in long_add and long_sub

New submission from Oren Milman: the proposed changes I believe the following checks are redundant: 1. in Objects/longobject.c in long_add: In case both a and b are negative, their absolute values are added using x_add, with the result stored in z. I