classification
Title: Add a new PyLong_AsUnsignedLongAndOverflow function
Type: enhancement Stage: needs patch
Components: Interpreter Core Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, h.venev, mark.dickinson, vstinner
Priority: normal Keywords: patch

Created on 2014-03-31 13:26 by h.venev, last changed 2014-04-16 07:33 by vstinner.

Files
File name Uploaded Description Edit
patch.patch h.venev, 2014-04-05 13:29 Patch review
pylong_as_unsigned_long_and_overflow.patch mark.dickinson, 2014-04-14 20:48 review
Messages (13)
msg215236 - (view) Author: Hristo Venev (h.venev) * Date: 2014-03-31 13:26
It could set *overflow to -1 on negative value and to 1 on overflow.
And PyLong_AsUnsignedLongLongAndOverflow.
msg215297 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-04-01 07:19
Sounds reasonable to me.  Were there particular uses you needed this for?  And do you want to work on a patch?
msg215313 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-04-01 13:18
> Sounds reasonable to me.  Were there particular uses you needed this for?  And do you want to work on a patch?

I don't understand why PyArg_Parse*() functions have formats for integers checking overflow for signed integers, but not for unsigned integers. For the zlib module, I had to develop my own format format (to get an unsigned int with overflow check) :-(
msg215605 - (view) Author: Hristo Venev (h.venev) * Date: 2014-04-05 13:29
Please apply in 3.4.1. I need this ASAP.
msg215607 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-04-05 13:36
> Please apply in 3.4.1. I need this ASAP.

New functions are new features, and new features are no more added after a release (ex: 3.4.0). If something is changed, it will done in Python 3.5.

You can add PyLong_AsUnsigned*AndOverflow() functions to your project so you will support older versions too, like Python 3.3.
msg215614 - (view) Author: Hristo Venev (h.venev) * Date: 2014-04-05 14:06
Will you release 3.5 in the next few weeks?
msg215619 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2014-04-05 15:15
No
msg215651 - (view) Author: Hristo Venev (h.venev) * Date: 2014-04-06 07:20
I will not add PyLong_AsUnsigned*AndOverflow in my code because I don't want my code to depend on the exact implementation of PyLong.

Are you seriously calling a 50-line function "feature"?

Anyway... I propose splitting the patch in two parts:
   - "cleanup": the changes to Objects/longobject.c
   - "feature": the changes to Include/longobject.h

"cleanup" can be applied to 3.4.1 because it adds no new features and helps maintainability. Backwards compatibility will not be broken.
"feature" can then be added in 3.5. Backwards compatibility should not be broken.
msg215652 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-04-06 09:33
Thanks for the patch!  I left a couple of comments on Rietveld.  The patch will also need docs and tests before it's ready to go in.

There's a behaviour change in the patch which needs looking into: before the patch, PyLong_AsUnsignedLong will not call the target object's __int__ method (so it should raise a TypeError for a float or Decimal instance, for example).  It looks to me as though the patch changes that;  was that change intentional?
msg215659 - (view) Author: Hristo Venev (h.venev) * Date: 2014-04-06 15:16
I did not intend to make it so that PyLong_AsUnsignedLong* to call __int__ but it looks like a good idea because PyLong_AsLong* does that and the patch is exactly about that - removing differences between converting to signed and unsigned.

Should I upload the patch with docs and with the warning fixed? Will it be applied in 3.4.1? It will be a lot easier to check the value of an int than to create a new PyLong = 2^64 and compare the number with that.

P.S.: is there a very fast way to check if a PyLong fits in unsigned long long with the limited API?
P.P.S.: Just a random idea: would it be a to rewrite PyLong to use GMP instead as a PyVarObject of mp_limb_t's?
msg215660 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-04-06 15:33
> it looks like a good idea

It's not a good idea, though: you risk breaking existing third-party extension modules in surprising ways, which isn't a friendly thing to do.  In any case, if anything the PyLong methods should be moving *away* from use of the nb_int slot.  This has been discussed multiple times previously; one reason that we haven't made that change yet is the problem of breaking backwards compatibility.

> Should I upload the patch with docs and with the warning fixed?

Sure, that would be great if you have the time.  Tests would be useful too, but one of us can fill those in (eventually).  It all depends how much of a hurry you're in. :-)

> Will it be applied in 3.4.1?

No: as already explained by others, it's a enhancement, not a bugfix, so it can't be applied until 3.5.

> P.S.: is there a very fast way to check if a PyLong fits in unsigned long long with the limited API?

Depends on your definition of 'very fast'.  There should be no need to compare with a constant: just try the conversion with PyLong_AsUnsignedLong and see if you get an OverflowError back.  My guess is that that's not going to be much slower than a custom PyLong_AsUnsignedLongAndOverflow, but the only way you'll find out is by timing your particular use-case.

> P.P.S.: Just a random idea: would it be a to rewrite PyLong to use GMP instead as a PyVarObject of mp_limb_t's?

I'll let Victor answer that one. :-)  In the mean time, see issue 1814.
msg215682 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-04-07 09:29
> > P.P.S.: Just a random idea: would it be a to rewrite PyLong to use GMP instead as a PyVarObject of mp_limb_t's?
> I'll let Victor answer that one. :-)  In the mean time, see issue 1814.

During the development of Python 3.0, I wrote a large patch to reuse directly GMP for Python int. My conclusion is here:
http://bugs.python.org/issue1814#msg77018
(hint: "it's not a good idea")

IMO the first problem is the memory allocation. GMP type doesn't fit well with Python type. GMP type for "int" has a fixed size, and then GMP allocates a second structure for digits. It's inefficient for small integers, and almost all Python int are small (smaller than 32 or 64 bits).
msg216209 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-04-14 20:48
Here's an updated patch, for PyLong_AsUnsignedLongAndOverflow only, including docs and tests.
History
Date User Action Args
2014-04-16 07:33:52vstinnersettitle: PyLong_AsUnsignedLongAndOverflow does not exist -> Add a new PyLong_AsUnsignedLongAndOverflow function
2014-04-14 20:48:55mark.dickinsonsetfiles: + pylong_as_unsigned_long_and_overflow.patch
keywords: + patch
messages: + msg216209
2014-04-07 09:29:23vstinnersetmessages: + msg215682
2014-04-06 15:33:56mark.dickinsonsetmessages: + msg215660
2014-04-06 15:16:57h.venevsetmessages: + msg215659
2014-04-06 09:33:38mark.dickinsonsetmessages: + msg215652
2014-04-06 07:20:17h.venevsetmessages: + msg215651
2014-04-05 15:15:14benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg215619
2014-04-05 14:06:21h.venevsetmessages: + msg215614
2014-04-05 13:36:51vstinnersetmessages: + msg215607
versions: + Python 3.5, - Python 3.4
2014-04-05 13:29:13h.venevsetfiles: + patch.patch

messages: + msg215605
versions: + Python 3.4, - Python 3.5
2014-04-01 18:05:04mark.dickinsonsetstage: needs patch
type: enhancement
components: + Interpreter Core, - Extension Modules
versions: + Python 3.5, - Python 3.4
2014-04-01 13:18:11vstinnersetnosy: + vstinner
messages: + msg215313
2014-04-01 07:19:49mark.dickinsonsetmessages: + msg215297
2014-03-31 13:51:36mark.dickinsonsetnosy: + mark.dickinson
2014-03-31 13:26:36h.venevcreate