GeezerVison is showing github.com — Raise ValueError rather of OverflowError in PyLong_AsUnsignedLong() · Issue #74020 · python/cpython
Raise ValueError rather of OverflowError in PyLong_AsUnsignedLong() #74020Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2017-03-17.08:44:59.699>
labels = ['interpreter-core', 'type-feature', '3.7']
title = 'Raise ValueError rather of OverflowError in PyLong_AsUnsignedLong()'
updated_at = <Date 2017-03-18.09:01:27.125>
user = 'https://github.com/serhiy-storchaka' bugs.python.org fields:
activity = <Date 2017-03-18.09:01:27.125>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2017-03-17.08:44:59.699>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 29834
keywords = []
message_count = 5.0
messages = ['289748', '289752', '289796', '289797', '289798']
nosy_count = 3.0
nosy_names = ['rhettinger', 'serhiy.storchaka', 'Oren Milman']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue29834'
versions = ['Python 3.7'] Linked PRs
Activityserhiy-storchaka commented on Mar 17, 2017 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. orenmn commented on Mar 17, 2017 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? rhettinger commented on Mar 18, 2017 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. rhettinger commented on Mar 18, 2017 Looking back in time, this API isn't as old as I thought. The other concerns about breaking a published API still stand. serhiy-storchaka commented on Mar 18, 2017 _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. Reacted by Erlend E. Aasland serhiy-storchaka commented on Aug 29, 2023 Argument Clinic converters unsigned_char(bitwise=False), unsigned_short(bitwise=False), unsigned_int(bitwise=False), unsigned_long(bitwise=False), unsigned_long_long(bitwise=False), and size_t raise ValueError for negative integer. It is impossible to implement them using the limited C API.
Many code converts Python integer to signed C type and then raise ValueError if it is negative (see for example handling maxlen in deque()). So, negative value can cause either OverflowError or ValueError, depending on platform. Reacted by Erlend E. Aasland serhiy-storchaka commented on Jun 27, 2024 New C API PyLong_AsNativeBytes() raises ValueError if the input is negative and the Py_ASNATIVEBYTES_REJECT_NEGATIVE flag was set. As well as the derived C API PyLong_ToUInt64() etc. serhiy-storchaka commented on Apr 28, 2025 See also #132987. Since many helper functions raise ValueError instead of OverflowError for negative values, adding support for the special __index__() method often results in this change. The further along, the more such changes will occur, and many of them will remain unnoticed if there were no specific tests. skirpichev commented on Apr 29, 2025 I'm with @rhettinger here that changing this will be a huge compatibility break. I suggest to do this with a deprecation period.
But the ValueError seems to be more acceptable here and allows more fine-tuned error reporting.
New C API PyLong_AsNativeBytes() raises ValueError if the input is negative and the Py_ASNATIVEBYTES_REJECT_NEGATIVE flag was set. As well as the derived C API PyLong_ToUInt64() etc.
On another hand, changing new API (introduced in 3.13 or even in 3.14) should be considered too. The OverflowError isn't too bad in this context. serhiy-storchaka commented on Apr 29, 2025
I suggest to do this with a deprecation period.
What is is a meaning of it, technically? skirpichev commented on Apr 29, 2025 We will warn people first, instead of just breaking code. I.e. now people are not expect ValueError from int_to_bytes() (with correct arguments). serhiy-storchaka commented on Apr 29, 2025 skirpichev commented on Apr 29, 2025 Emit a warning together with raising an OverflowError error. Then - after a deprecation period - change error type. serhiy-storchaka commented on Apr 30, 2025 That would create more problems. For example, look in Modules/_struct.c. The code replaces OverflowError (and ValueError) with struct.error. Now the user will get a warning about OverflowError/ValueError with raising struct.error -- instead of completely invisible change. picnixz commented on Apr 30, 2025 Maybe have an exception class inheriting from ValueError and OverflowError? skirpichev commented on Apr 30, 2025
That would create more problems. For example, look in Modules/_struct.c. The code replaces OverflowError (and ValueError) with struct.error. Now the user will get a warning about OverflowError/ValueError with raising struct.error -- instead of completely invisible change.
Yes, I see. The problem is that C-API has no functions to filter out warnings. Maybe it should?
Maybe have an exception class inheriting from ValueError and OverflowError?
Any suggestion on naming? picnixz commented on Apr 30, 2025 I'm tempted of having a DomainError but I fear it'll clash with existing ones (the name is likely not that rare but I may be mistaken). Also it wouldn't solve the issue when someone handles separately ValueError and OverflowError Reacted by Erlend E. Aasland erlend-aasland commented on May 11, 2025 There's also the possibility of just deprecating OverflowError in What's New/NEWS, and then changing it after x years.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|