_PyLong_AsScaledDouble() writes the exponent in a int which have to be multiplied by PyLong_SHIFT to give the power of 2. I proposed to multiply the exponent by PyLong_SHIFT in _PyLong_AsScaledDouble() to directly get the power of 2. It avoids complex code to test integer overflow in code using _PyLong_AsScaledDouble() (the test is only done once, in _PyLong_AsScaledDouble). I also propose to change exponent type from "int*" to "unsigned int*". Previous maximum exponent was INT_MAX//PyLong_SHIFT (eg. 143165576 for PyLong using base 2^15). The new maximum is now UINT_MAX//PyLong_SHIFT, the double ;-) And since issue #4258 is commited (Use 30-bit digits instead of 15-bit digits for Python integers), PyLong_SHIFT value may be different depending on the compiler option. Using my patch, the long implement will be a little bit more hidden. The function _PyLong_AsScaledDouble() should be private because the name starts with "_". But I see it in Include/longobject.h. In Python, it's used in longobject.c and mathmodule.c.
Thanks for this. I'll take a look. How about making the exponent type size_t or Py_ssize_t, rather than [unsigned] int? It seems to me that that fits better with the usual size in digits.
Long double (80 bits) exponent is in range [-16382; 16383] and so would fits in an int, unsigned int, size_t or Py_ssize_t. I don't know if a signed or unsigned number is better. I know only one operation on exponents: a-b in PyLong_AsDouble.
> Long double (80 bits) exponent is in range [-16382; 16383] and so would > fits in an int, unsigned int, size_t or Py_ssize_t. Sure, but I don't think that's relevant to the point I was attempting to make: PyLong_AsScaledDouble returns the number of bits in the given PyLong, and that number can be (theoretically) as large as PY_SSIZE_T_MAX * PyLong_SHIFT. So Py_ssize_t is a better fit than int to hold this number: otherwise you'll get unnecessary overflows on a 64-bit system. I'm working on refactoring PyLong_AsScaledDouble and PyLong_AsDouble to have them both use the same core code. This would make PyLong_AsScaledDouble correctly rounded, and hence independent of the implementation of a PyLong (in particular, independent of the decision to use 15-bit or 30-bit digits for PyLongs: this decision really shouldn't affect the result of a log function or any other functions using _PyLong_AsScaledDouble).
Here's a patch: - rename _PyLong_AsScaledDouble to _PyLong_Frexp (for the benefit of anyone foolish enough to be using an undocumented private API function) - make the exponent type Py_ssize_t instead of int - do the scaling by PyLong_SHIFT in _PyLong_Frexp instead of in the functions that call it (e.g., loghelper in the math module) - remove longintrepr.h include from math module, since it's no longer needed - change _PyLong_Frexp algorithm to do correct rounding - refactor PyLong_Double to use _PyLong_Frexp.
