Remove uses of nb_long slot, and rename to nb_reserved.

[mdickinson]

BPO	4910
Nosy	@mdickinson, @benjaminp
Files	issue4910_1.patch: Stage 1 of nb_long removal issue4910_1a.patch: Stage 1 nb_long removal (improved patch) issue4910_2.patch issue4910_3.patch issue4910_4.patch: deprecate PyNumber_Int

^{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/benjaminp'
closed_at = <Date 2009-02-11.17:07:06.097>
created_at = <Date 2009-01-10.20:32:22.374>
labels = ['type-bug']
title = 'Remove uses of nb_long slot, and rename to nb_reserved.'
updated_at = <Date 2009-02-11.17:07:06.096>
user = 'https://github.com/mdickinson'

bugs.python.org fields:

activity = <Date 2009-02-11.17:07:06.096>
actor = 'mark.dickinson'
assignee = 'benjamin.peterson'
closed = True
closed_date = <Date 2009-02-11.17:07:06.097>
closer = 'mark.dickinson'
components = []
creation = <Date 2009-01-10.20:32:22.374>
creator = 'mark.dickinson'
dependencies = []
files = ['12696', '12697', '12754', '12756', '12781']
hgrepos = []
issue_num = 4910
keywords = ['patch']
message_count = 13.0
messages = ['79571', '79581', '79655', '79659', '79689', '79902', '79907', '79913', '79918', '79973', '80031', '80055', '81652']
nosy_count = 2.0
nosy_names = ['mark.dickinson', 'benjamin.peterson']
pr_nums = []
priority = 'critical'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue4910'
versions = ['Python 3.0', 'Python 3.1']

[mdickinson]

In Python 3.x:

>>> int.__long__.__doc__
'x.__long__() <==> long(x)'

But the long builtin no longer exists.

I'm actually not sure why the nb_long slot still exists in 3.x at all.
Can anyone enlighten me?

[mdickinson]

long(x) replaced by int(x) in r68508.

I'm still wondering about the nb_long slot, though.

[mdickinson]

Here's a patch against the py3k branch that gets rid of the two existing
uses of nb_long in the core:

• in PyNumber_Long, conversion was attempted first using nb_int and
  then using nb_long. The patch simply removes the nb_long code, so
  int(x) no longer attempts to use the __long__ method for conversion.

• in Modules/_struct.c, there's a call to nb_long in a function that's
  attempting to turn an arbitrary PyObject into a PyLongObject; the
  patch replaces this with nb_int (and updates an error message).

• In Lib/test/test_long.py, __long__ has been replaced with __int__
  in a test that __int__/long takes precedence over __trunc__
  for conversion to int.

With this patch, all tests pass on my (OS X 10.5/Intel) machine.
(Except test_socket, but I'm 97.2% certain that's unrelated.)

If someone can review this quickly I'll move on to the next patch towards
removing nb_long. (My issue bpo-1717 experience suggests that it ought to be
easier to effect the removal via a series of 3 or 4 short, easy-to-review
patches with clear intent than via one bigger, more confused patch.)

I think it would be good if nb_long could be altered before 3.0.1.

Benjamin, do you have time to take a look?

[mdickinson]

That was a pretty poor patch. Here's a better one:

• added Misc/NEWS entry
• added tests to check that __long__ is never called
• removed Modules/_struct.c change, in the interests of
  keeping the patch focused.

[benjaminp]

The first installment looks good!

[mdickinson]

Thanks, Benjamin! Checked in in r68553, backported to 3.0 in r68556.

Here's the second patch, which fixes almost all remaining uses of nb_long
but stops short of actually removing/renaming the nb_long slot.

Notes:

(1) I haven't tested the change to PC/winreg.c

(2) The Modules/_struct.c change does introduce a change in behaviour:
for example, before the patch,

struct.pack('q', decimal.Decimal(1))

raises struct.error. After the patch, the packing succeeds. I *think*
the patched behaviour is probably the right behaviour, since it agrees
with 2.x, but it's not 100% clear to me what the intentions of the struct
module are with respect to integer packing of non-integer types. This is
probably a question for another issue.

[benjaminp]

On Thu, Jan 15, 2009 at 10:48 AM, Mark Dickinson <report@bugs.python.org> wrote:

Mark Dickinson <dickinsm@gmail.com> added the comment:

Thanks, Benjamin! Checked in in r68553, backported to 3.0 in r68556.

Here's the second patch, which fixes almost all remaining uses of nb_long
but stops short of actually removing/renaming the nb_long slot.

Notes:

(1) I haven't tested the change to PC/winreg.c

This looks correct. In fact, I don't really see the point of having
PyHKEY_unaryFailureFunc since a TypeError will automatically be raised
if the slot is NULL, but that is certainly another issue.

(2) The Modules/_struct.c change does introduce a change in behaviour:
for example, before the patch,

struct.pack('q', decimal.Decimal(1))

raises struct.error. After the patch, the packing succeeds. I *think*
the patched behaviour is probably the right behaviour, since it agrees
with 2.x, but it's not 100% clear to me what the intentions of the struct
module are with respect to integer packing of non-integer types. This is
probably a question for another issue.

Since Decimal implements __int__ and that's what the struct module is
converting with, I think this is fine.

Overall, the patch looks fine. I wonder if we should mark
PyNumber_Long as deprecated in the docs, though.

[mdickinson]

I wonder if we should mark
PyNumber_Long as deprecated in the docs, though.

It would be nice to standardize on one of (PyNumber_Long, PyNumber_Int)
and document the other as deprecated.

Maybe it would make more sense to stick with PyNumber_Long and deprecate
PyNumber_Int, though? It would make 2.x -> 3.x transitions easier, and
would be consistent with some of the rest of the API: we seem to be
using PyLong_Check in preference to PyInt_Check in current code...

[mdickinson]

Second patch applied in r68619 (py3k) and r68620 (3.0 maintenance branch).

Here's the third and final patch: rename the nb_long slot to nb_reserved,
and change its type to (void *).

[benjaminp]

+1 for applying the last patch.

[mdickinson]

Thanks again for reviewing, Benjamin. nb_long renamed in r68651, merged
to 3.0 (along with a few other long->int cleanups) in r68666.

There seems to be a consensus on deprecating PyNumber_Int. I'll leave
this open until that's taken care of.

[mdickinson]

Here's a patch that deprecates PyNumber_Int in favour of PyNumber_Long:

• move PyNumber_Int definition from abstract.h to intobject.h (and
  fix up comments at the top of intobject.h)
• mark PyNumber_Int as deprecated in the c-api docs, with a promise
  to remove it for 3.1.

I suppose that in theory this goes too far: we should really deprecate in
3.1 and remove in 3.2. But given all the other stuff that's going on for
3.0.1, this doesn't seem too bad. Benjamin, what do you think?

N.B. We should remember to actually remove intobject.h for 3.1. :)

[mdickinson]

PyNumber_Int deprecated in r69517, r69518.