classification
Title: Raise ValueError rather of OverflowError in PyLong_AsUnsignedLong()
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Oren Milman, rhettinger, serhiy.storchaka
Priority: normal Keywords:

Created on 2017-03-17 08:44 by serhiy.storchaka, last changed 2017-03-18 09:01 by serhiy.storchaka.

Messages (5)
msg289748 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-17 08:44
OverflowError is raised when Python integer doesn't fit in C integer type due to platform limitations. Different platforms have different limits. But in PyLong_AsUnsignedLong() only the upper limit is platform-depended. Negative integers always are not accepted. PyLong_AsUnsignedLong() is used for values that can be only non-negative. I think that ValueError is more appropriate in this case than OverflowError.
msg289752 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-03-17 09:46
note that there are functions that rely on the fact that
PyLong_AsUnsignedLong currently raises OverflowError for both cases.

such functions would probably use
PyErr_ExceptionMatches(PyExc_OverflowError)
or something like
PyErr_GivenExceptionMatches(err, PyExc_OverflowError)

_Py_Uid_Converter() (in Modules/posixmodule.c) is an example, but
ISTM there aren't many such functions.

however, this is only in cpython's C code. I don't know how many
functions in cpython's python code rely on this.

also, maybe there is a lot of user code that rely on this?
msg289796 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-03-18 08:04
Strong -1 on this.  For zero benefit, this breaks everything (within Python and third-party code) that ever relied on the documented behavior , https://docs.python.org/3/c-api/long.html#c.PyLong_AsUnsignedLong .

The cause perfectly fits the definition of an OverflowError:

class OverflowError(ArithmeticError)
 |  Result too large to be represented.

The time to challenge API design decisions is when they are created, not after they've been published and relied upon for over decade.

Also, we really don't everyone who writes cross-platform code and tests to have to catch both exceptions because they don't know which version is being run.  This creates yet another barrier to upgrading Python and as far as I can tell isn't solving any reported user problem.  Instead, it is second-guessing design decisions made long ago.
msg289797 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-03-18 08:10
Looking back in time, this API isn't as old as I thought.  The other concerns about breaking a published API still stand.
msg289798 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-18 09:01
_Py_Uid_Converter() in Modules/posixmodule.c is not an example because it calls PyLong_AsUnsignedLong() only for positive integers > LONG_MAX.

PyLong_AsUnsignedLong() is used not much. The motivation of this issue was that if use PyLong_AsUnsignedLong() for converting non-negative by its nature values ValueError can be more appropriate since this limitation is not platform or implementation dependent. Strictly speaking raising OverflowError for negative values doesn't fits the definition of an OverflowError, since the result is not large at all.

I was going to investigate all usages of PyLong_AsUnsignedLong() and if in majority of them ValueError is appropriate and desirable, changing the exception type at that level can make the implementation simpler.
History
Date User Action Args
2017-03-18 09:01:27serhiy.storchakasetmessages: + msg289798
2017-03-18 08:10:43rhettingersetnosy: - gvanrossum
messages: + msg289797
2017-03-18 08:04:19rhettingersetnosy: + rhettinger, gvanrossum
messages: + msg289796
2017-03-17 09:46:30Oren Milmansetmessages: + msg289752
2017-03-17 08:45:21serhiy.storchakalinkissue29833 dependencies
2017-03-17 08:44:59serhiy.storchakacreate