Repository navigation
gh-123497: New limit for Python integers on 64-bit platforms - #123498
serhiy-storchaka wants to merge 7 commits into
Conversation
Sorry, something went wrong.
Instead of be limited just by the size of addressable memory (2**63 bytes), Python integers are now also limited by the number of bits, so the number of bit now always fit in 64-bit integer. Both limits are much larger than what can be available on practice, so there is no effect on users. _PyLong_NumBits() and _PyLong_Frexp() are now always successful.
vstinner
left a comment
There was a problem hiding this comment.
Overall, the change LGTM, but it would be easier to review and it may make more sense if splitted in 3 parts:
- change MAX_LONG_DIGITS limit and related changes -- maybe put long_rshift() and long_lshift() in this part
- change _PyLong_Frexp()
- _PyLong_NumBits() is always successful, bit_length(), bit_count(), etc.
Sorry, something went wrong.
|
Is it necessary? The PR is not so large and I can split it on even smaller part parts, but this is an additional work and is prone for errors. I can not guarantee that the intermediate state will be correct. |
Sorry, something went wrong.
| if (dx == 1.0) { | ||
| if (a_bits == INT64_MAX) | ||
| goto overflow; | ||
| assert(a_bits < UINT64_MAX); |
There was a problem hiding this comment.
uint64_t a_bits; cannot be greater than UINT64_MAX.
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, but it can be equal.
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM. See also @erlend-aasland's coding style suggestions.
Sorry, something went wrong.
Co-authored-by: Victor Stinner <vstinner@python.org>
|
I have no idea what are the problems with building on Windows. Building locally fails with the same error, but running the reported command manually succeeded. And it seems that _PyLong_NumBits() is not even called during the run. |
Sorry, something went wrong.
|
Fixed building on Windows. MAX_LONG_DIGITS was incorrectly set on 32-bit platforms. #123724 is an alternative PR that uses int64_t instead of uint64_t for bit counts. capi-workgroup/api-evolution#52 is a discussion about using unsigned or signed types. I have no strong preferences, although the code with unsigned types is slightly simpler (bit count is non-negative by its nature). |
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
In terms of the implementation, I prefer unsigned, it avoids to check if a number is negative by design (even for assertions). So, what do you choose? |
Sorry, something went wrong.
|
Merged the signed version instead. |
Sorry, something went wrong.
Instead of be limited just by the size of addressable memory (2**63 bytes), Python integers are now also limited by the number of bits, so the number of bit now always fits in 64-bit integer.
Both limits are much larger than what can be available on practice, so there is no effect on users.
_PyLong_NumBits() and _PyLong_Frexp() are now always successful.