gh-123497: New limit for Python integers on 64-bit platforms #123498
Conversation
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.
serhiy-storchaka
requested review from
rhettinger and
isidentical
as code owners
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.
|
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. |
| @@ -3505,21 +3494,13 @@ _PyLong_Frexp(PyLongObject *a, int64_t *e) | |||
| /* Rescale; make correction if result is 1.0. */ | |||
| dx /= 4.0 * EXP2_DBL_MANT_DIG; | |||
| 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.
There was a problem hiding this comment.
Yes, but it can be equal.
| else { | ||
| return PyLong_FromLong(0); | ||
| } |
There was a problem hiding this comment.
else is superfluous.
| else { | |
| return PyLong_FromLong(0); | |
| } | |
| return PyLong_FromLong(0); |
... or:
| else { | |
| return PyLong_FromLong(0); | |
| } | |
| return _PyLong_GetZero(); |
There was a problem hiding this comment.
But this will lose symmetry. Note that this code is a duplicate of https://github.com/python/cpython/pull/123498/files#diff-1a6e70e2beeecad88840c67284ac4d54a36998029244771fcc820e801390726aR5334-R5339
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 |
|
Fixed building on Windows. MAX_LONG_DIGITS was incorrectly set on 32-bit platforms. #123724 is an alternative PR that uses 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). |
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.