can't set big int-like objects to items in array 'Q', 'L' and 'I'

[orenmn]

BPO	28298
Nosy	@mdickinson, @meadori, @serhiy-storchaka, @orenmn
PRs	bpo-28298: make array 'Q', 'L' and 'I' accept big intables as elements #570 [3.6] bpo-28298: make array 'Q', 'L' and 'I' accept big intables as elements #579 [3.5] bpo-28298: make array 'Q', 'L' and 'I' accept big intables as elements #588 [Do Not Merge] Sample of CPython life with blurb. #703
Files	patchedCPythonTestOutput_ver1.txt: test output of CPython with my patches (tested on my PC) - ver1 CPythonTestOutput.txt: test output of CPython without my patches (tested on my PC) issue28298_ver1.diff: proposed patches diff file - ver1

^{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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-03-10.06:14:23.292>
created_at = <Date 2016-09-28.10:09:35.874>
labels = ['3.7', 'type-bug', 'library']
title = "can't set big int-like objects to items in array 'Q', 'L' and 'I'"
updated_at = <Date 2017-03-24.22:38:54.246>
user = 'https://github.com/orenmn'

bugs.python.org fields:

activity = <Date 2017-03-24.22:38:54.246>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-03-10.06:14:23.292>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-09-28.10:09:35.874>
creator = 'Oren Milman'
dependencies = []
files = ['44860', '44861', '44862']
hgrepos = []
issue_num = 28298
keywords = ['patch']
message_count = 13.0
messages = ['277604', '277685', '277704', '277707', '277729', '288866', '289218', '289234', '289336', '289341', '290243', '290251', '290252']
nosy_count = 4.0
nosy_names = ['mark.dickinson', 'meador.inge', 'serhiy.storchaka', 'Oren Milman']
pr_nums = ['570', '579', '588', '703']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28298'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[orenmn]

------------ current state ------------
On my Windows 10, on a 32-bit Python, The following runs fine:
import array
array.array('L').append(2 ** 32 - 1)

However, in the following, an OverflowError('Python int too large to convert to C long') is raised on the last line:
import array
class LikeInt:
def __init__(self, intVal):
self.intVal = intVal
def __int__(self):
return self.intVal
array.array('L').append(LikeInt(2 ** 32 - 1))

The reason for this behavior is the implementation of the function LL_setitem (in Modules/arraymodule.c) (edited brutally for brevity):
    LL_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
    {
        unsigned long x;
        if (PyLong_Check(v)) {
            x = PyLong_AsUnsignedLong(v);
        }
        else {
            long y;
            PyArg_Parse(v, "l;array item must be integer", &y);
            x = (unsigned long)y;
        }
        (ap->ob_item)[i] = x;
    }
The problem is that PyArg_Parse is used to convert a Python int into a C long. So PyArg_Parse fails when it is given a Python int which is in range(LONG_MAX + 1, ULONG_MAX + 1), even though such Python int can be stored in a C unsigned long.

It is quite the same for array('I') and array('Q') (i.e. in II_setitem and in QQ_setitem, respectively).

With regard to relevant changes made in the past, PyArg_Parse was always used (in case '!PyLong_Check(v)'), since adding the original versions of:
* II_setitem and LL_setitem (back then, they were called I_setitem and L_setitem, respectively), in changeset 4875 (https://hg.python.org/cpython/rev/911040e1bb11)
* QQ_setitem, in changeset 72430 (https://hg.python.org/cpython/rev/15659e0e2b2e)

------------ proposed changes ------------

    1. In Modules/arraymodule.c, change the implementation of LL_setitem (and likewise, of II_setitem and QQ_setitem) roughly to the following:
        LL_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
        {
            unsigned long x;
            if (!PyLong_Check(v)) {
                v = _PyLong_FromNbInt(v);
            }
            x = PyLong_AsUnsignedLong(v);
            (ap->ob_item)[i] = x;
        }

2. In [Lib/test/test_array.py](https://github.com/python/cpython/blob/main/Lib/test/test_array.py), add tests:
    * to verify the bug is fixed, i.e. test the bounds of setting int-like objects to items in an integers array
    * to verify float objects can't be set to items in an integers array (as is already the behavior, but there aren't any tests for it)

3. While we are in [Lib/test/test_array.py](https://github.com/python/cpython/blob/main/Lib/test/test_array.py), remove any checks whether 'long long' is available, as it must be available, since changeset 103105 (https://hg.python.org/cpython/rev/9206a86f7321).

Note that issue bpo-12974 (opened in 2011) proposes deprecating the ability to set int-like objects to items in an integers array. (I am not sure how that affects my patch, but I guess it should be noted.)

------------ diff ------------
The proposed patches diff file is attached.

(Note that the I didn't propose to change the perplexing error message 'unsigned int is greater than maximum' in II_setitem, as there are many such messages in the codebase, and I am working on a patch for them as part of issue bpo-15988.)

------------ tests ------------
I ran 'python_d.exe -m test -j3' (on my 64-bit Windows 10) with and without the patches, and got quite the same output. (That also means my new tests in test_array passed.)
The outputs of both runs are attached.

[mdickinson]

One comment here: it's not the presence of __int__ that makes a type integer-like; it's the presence of __index__. (Decimal and float both supply __int__, but shouldn't be regarded as integer-like, for example.) I'm guessing that we're going to have the same issues with __index__ in place of __int__, though.

[orenmn]

You are right about the terminology of course. My bad.

Anyway, the bug is not related to __index__ (or nb_index), because the three aforementioned functions are using (in case '!PyLong_Check(v)') PyArg_Parse with the formats 'l' or 'L'.
Ultimately, convertsimple (in Python/getargs.c) would call PyLong_AsLong or PyLong_AsLongLong (respectively).
These two (in case '!PyLong_Check(v)') only try to use nb_int, as documented:
* https://docs.python.org/3.7/c-api/long.html?highlight=pylong_aslong#c.PyLong_AsLong
* https://docs.python.org/3.7/c-api/long.html?highlight=pylong_aslong#c.PyLong_AsLongLong

But just in case, I ran the following:
import array
class LikeInt:
def __init__(self, intVal):
self.intVal = intVal
def __index__(self):
return self.intVal
array.array('L').append(LikeInt(0))
and got a 'TypeError: an integer is required (got type LikeInt)'

[mdickinson]

and got a 'TypeError: an integer is required (got type LikeInt)'

Right, see bpo-12974 for more on arrays, __int__ and __index__.

[orenmn]

Ah, I should have read more about __int__ and __index__ before writing my last reply.
So IIUC, this is a somewhat painful issue, which could be resolved by:
* bpo-20092
* replacing _PyLong_FromNbInt with PyNumber_Index in:
- Objects/longobject.c (as you suggested in https://bugs.python.org/issue12965#msg146252)
- Modules/zlibmodule.c in ssize_t_converter
I didn't find any issue for the second one. If there is really no such issue, why is that? (I am sorry if I am the thousandth one to ask)
The first paragraph of your comment in https://bugs.python.org/issue21111#msg215660 suggests it might break a lot of code in the wild, but shouldn't we start with raising a deprecation warning?

With regard to my patch - should I make the following changes?
1. in Modules/arraymodule.c - add a call to PyNumber_Index before the call to _PyLong_FromNbInt (and move them into a helper function, as done in Modules/_struct.c)
2. in Lib/test/test_array.py:
* replace the name 'LikeInt' with 'IntCompatible' (I am not sure this is an appropriate name either. If you have a better name in mind, please share :) )
* add tests for LikeInt (this time, a class with a __index__ method)
3. In Doc/library/array.rst:
* add a note saying that setting int-like objects (i.e. objects with __index__) to items in an integers array is possible
* while we are here, remove the note about the availability of 'q' and 'Q' only in certain platforms

[orenmn]

ping

[serhiy-storchaka]

LGTM. But maybe combine PyFloat_Check+_PyLong_FromNbInt in one helper function?

Could you please create a PR Oren?

[orenmn]

yes and yes.
I would start with a PR for 3.7.

and thanks for the review :)

[serhiy-storchaka]

Thank you for your contribution Oren.

[orenmn]

Thanks for the reviews :)