classification
Title: long(4.2) now returns an int
Type: behavior Stage:
Components: None Versions: Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, barry-scott, benjamin.peterson, jyasskin, skip.montanaro
Priority: release blocker Keywords: patch

Created on 2008-09-04 20:49 by barry-scott, last changed 2008-09-09 15:45 by skip.montanaro. This issue is now closed.

Files
File name Uploaded Description Edit
float_long.patch amaury.forgeotdarc, 2008-09-06 21:22
Messages (13)
msg72519 - (view) Author: Barry Alan Scott (barry-scott) Date: 2008-09-04 20:49
I am testing PySVN against python2.6b3.

I see a failure when PyNumber_Long is called with a Float.
It raises TypeError.

The same code works on 2.3, 2.4 and 2.5.

Looking with GDB I see:

(gdb) bt
#0  PyNumber_Long (o=0x1809384) at Objects/abstract.c:1735
#1  0x020f8e70 in Py::Long::Long (this=0xbfffefc8, ob=@0xbfffefc0) at
pysvn_client_cmd_list.cpp:739

1681            m = o->ob_type->tp_as_number;
1682            if (m && m->nb_long) { /* This should include subclasses
of long */
1683                    /* Classic classes always take this branch. */
1684                    PyObject *res = m->nb_long(o);
1685                    if (res && (!PyInt_Check(res) &&
!PyLong_Check(res))) {

res does not contain the value that nb_long(o) calculated.
and the if on 1685 is false so you get a type error.

I have compiled on Mac OS X 10.4 powerpc and fedora 8 x86.
Both fail in the exact same way.

If you need to reproduce you will need to build pysvn and
run a command that triggers the problem. I don't have a
smaller example.
msg72529 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-04 21:56
Note to others: PySvn uses the PyCXX classes. The call in question is
something similar to
   Py::Long( Py::Float( double( someValue ) ) )

Barry, what is the exact error message that you get?
What do you mean by "res does not contain the value that nb_long(o)
calculated" ?
And what is the exact content of the "o" variable at the time of the
failure? (you could look for example at o->ob_type->tp_name)
msg72671 - (view) Author: Barry Alan Scott (barry-scott) Date: 2008-09-06 15:45
You are right that its the Py::Long( Py::Float( double( x ) ) )
that is triggering this problem.

Here is the gdb from the powerpc build that shows the
info you asked for and show res being corrupt.

I'm going to try and build a smaller version of this
problem using a minimal PyCXX module.

>>> import pysvn;pysvn.Client().ls('pysvn/__init__.py')

Breakpoint 1, bp () at pysvn_client_cmd_list.cpp:33
33      }
(gdb) c
Continuing.
Current language:  auto; currently c++

Breakpoint 2, pysvn_client::cmd_ls (this=0x1114830, a_args=@0xbffff118,
a_kws=@0xbffff120) at pysvn_client_cmd_list.cpp:138
138             Py::Long l_tmp( f_tmp );
(gdb) b PyNumber_Long
Breakpoint 5 at 0x213cfc: file Objects/abstract.c, line 1673.
(gdb) c
Continuing.

Breakpoint 5, PyNumber_Long (o=0x1809384) at Objects/abstract.c:1673
1673            if (trunc_name == NULL) {
(gdb) p o
$25 = (PyObject *) 0x1809384
Current language:  auto; currently c
(gdb) p *o
$26 = {
  ob_refcnt = 1, 
  ob_type = 0x35723c
}
(gdb) p *o->ob_type
$27 = {
  ob_refcnt = 4, 
  ob_type = 0x35f0bc, 
  ob_size = 0, 
  tp_name = 0x339d98 "float", 
  tp_basicsize = 16, 
  tp_itemsize = 0, 
  tp_dealloc = 0x23de30 <float_dealloc>, 
  tp_print = 0x23fa90 <float_print>, 
  tp_getattr = 0, 
  tp_setattr = 0, 
  tp_compare = 0, 
  tp_repr = 0x23fa50 <float_repr>, 
  tp_as_number = 0x357ab4, 
  tp_as_sequence = 0x0, 
  tp_as_mapping = 0x0, 
  tp_hash = 0x23e570 <float_hash>, 
  tp_call = 0, 
  tp_str = 0x23fa10 <float_str>, 
  tp_getattro = 0x263800 <PyObject_GenericGetAttr>, 
  tp_setattro = 0, 
  tp_as_buffer = 0x0, 
  tp_flags = 394747, 
  tp_doc = 0x357a4c "float(x) -> floating point number\n\nConvert a
string or number to a floating point number, if possible.", 
  tp_traverse = 0, 
  tp_clear = 0, 
  tp_richcompare = 0x23e000 <float_richcompare>, 
  tp_weaklistoffset = 0, 
  tp_iter = 0, 
  tp_iternext = 0, 
  tp_methods = 0x35733c, 
  tp_members = 0x0, 
  tp_getset = 0x357300, 
  tp_base = 0x0, 
  tp_dict = 0x0, 
  tp_descr_get = 0, 
  tp_descr_set = 0, 
  tp_dictoffset = 0, 
  tp_init = 0, 
  tp_alloc = 0, 
  tp_new = 0x241a10 <float_new>, 
  tp_free = 0, 
  tp_is_gc = 0, 
  tp_bases = 0x0, 
  tp_mro = 0x0, 
  tp_cache = 0x0, 
  tp_subclasses = 0x0, 
  tp_weaklist = 0x0, 
  tp_del = 0, 
  tp_version_tag = 0
}
(gdb) n
1679            if (o == NULL)
(gdb) 
1681            m = o->ob_type->tp_as_number;
(gdb) 
1682            if (m && m->nb_long) { /* This should include subclasses
of long */
(gdb) p *m
$28 = {
  nb_add = 0x242980 <float_add>, 
  nb_subtract = 0x242c40 <float_sub>, 
  nb_multiply = 0x242f00 <float_mul>, 
  nb_divide = 0x2431c0 <float_classic_div>, 
  nb_remainder = 0x243510 <float_rem>, 
  nb_divmod = 0x243830 <float_divmod>, 
  nb_power = 0x243bf0 <float_pow>, 
  nb_negative = 0x241d70 <float_neg>, 
  nb_positive = 0x241b60 <float_float>, 
  nb_absolute = 0x241e70 <float_abs>, 
  nb_nonzero = 0x23e580 <float_nonzero>, 
  nb_invert = 0, 
  nb_lshift = 0, 
  nb_rshift = 0, 
  nb_and = 0, 
  nb_xor = 0, 
  nb_or = 0, 
  nb_coerce = 0x241f70 <float_coerce>, 
  nb_int = 0x23e6e0 <float_trunc>, 
  nb_long = 0x23e6e0 <float_trunc>, 
  nb_float = 0x241b60 <float_float>, 
  nb_oct = 0, 
  nb_hex = 0, 
  nb_inplace_add = 0, 
  nb_inplace_subtract = 0, 
  nb_inplace_multiply = 0, 
  nb_inplace_divide = 0, 
  nb_inplace_remainder = 0, 
  nb_inplace_power = 0, 
  nb_inplace_lshift = 0, 
  nb_inplace_rshift = 0, 
  nb_inplace_and = 0, 
  nb_inplace_xor = 0, 
  nb_inplace_or = 0, 
  nb_floor_divide = 0x243b60 <float_floor_div>, 
  nb_true_divide = 0x244340 <float_div>, 
  nb_inplace_floor_divide = 0, 
  nb_inplace_true_divide = 0, 
  nb_index = 0
}
(gdb) n

Breakpoint 3, PyNumber_Long (o=0x1809384) at Objects/abstract.c:1684
1684                    PyObject *res = m->nb_long(o);
(gdb) s
float_trunc (v=0x1809384) at Objects/floatobject.c:1084
1084            double x = PyFloat_AsDouble(v);
(gdb) p o
$29 = (PyObject *) 0x1809384
(gdb) p v
$30 = (PyObject *) 0x1809384
(gdb) n
1087            (void)modf(x, &wholepart);
(gdb) p x
$31 = 0
(gdb) n
1100            if (LONG_MIN < wholepart && wholepart < LONG_MAX) {
(gdb) p wholepart
$32 = 4555
(gdb) n
1102                    return PyInt_FromLong(aslong);
(gdb) p asLong
No symbol "asLong" in current context.
(gdb) p aslong
No symbol "aslong" in current context.
(gdb) n
1105    }
(gdb) n
PyNumber_Long (o=0x1809384) at Objects/abstract.c:1685
1685                    if (res && (!PyInt_Check(res) &&
!PyLong_Check(res))) {
(gdb) p res
$33 = (PyObject *) 0x383cf0
(gdb) p *res
$34 = {
  ob_refcnt = 1362084, 
  ob_type = 0x140
}
(gdb) n

Breakpoint 4, PyNumber_Long (o=0x1809384) at Objects/abstract.c:1735
1735    }
(gdb)
msg72681 - (view) Author: Barry Alan Scott (barry-scott) Date: 2008-09-06 18:36
O.k. I know what is going on.

Here is the description from abstracts.h for PyNumber_Long:

     PyAPI_FUNC(PyObject *) PyNumber_Long(PyObject *o);

       /*
	 Returns the o converted to a long integer object on success,
	 or NULL on failure.  This is the equivalent of the Python
	 expression: long(o).

       */

Its says that I can expect a long integer. However PyNumber_long
can return an int or a long.

PyCXX checks for a long, but an int is not a long and I raise
a type error.

This is a contract break on the Python API.

The change that causes this break is in floatobject.c
From 2.5.2 code:

static PyNumberMethods float_as_number = {
...
	float_int, 	/*nb_int*/
	float_long, 	/*nb_long*/

From 2.6b3 code:

static PyNumberMethods float_as_number = {
...
	float_trunc, 	/*nb_int*/
	float_trunc, 	/*nb_long*/

float_trunc returns either an int or a long.
Which is not what is required for the API.

Here is the same bug at the pure python level.

 $ python2.6
Python 2.6b3 (r26b3:65922, Aug 25 2008, 15:44:46) 
[GCC 4.0.1 (Apple Computer, Inc. build 5370)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> long(4)
4L
>>> long(4.3)
4
>>> long("6")
6L
>>> type(long(4.3))
<type 'int'>
>>> 

Barry
msg72685 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-06 20:11
You are right: long(4.2) used to return a long.

This was changed by the introduction of the float_trunc() function,
which is now used for float.__trunc__, float.__int__ and float.__long__.

OTOH, long() has always been allowed to return an int. Checked with
python2.2:
>>> class C:
...   def __long__(self): return 4
...
>>> type(long(C()))
<type 'int'>

I suggest that:
- your code should be more tolerant, specially when calling API
functions from the "Abstract Objects Layer", accept both longs and ints.
- Concerning long(float()), the new behavior breaks existing code, and
should be reverted. I will try to come with a patch.
msg72702 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-06 21:22
Here is a patch that restores the float->long conversion. The function
was simply copied from 2.5.
msg72718 - (view) Author: Barry Alan Scott (barry-scott) Date: 2008-09-06 22:43
My concern and yours is that this is not backwards
compatible. I would hate to see "random" failures
of extensions written using PyCXX because of this.

I'm tempted to says that I'll keep PyCXX 5.x as is for
Python 2.x and leave all the changes in semantics
for PyCXX 6.0 that will support Python 3.0.
And in Python 3.0 this problem does not exist
by design.

I don't think you example proves anything.
Python does not check at the pure python level at all.

>>> class X:
...     def __long__( self ):
...             return "Hello"
... 
>>> long( X() )
'Hello'
>>> 

You get all you deserve if you define __long__ and break its
API.
msg72738 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-09-07 13:24
Jeffery, you made this change in r59671. Can you comment?
msg72798 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-09-08 22:23
In the meantime, Amaury, you patch is good.
msg72829 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-09 07:39
Applied as r66332
msg72860 - (view) Author: Skip Montanaro (skip.montanaro) * Date: 2008-09-09 13:57
Just a quick nit, but it seems to me that since 2.6 still actually
distinguishes between longs and ints that the two error messages in
PyLong_FromFloat should mention "long" instead of "integer".

Skip
msg72869 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-09 15:11
I suppose you meant PyLong_FromDouble()? I think the messages talk about
abstract numbers, not a specific python type: "infinity/NaN cannot be
converted to an integral value".
msg72874 - (view) Author: Skip Montanaro (skip.montanaro) * Date: 2008-09-09 15:45
Amaury> I suppose you meant PyLong_FromDouble()? 

Yes, sorry.

    Amaury> I think the messages talk about abstract numbers, not a specific
    Amaury> python type: "infinity/NaN cannot be converted to an integral
    Amaury> value".

Well, 2.5 called it "long".

Skip
History
Date User Action Args
2008-09-09 15:45:26skip.montanarosetmessages: + msg72874
2008-09-09 15:11:01amaury.forgeotdarcsetstatus: open -> closed
resolution: fixed
messages: + msg72869
2008-09-09 13:57:30skip.montanarosetnosy: + skip.montanaro
messages: + msg72860
2008-09-09 07:39:24amaury.forgeotdarcsetmessages: + msg72829
2008-09-08 22:23:12benjamin.petersonsetkeywords: - needs review
messages: + msg72798
2008-09-07 13:24:48benjamin.petersonsetnosy: + jyasskin, benjamin.peterson
messages: + msg72738
2008-09-06 22:43:08barry-scottsetmessages: + msg72718
2008-09-06 21:22:50amaury.forgeotdarcsetkeywords: + patch, needs review
files: + float_long.patch
messages: + msg72702
2008-09-06 20:11:42amaury.forgeotdarcsetpriority: release blocker
messages: + msg72685
title: PyNumber_Long fails from Float -> long(4.2) now returns an int
2008-09-06 18:36:03barry-scottsetmessages: + msg72681
2008-09-06 15:45:50barry-scottsetmessages: + msg72671
2008-09-04 21:56:04amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg72529
2008-09-04 20:49:16barry-scottcreate