[Python-3000] PyLong_Check() behaviour clarification (test_getargs2 failing on Windows x64)

[Neal Norwitz]

On Thu, Mar 20, 2008 at 2:13 AM, Trent Nelson <tnelson at onresolve.com> wrote:
> test_getargs2 is failing on Windows x64:
>  test test_getargs2 failed -- Traceback (most recent call last):
>   File "S:\buildbots\python.x64\3.0.nelson-win64\build\lib\test\test_getargs2.py", line 190, in test_n
>     self.failUnlessEqual(99, getargs_n(Long()))
>  TypeError: 'Long' object cannot be interpreted as an integer
>
>  Drilling down into the source, this code arrives at Python/getargs.c:convertsimple().  On Windows x64, SIZEOF_SIZE_T != SIZEOF_LONG, so there's a case 'n' statement that is defined that does the following:
>
>     iobj = PyNumber_Index(arg);
>
>  I'm a little confused.  At this point, arg is a PyObject *, and arg->obj_type->tp_as_number->nb_int has a non-zero value.  However, PyNumber_Index(arg) performs two checks, and this object fails both, which results in the TypeError being raised.

PyNumber_Index() works on an object that is an int or long or has an
__index__ method.  The "Long" class has only an __int__ method.  It is
otherwise a regular user-defined class that has no super class.

>  The first check is PyLong_Check(arg), which is handled via PyType_FastSubclass, which ends up doing the following:
>
>     ((arg->obj_type->tp_flags & Py_TPFLAGS_LONG_SUBCLASS) != 0)
>
>  tp_flags is 284160 at this point.  Py_TPFLAGS_LONG_SUBCLASS is 16777216 and Py_TPFLAGS_INT_SUBCLASS is 8388608, so this check fails.  Why doesn't tp_flags have either one of these values here?  obj_type->nb_int has a value, so shouldn't flags reflect the type?

Despite the class name of "Long", the object doesn't subclass int or
long.  That's why the tp_flags checks are failing.

nb_int has a value because of the __int__ method defined on Long.

>  I've just checked a 32-bit build and PyNumber_Index for a PyObject * representing a literal 99 also exhibits the same behaviour as above.
>
>  Also, if we're a 32-bit build, the 'n' code path visits PyLong_AsLong(), which calls PyLong_AsLongAndOverflow() -- the test cases calling getargs_n() test for the overflow being raised correctly.  Notionally, on x64 Windows where ((size_t == long long) > long), shouldn't we have a PyLong_AsLongLongAndOverflow() method?

The APIs seem very confused.  I'm not sure that is the best way to go
either.  Conceptually from what the test is trying to do, we need to
convert from a user-defined type (Long) to an PyObject (Python long or
PyLongObject) and then convert that to a Py_ssize_t.  I'm not sure the
test makes the most sense though.  There should be a test added that
uses a value between 2**32 and 2**64 to ensure we cover the win64
case.

I think the best thing to do for now (we need to clean up all the APIs
and have them make sense) is change the code that handles 'n' as
follows:

        {
                PyObject *iobj;
                Py_ssize_t *p = va_arg(*p_va, Py_ssize_t *);
                Py_ssize_t ival = -1;
                if (float_argument_error(arg))
                        return converterr("integer<n>", arg, msgbuf, bufsize);
                iobj = PyNumber_Index(arg);
                /* Code added here to handle __int__ methods on
classes which don't subclass int. */
                if (iobj == NULL) {
                        PyErr_Clear();
                        /* Using PyNumber_Long() here is probably too
permissive.  It converts strings and objects with __trunc__. Probably
we should just check the nb_int (or is it nb_long in 3k?) slot
specifically and call that? */
                        iobj = PyNumber_Long(arg);
                }
                if (iobj != NULL)
                        ival = PyLong_AsSsize_t(arg);
                if (ival == -1 && PyErr_Occurred())
                        return converterr("integer<n>", arg, msgbuf, bufsize);
                *p = ival;
                break;
        }

I'm not sure if we are abusing the 'n' format here or not.  The doc
(Doc/c-api/arg.rst) just says it turns the arg into a Py_ssize_t.  I
don't know if __index__ should really be (ab)used or not.  PEP 357
added __index__.

n