Issue 1533: Bug in range() function for large values (original) (raw)
Created on 2007-12-01 00:28 by robertwb, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Messages (57)
Author: Robert Bradshaw (robertwb)
Date: 2007-12-01 00:28
Range accepts arguments coerce-able into ints via int, but rejects arguments coerce-able into longs but to large to fit into an int.
The problem is in handle_range_longs in bltinmodule.c:1527-1541. If they type is not an int or long, it should try to make it so before failing (for consistency with smaller values at least).
Attached is a file that reproduces this bug.
Author: John Smith (josm)
Date: 2007-12-01 01:37
Is this a bug? print range(MyInt(264), MyInt(264+10)) produced TypeError: range() integer start argument expected, got instance.
print range(int(MyInt(264)), int(MyInt(264+10))) should work.
Author: Robert Bradshaw (robertwb)
Date: 2007-12-01 01:42
Yes, that is a workaround, but
range(MyInt(n), MyInt(n+10))
should work for any valid value of n, not just some of them.
Author: Rafael Zanella (zanella)
Date: 2008-02-21 11:16
FWIW, using xrange() it seems to give the proper error message:
Traceback (most recent call last): File "bad_range.py", line 12, in print xrange(MyInt(264), MyInt(264+10)) OverflowError: long int too large to convert to int
Author: Robert Bradshaw (robertwb)
Date: 2008-02-21 17:02
Yes, the error for xrange is more illustrative of the problem, but just shows that xrange has this a too. Why should xrange be invalid for non-word sized values (especially as range works)? Incidentally, just a week ago I had to write my own iterator for a project because xrange couldn't handle large values.
Author: Rafael Zanella (zanella)
Date: 2008-02-24 13:19
According to the documentation (http://docs.python.org/dev/library/functions.html) "The arguments must be plain integers", so I think the wrong thing here is to run the object's int() under the range()'s hood. I think the right thing to do would be to explicitly invoke int() on passing an non-int argument as parameter.
Author: Alexander Belopolsky (belopolsky) * 
Date: 2008-02-24 21:03
This is related to and .
contains a simple patch that extend acceptable range of argument to +/-2**63
contains a complete long integer support implementation and
was accepted in Py3k.
It looks like all three issues can be closed by either accepting or rejecting patch for 2.6 and marking patch as accepted for Py3k.
Author: Christian Heimes (christian.heimes) *
Date: 2008-02-24 21:07
I'm -10 on the patch in ( +/-2**63).
Reason: It's a good thing that the range of "range" is limited since it returns a list of integers. range(232) allocates (232)*16 bytes + small overhead for ints plus the space for the list (probably (2**32)*sizeof(ptr) which is 4 or 8 bytes). So far the memory for the ints is never returned to the system. I'm working on the problem.
Author: Alexander Belopolsky (belopolsky) *
Date: 2008-02-24 21:44
Christian,
I was probably a bit sloppy using "range" instead of "xrange," but is limited to xrange only. Are you still -10 on extending xrange on 64-bit platforms to +/- 2**63? If so, what is your position on backporting py3k's unlimited range implementation?
Author: Alexander Belopolsky (belopolsky) *
Date: 2008-02-24 22:00
So far the memory for the ints is never returned to the system. I'm working on the problem.
Christian,
Are you working on the memory problem or on this issue? I think I have a solution to OP's problem, but don't want to duplicate your effort.
Author: Christian Heimes (christian.heimes) *
Date: 2008-02-24 22:14
I'm working on the memory problem. See #2039 and #2013.
xrange is a totally different story. I'm +0 on changing xrange. Is Python 3.0's range function compatible with xrange? If the answer is yes, we may reuse the code for an unlimited xrange.
Author: Alexander Belopolsky (belopolsky) *
Date: 2008-02-24 23:08
Attached patch addresses OP's issue:
$ ./python.exe bad_range.py [8, 9, 10, 11, 12, 13, 14, 15, 16, 17] here [18446744073709551616L, 18446744073709551617L, 18446744073709551618L, 18446744073709551619L, 18446744073709551620L, 18446744073709551621L, 18446744073709551622L, 18446744073709551623L, 18446744073709551624L, 18446744073709551625L] [18446744073709551616L, 18446744073709551617L, 18446744073709551618L, 18446744073709551619L, 18446744073709551620L, 18446744073709551621L, 18446744073709551622L, 18446744073709551623L, 18446744073709551624L, 18446744073709551625L]
The only existing test that fails is range(1e100, 1e101, 1e101) producing a TypeError. It will now produce
range(1e100, 1e101, 1e101) main:1: DeprecationWarning: integer argument expected, got float [10000000000000000159028911097599180468360808563945281389781327557747838 772170381060813469985856815104L]
Note that range(1e100, 1e101) would still fail:
range(1e100, 1e101) Traceback (most recent call last): File "", line 1, in OverflowError: range() result has too many items
An alternative solution would be to disallow non-ints regardless of their value, but that is more likely to break someone's code.
Author: Robert Bradshaw (robertwb)
Date: 2008-02-25 17:52
Alexander Belopolsky's patch looks like the right fix for range() to me.
The xrange limits still hold, but that should probably be a separate issue/ticket.
Author: Akira Kitada (akitada) *
Date: 2008-11-29 20:28
I'm just curious to know which is the right behavior.
Python 3.0
Traceback (most recent call last): File "bad_range.py", line 7, in print(range(MyInt(23), MyInt(23+10))) TypeError: 'MyInt' object cannot be interpreted as an integer
Python 2.7a0
[8, 9, 10, 11, 12, 13, 14, 15, 16, 17] here [18446744073709551616L, 18446744073709551617L, 18446744073709551618L, 18446744073709551619L, 18446744073709551620L, 18446744073709551621L, 18446744073709551622L, 18446744073709551623L, 18446744073709551624L, 18446744073709551625L] Traceback (most recent call last): File "bad_range.py", line 11, in print(range(MyInt(264), MyInt(264+10))) TypeError: range() integer start argument expected, got instance.
Author: Robert Bradshaw (robertwb)
Date: 2008-11-29 20:45
I think both behaviors are wrong, the 3.0 one is backwards incompatible, and the 2.7 one is inconsistent (accepting MyInt if it's < 32 bits, rejecting it for > 64 bits).
For our particular use case, it is very annoying to not be able to use non-ints. It goes against the principle duck typing by simply defining the int and index methods.
Author: Akira Kitada (akitada) *
Date: 2008-11-29 22:53
Updating versions.
Author: Martin v. Löwis (loewis) *
Date: 2008-12-10 08:52
This change is out of scope for 2.5.3 (plus, the desired behavior still seems to be debated)
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 00:34
As far as I can tell there's no bug in 3.x: the 3.x range happily accepts an instance of a class that defines index.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 00:53
Currently, in trunk, I get:
range(0.0, 11.0, 1.1) Traceback (most recent call last): File "", line 1, in TypeError: range() integer start argument expected, got float.
But with Alexander's patch on trunk, I get:
range(0.0, 11.0, 1.1) [0L, 1L, 2L, 3L, 4L, 5L, 6L, 7L, 8L, 9L, 10L]
I'm not sure whether this is intentional or not, but IIRC it was a very deliberate choice not to allow float arguments to range (especially when, as here, the arguments are simply being truncated). I don't think this is an acceptable change for 2.7 (still less for 2.6).
Any patch for this issue should not change the behaviour for small arguments.
IMO, the right solution is to convert arguments via index when possible (as 3.x appears to already do). However, this would be a new feature. I suggest closing this as a 'won't fix'.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 01:05
IIRC, it was a very deliberate choice not to allow float arguments to range
Ignore this bit. IDRC. It was a deliberate choice not to let something range(0.0, 1.0, 0.1) work to give [0.0, 0.1, ...], since the usual floating-point difficulties give uncertainty about the endpoint.
That float arguments were allowed (and silently truncated to integers) is merely unfortunate. :) And it's no longer permitted in 2.7; I wouldn't want to go back to permitting float arguments here.
I'll set this to pending; it should be closed unless someone comes up with a simple fix in the near future.
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-01 02:01
I agree that this issue should be closed with no further action, but for historical accuracy the resolution should be "out of date" rather than "won't fix". The original bug was about range() behavior when it get arguments that are not ints, but "coerce-able into ints via int". Since range() no longer accepts such arguments, the issue is moot and there is nothing to fix or not fix here.
As a pie in the sky idea, I always wanted a range function that would work on any arguments that support addition and ordering. For example range(date(2010,1,1), date(2010, 2, 1), timedelta(7)) to return all Fridays in January, 2010. However, since advent of generator functions, this became simply
def range(start, stop, step): while start < stop: yield start start += step
and thus unnecessary for stdlib.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 07:50
Alexander: range does still accept such arguments (in 2.7); just not floats:
from decimal import Decimal range(Decimal(20), Decimal(20)) [] range(Decimal('1e100'), Decimal('1e100')) Traceback (most recent call last): File "", line 1, in TypeError: range() integer start argument expected, got Decimal.
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-01 14:48
On Sat, May 1, 2010 at 3:50 AM, Mark Dickinson <report@bugs.python.org> wrote:
Mark Dickinson <dickinsm@gmail.com> added the comment:
Alexander: range does still accept such arguments (in 2.7); just not floats:
from decimal import Decimal range(Decimal(20), Decimal(20)) []
Decimal must be a special case. With the code attached by OP and trunk:80673, I get
$ ./python.exe bad_range.py ... Traceback (most recent call last): File "bad_range.py", line 12, in print range(MyInt(264), MyInt(264+10)) TypeError: range() integer start argument expected, got instance.
Same with new style MyInt:
$ ./python.exe bad_range1.py ... Traceback (most recent call last): File "bad_range1.py", line 12, in print range(MyInt(264), MyInt(264+10)) TypeError: range() integer start argument expected, got MyInt.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 14:59
Decimal is behaving in exactly the same way as MyInt, isn't it? What do you get for range(MyInt(20), MyInt(20))?
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-01 15:35
On Sat, May 1, 2010 at 10:59 AM, Mark Dickinson <report@bugs.python.org> wrote:
Mark Dickinson <dickinsm@gmail.com> added the comment:
Decimal is behaving in exactly the same way as MyInt, isn't it? What do you get for range(MyInt(20), MyInt(20))?
Hmm, maybe there is a 2.7 bug here after all:
[20, 21, 22]
range(MyInt(264), MyInt(264+3)) Traceback (most recent call last): File "", line 1, in TypeError: range() integer start argument expected, got instance.
Same with Decimal:
Traceback (most recent call last): File "", line 1, in TypeError: range() integer start argument expected, got Decimal.
Author: Robert Bradshaw (robertwb)
Date: 2010-05-01 19:02
Thank you Alexander. Yes, there is still an issue for large operands, and the attached patch does fix it. Floats are explicitly checked for and rejected by PyArg_ParseTuple for the "l" format (as called by builtin_range) so to preserve this behavior we can explicitly check in the argument parsing of handle_range_longs as well.
This all goes away in Py3 due to the unification of int and long. (And I agree that using index rather than int fits better there).
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-01 19:20
Robert,
Your patch segfaults on Lib/test/test_builtin.py, but I agree with the approach. I'll see if it is easy to fix.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 19:31
Hmm, maybe there is a 2.7 bug here after all:
Yes, indeed! Which is why I was suggesting 'wont fix' rather than 'out of date' :)
The bltinmodule2.diff patch from Robert causes a segfault in test_builtin on my system; I haven't looked at the patch itself properly, but there's likely some refcounting problem.
The patch also lacks tests.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 20:03
A couple of suggestions regarding the patch:
(1) I'd suggest leaving the first part of handle_range_longs intact, up to and including the "/* ilow and ihigh correct now; do istep */" block. Then build out the three "if (!PyInt_Check(...)) ..." blocks below to include argument conversion. I think the patch would look cleaner this way.
(2) Rather than using PyNumber_Long, I'd prefer an explicit check for, and call to, nb_int. This is the behaviour that's used for the 'l' getargs format. PyNumber_Long is considerably more complicated, and involves looking at trunc and long; so if you use PyNumber_Long you'll still end up with inconsistent behaviour between small and large arguments.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-01 20:12
I am attaching a patch which does not crash or leak (as tested with -R :) and adds a unit test for OP's case.
The problem, however is that it does not work if new style classes are used in the test case. See disabled (with if 0) test in the patch.
I'll give it some more thought.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-01 20:15
Mark,
I did not see your last comment before uploading the last patch. I think your suggestion to bypass PyNumber_Long will fix the new/old style classes discrepancy.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-01 20:21
Alexander, I think it should be as simple as replacing the if (!PyInt_Check(ilow) && ...) block with something like this:
if (!PyInt_Check(ilow) && !PyLong_Check(ilow)) {
PyNumberMethods *nb = Py_TYPE(ilow)->tp_as_number;
PyObject *temp;
if (PyFloat_Check(ilow) || nb == NULL || nb->nb_int == NULL) {
PyErr_Format(PyExc_TypeError,
"range() integer start argument expected, got %s.",
ilow->ob_type->tp_name);
goto Fail;
}
temp = (*nb->nb_int)(ilow);
Py_DECREF(ilow);
ilow = temp;
if (temp == NULL)
goto Fail;
}and then doing the same for the ihigh and istep blocks. But I haven't tested this.
Mark
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-02 00:03
This patch, bltinmodule4.diff, seems to work for all cases.
Mark,
It looks like I came up with almost the same logic as you did. The only difference that I can see is that your code does not check that nb_int returns an integer. Also I put repeated logic into a separate function.
The patch includes unit tests that pass reference leak test.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-02 08:37
Thanks---the new patch looks good. Pulling the argument conversion out into a separate function makes the whole thing much cleaner.
I still have a couple of nits:
Please add a comment before get_range_argument indicating what it's for. I'd also consider naming the function something more descriptive like 'convert_range_argument' rather than 'get_range_argument', but I've never been good with names...
Good catch about checking the return type of nb_int. The error message should refer to "int" though, not "nb_int": "nb_int" won't make much sense to most Python users.
I notice that get_range_argument steals a reference to arg. That's fine of course, but it's a little bit unusual, so there should be a comment pointing that out somewhere. It might be preferable to not steal the reference, and just do the usual 'return a new reference' thing instead; I know that leads to a little bit more boilerplate in handle_range_longs, but I think the code ends up clearer, and there won't be surprises for someone who tries to reuse the code in get_range_argument for their own conversions. I leave it up to you. :)
Please could you also add a test for small arguments for each test class?
Style nit: the codebase mostly avoids assignments in 'if' conditions; please separate the assignment and the NULL test in lines like "if (!(ilow = get_range_argument(ilow, "start")))". Also, "if (ilow == NULL)" is preferable to "if (!ilow)".
Thanks again for doing this.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-02 09:06
Thinking about it a bit more, I really would prefer get_range_argument not to steal a reference. If I'm reading a bit of C code and encounter something like:
obj = transform(obj); if (obj == NULL) ...
my hindbrain immediately starts fretting that something's wrong, and I have to go and ferret out the definition of 'transform' to be sure. In contrast, patterns like:
temp = transform(obj); Py_DECREF(obj); obj = temp; if (obj == NULL) ...
are so common and familiar in the Python codebase that they don't raise the same sort of red flag.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-02 09:44
BTW, I've changed the various "nb_int should return int object" error messages in Objects/intobject.c to the more meaningful and accurate message: "int method should return an integer", in r80695.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-03 03:43
Attached patch, .diff, simplifies reference acrobatics at the expense of extra local variables. For the first time the first compilation did not have reference counting errors!
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-03 03:46
On Sun, May 2, 2010 at 4:37 AM, Mark Dickinson <report@bugs.python.org> wrote: ..
- Please could you also add a test for small arguments for each test class?
Working on it ...
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-03 03:52
- Please could you also add a test for small arguments for each test class?
Done.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-04 11:27
Thanks for the fixes. The latest patch looks good to me.
Alexander, is it okay for me to commit this?
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-04 12:30
On May 4, 2010, at 7:27 AM, Mark Dickinson <report@bugs.python.org>
wrote:
Mark Dickinson <dickinsm@gmail.com> added the comment:
Thanks for the fixes. The latest patch looks good to me.
Alexander, is it okay for me to commit this?
Sure. Why do you have to ask?
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-04 12:33
Hi Alexander,
I took the liberty of messing with your patch slightly; I didn't want to ask you to make further changes since the patch was fine, and my messing was mostly just to satisfy my own fussiness (only the first two items were really necessary):
Minor updates:
- add Misc/NEWS entry
- remove trailing whitespace and fix spaces that should have been a tab
- added some extra tests to check all possible combinations of bad arguments, purely to check for refcounting problems
- initialize low to NULL, to match the Py_XDECREF(low) (could change that Py_XDECREF to Py_DECREF instead, but the code's more resistant to random refactorings this way --- see next item :)
- randomly refactor: regroup blocks for ease of reading
- don't do PyLong_FromLong(1) until it's needed ('zero' is different, since it's always used in the non-error case)
- [micro-optimization]: don't pass a known zero value to get_range_long_argument
Any objections or comments? Can I apply this version of the patch?
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-04 12:41
I see. Let me take a look.
BTW, I tried to use tabs for indentation in the patch, but emacs was
not always happy about it. Is there some c-mode setting that I don't
know about that would take care of that?
On May 4, 2010, at 8:34 AM, Mark Dickinson <report@bugs.python.org>
wrote:
Mark Dickinson <dickinsm@gmail.com> added the comment:
Hi Alexander,
I took the liberty of messing with your patch slightly; I didn't
want to ask you to make further changes since the patch was fine,
and my messing was mostly just to satisfy my own fussiness (only the
first two items were really necessary):Minor updates:
- add Misc/NEWS entry
- remove trailing whitespace and fix spaces that should have been a
tab- added some extra tests to check all possible combinations of bad arguments, purely to check for refcounting problems
- initialize low to NULL, to match the Py_XDECREF(low) (could change that Py_XDECREF to Py_DECREF instead, but the code's more resistant to random refactorings this way --- see next item :)
- randomly refactor: regroup blocks for ease of reading
- don't do PyLong_FromLong(1) until it's needed ('zero' is different, since it's always used in the non-error case)
- [micro-optimization]: don't pass a known zero value to get_range_long_argument
Any objections or comments? Can I apply this version of the patch?
Added file: http://bugs.python.org/file17200/issue1533_metd.diff
Python tracker <report@bugs.python.org> <http://bugs.python.org/issue1533>
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-04 12:43
Re emacs:
C-c . python
should set a python 2.x-friendly indentation mode.
There's also a python-new style floating around somewhere on the web (not part of emacs as standard), suitable for the 4-space indent style that's supposed to be used for new code.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-04 15:11
On Tue, May 4, 2010 at 8:34 AM, Mark Dickinson <report@bugs.python.org> wrote: ..
I took the liberty of messing with your patch slightly; I didn't want to ask you to make further changes since the patch was fine, and my messing was mostly just to satisfy my own fussiness (only the first two items were really necessary):
Thank you for doing it. The patch is good to go, questions and comments below are just for my education.
Minor updates:
- add Misc/NEWS entry
What is the standard practice on this? I thought Misc/NEWS entry was similar to commit message and would be written by a committer. What are the guidelines for writing such entries? I see that some entries simply repeat the title of the issues while others got into greater details.
- remove trailing whitespace and fix spaces that should have been a tab
I've looked at my patch through cat -et and couldn't find any tab/space issues. I did notice one empty line with a single space that you removed. Do you use an automated checking tool for this? I just did grep " $". BTW, the current trunk version (r80752) of Python/bltinmodule.c has two lines with trailing white space:
$ cat -n Python/bltinmodule.c | grep " $" | cat -et 641^I^I^IPyErr_SetString(PyExc_TypeError, $ 2966^I $
- added some extra tests to check all possible combinations of bad arguments, purely to check for refcounting problems
With arguments processing segregated in a common function, I am not sure whether additional tests actually increase coverage. This brings a question: what is the recommended way to produce coverage statistics for C modules? regrtest.py --coverage seems to be for python modules only.
- initialize low to NULL, to match the Py_XDECREF(low) (could change that Py_XDECREF to Py_DECREF instead, but the code's more resistant to random refactorings this way --- see next item :)
Good catch. Wouldn't the same argument apply to ilow? Wouldn't static code checkers complain about redundant initialization?
- randomly refactor: regroup blocks for ease of reading
I actually disagree that your regrouping makes the code clearer. In my version, all idiosyncratic argument processing is done with borrowed references first followed by common processing in three similar blocks. This, however, is purely matter of taste. Note that I considered changing i* names to raw* names, but decided not to introduce more changes than necessary. In your grouping, however, the similarity of variable names is more of an issue. This said, I don't really have any problem with your choice.
- don't do PyLong_FromLong(1) until it's needed ('zero' is different, since it's always used in the non-error case)
Yes, I considered that. A further micro-optimization would be to initialize a static variable in module initialization and reuse it in get_len_of_range_longs as well. I decided to put it next to zero instead to simplify the logic.
- [micro-optimization]: don't pass a known zero value to get_range_long_argument
Is it really worth it? Default start is probably not that common in case of long arguments.
Any objections or comments? Can I apply this version of the patch?
The above are not objections. Please apply this version of the patch.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-04 16:14
[Some of the Alexander's questions about procedures aren't really related to this issue; I've answered those offline. Here are the answers to the others.]
- initialize low to NULL, to match the Py_XDECREF(low) (could change that Py_XDECREF to Py_DECREF instead, but the code's more resistant to random refactorings this way --- see next item :)
Good catch. Wouldn't the same argument apply to ilow? Wouldn't static code checkers complain about redundant initialization?
ilow doesn't need to be initialized because the PyArgs_ParseTuple is guaranteed to either fail or initialize it, and I can't see that the PyArgs_ParseTuple call is likely to change. But I admit that the lack of initialization here also makes me uncomfortable, especially in combination with the assert that's there. I might add an initialization.
Do static code checkers really complain about redundant initializations? If anything, it seems like good defensive programming practice to initialize variables, even unnecessarily---later refactoring might make those initializations necessary.
- randomly refactor: regroup blocks for ease of reading
I actually disagree that your regrouping makes the code clearer. In my version, all idiosyncratic argument processing is done with borrowed references first followed by common processing in three similar blocks. This, however, is purely matter of taste. Note that I considered changing i* names to raw* names, but decided not to introduce more changes than necessary. In your grouping, however, the similarity of variable names is more of an issue. This said, I don't really have any problem with your choice.
Okay, fair enough. I agree it's a matter of taste. I like the three separate blocks, one for each argument, especially since the refcounting semantics are clear: each block adds exactly one reference. But each to his own. :)
- don't do PyLong_FromLong(1) until it's needed ('zero' is different, since it's always used in the non-error case)
Yes, I considered that. A further micro-optimization would be to initialize a static variable in module initialization and reuse it in get_len_of_range_longs as well. I decided to put it next to zero instead to simplify the logic.
Hmm. Possibly. I have an unhealthy and probably irrational aversion to non-constant static variables, even if the only time that the variable is changed is at module initialization.
- [micro-optimization]: don't pass a known zero value to get_range_long_argument
Is it really worth it? Default start is probably not that common in case of long arguments.
Yes, possibly not. :) Partly I made this change because the assignment 'ilow = zero;' again raises a red flag for me, because it's not accompanied by a 'Py_INCREF(zero);' as I'd expect it to be. I realize that in this case it works out (because ilow is already a borrowed reference, and we're replacing it with a borrowed reference to zero), but it's sufficiently unusual that I have to think about it. This is personal preference again, I guess.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-04 16:32
Applied to trunk in r80758.
Do people want this to go into 2.6 as well? The patch would need to be modified to produce a warning for floats instead of giving a TypeError (and the tests would need to be modified to test for that warning).
Author: Alexander Belopolsky (Alexander.Belopolsky)
Date: 2010-05-04 16:36
On Tue, May 4, 2010 at 12:32 PM, Mark Dickinson <report@bugs.python.org> wrote:
Mark Dickinson <dickinsm@gmail.com> added the comment:
Applied to trunk in r80758.
Do people want this to go into 2.6 as well?
Also, should additional unit tests forward ported to 3.x? If so, let me do it as an exercise in creating a ready to commit patch. :-)
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-04 16:43
+1 for forward-porting/adapting relevant tests to py3k.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-04 18:30
I am attaching a py3k patch that adds new tests. Since there are no end user visible changes, I don't believe a Misc/NEWS entry is needed. A commit message may read:
Issue #1533: Tests only. Added tests for consistency in range function argument processing.
Note that the first chunk:
# Reject floats when it would require PyLongs to represent.# (smaller floats still accepted, but deprecated)
# Reject floats.self.assertRaises(TypeError, range, 1., 1., 1.)
is applicable to the trunk as well.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-05 22:49
Perfect! Committed in r80836 (py3k); fixed that one test and comment in r80842 in trunk.
Alexander, do you want to tackle the 2.6 backport?
BTW, I think in most cases it's unnecessary to add Python 3.3 to the Versions field above, since there's no corresponding svn branch for 3.3. (But it's useful if there's a task that's specifically aimed at 3.3 and not earlier versions---e.g. if something's been deprecated in 3.2 and needs to be removed in 3.3.)
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-05 22:51
That should have been r80839, not r80842.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-05 23:35
Alexander, do you want to tackle the 2.6 backport?
I've never done a maintenance branch backport, but here is my attempt:
- Checkout release26-maint
- Apply 80757:80758 diff, fix rejected NEWS patch
- Ignore 80838:80839 diff - small floats are accepted in 2.6 range.
- Replace small float with large float in bad argument tests.
- make; make test; make patchcheck
I could probably use svn merge instead of svn diff + patch. Did I miss anything important?
BTW, I've discovered "make patchcheck", does it check C files white space issues or only python files?
Mark,
I've sent you two emails off the tracker, but it looks like they did not make it through your spam filters.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-06 07:59
I've never done a maintenance branch backport, but here is my attempt: [...]
Yes, that sounds about right. But after all that, you'll still need to modify the patch somewhat, since the requirements are different for 2.6: floats should give a DeprecationWarning rather than a TypeError. I think that's a straightforward change for Python/bltinmodule.c. The trickier bit is coming up with tests that work properly---i.e., check that the appropriate warnings are produced, and that the the appropriate values are returned. Look into the 'catch_warnings' function in the warnings module; (there's also 'check_warnings' in test_support, but I think that doesn't exist in 2.6).
'make patchcheck' only checks Python files and ReST files, as far as I can tell.
[I got your off-tracker emails; will respond anon.]
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-05-06 17:42
the requirements are different for 2.6: floats should give a DeprecationWarning rather than a TypeError.
I thought about it, but the comment in test_builtin.py,
# Reject floats when it would require PyLongs to represent.
# (smaller floats still accepted, but deprecated) convinced me that raising TypeError on large floats is a feature. I don't have a strong opinion on this issue, but I think a conservative approach is not to change current behavior in the maintenance branch unless it is clearly a bug.
I did add a test checking that "smaller floats still accepted, but deprecated."
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-07 13:12
Yes, okay---that makes some sense; I'm happy to leave floats as they are (i.e., DeprecationWarning for small floats; TypeError for larger floats) and just fix use of int for non-floats.
I'll look at the patch.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-05-07 13:26
The backport looks fine. Applied in r80917. Thanks, Alexander.
History
Date
User
Action
Args
2022-04-11 14:56:28
admin
set
github: 45874
2010-05-07 13:26:30
mark.dickinson
set
status: open -> closed
resolution: accepted -> fixed
messages: +
stage: patch review -> resolved
2010-05-07 13:12:49
mark.dickinson
set
assignee: belopolsky -> mark.dickinson
messages: +
2010-05-06 17:42:31
belopolsky
set
files: - issue1533-release26-maint.diff
2010-05-06 17:42:20
belopolsky
set
files: + issue1533-release26-maint.diff
messages: +
2010-05-06 07:59:13
mark.dickinson
set
messages: +
2010-05-05 23:39:21
belopolsky
set
keywords: + 26backport
versions: - Python 3.1, Python 2.7, Python 3.2, Python 3.3
2010-05-05 23:35:14
belopolsky
set
files: + issue1533-release26-maint.diff
nosy: - Alexander.Belopolsky
messages: +
2010-05-05 22:51:37
mark.dickinson
set
messages: +
2010-05-05 22:51:07
mark.dickinson
set
messages: -
2010-05-05 22:50:37
mark.dickinson
set
messages: +
2010-05-05 22:49:36
mark.dickinson
set
messages: +
2010-05-04 18:30:17
belopolsky
set
files: + issue1533-py3k-tests.diff
messages: +
versions: + Python 3.1, Python 3.2, Python 3.3
2010-05-04 16:43:18
mark.dickinson
set
assignee: mark.dickinson -> belopolsky
messages: +
2010-05-04 16:36:45
Alexander.Belopolsky
set
messages: +
2010-05-04 16:32:54
mark.dickinson
set
messages: +
2010-05-04 16:14:40
mark.dickinson
set
messages: +
2010-05-04 15:12:11
belopolsky
set
assignee: belopolsky -> mark.dickinson
2010-05-04 15:11:17
belopolsky
set
messages: +
2010-05-04 12:43:47
mark.dickinson
set
messages: +
2010-05-04 12:41:26
Alexander.Belopolsky
set
messages: +
2010-05-04 12:34:03
mark.dickinson
set
files: + issue1533_metd.diff
messages: +
2010-05-04 12:30:44
Alexander.Belopolsky
set
nosy: + Alexander.Belopolsky
messages: +
2010-05-04 11:27:10
mark.dickinson
set
resolution: accepted
messages: +
2010-05-03 03:52:08
belopolsky
set
files: - issue1533.diff
2010-05-03 03:52:00
belopolsky
set
files: + issue1533.diff
messages: +
2010-05-03 03:46:52
belopolsky
set
messages: +
2010-05-03 03:44:09
belopolsky
set
files: - bltinmodule4.diff
2010-05-03 03:44:02
belopolsky
set
files: - bltinmodule3.diff
2010-05-03 03:43:56
belopolsky
set
files: - bltinmodule.diff
2010-05-03 03:43:47
belopolsky
set
files: + issue1533.diff
messages: +
2010-05-02 09:44:20
mark.dickinson
set
messages: +
2010-05-02 09:06:12
mark.dickinson
set
messages: +
2010-05-02 08:37:40
mark.dickinson
set
messages: +
2010-05-02 00:06:27
belopolsky
set
files: - bltinmodule4.diff
2010-05-02 00:06:20
belopolsky
set
files: + bltinmodule4.diff
2010-05-02 00:03:20
belopolsky
set
files: + bltinmodule4.diff
messages: +
stage: test needed -> patch review
2010-05-01 20:21:13
mark.dickinson
set
messages: +
2010-05-01 20:15:34
belopolsky
set
messages: +
2010-05-01 20:12:22
belopolsky
set
files: + bltinmodule3.diff
messages: +
2010-05-01 20:03:08
mark.dickinson
set
messages: +
2010-05-01 19:33:28
mark.dickinson
set
status: closed -> open
resolution: out of date -> (no value)
2010-05-01 19:31:40
mark.dickinson
set
messages: +
2010-05-01 19:24:17
belopolsky
set
assignee: belopolsky
nosy: + belopolsky, - Alexander.Belopolsky
2010-05-01 19:20:52
Alexander.Belopolsky
set
messages: +
2010-05-01 19:02:40
robertwb
set
files: + bltinmodule2.diff
messages: +
2010-05-01 15:35:55
Alexander.Belopolsky
set
messages: +
2010-05-01 14:59:37
mark.dickinson
set
messages: +
2010-05-01 14:48:10
Alexander.Belopolsky
set
messages: +
2010-05-01 07:50:09
mark.dickinson
set
messages: +
2010-05-01 04:58:29
loewis
set
status: open -> closed
resolution: wont fix -> out of date
2010-05-01 02:01:18
Alexander.Belopolsky
set
status: pending -> open
nosy: + Alexander.Belopolsky, - belopolsky
messages: +
2010-05-01 01:05:57
mark.dickinson
set
status: open -> pending
resolution: wont fix
messages: +
2010-05-01 00:53:38
mark.dickinson
set
messages: +
stage: test needed
2010-05-01 00:34:52
mark.dickinson
set
messages: +
versions: - Python 3.1, Python 3.2
2010-02-10 20:24:56
mark.dickinson
set
nosy: + mark.dickinson
2010-02-09 16:39:25
brian.curtin
set
priority: normal
type: crash -> behavior
versions: + Python 3.1, Python 3.2, - Python 3.0
2009-01-02 21:50:18
rhettinger
set
assignee: rhettinger -> (no value)
2008-12-10 08:52:59
loewis
set
nosy: + loewis
messages: +
versions: - Python 2.5, Python 2.5.3
2008-11-29 22:53:58
akitada
set
messages: +
versions: + Python 2.6, Python 3.0, Python 2.7, Python 2.5.3
2008-11-29 20:45:36
robertwb
set
messages: +
2008-11-29 20:28:15
akitada
set
nosy: + akitada
messages: +
2008-02-25 17:52:53
robertwb
set
messages: +
2008-02-24 23:08:45
belopolsky
set
files: + bltinmodule.diff
keywords: + patch
messages: +
2008-02-24 22:14:51
christian.heimes
set
messages: +
2008-02-24 22:00:36
belopolsky
set
messages: +
2008-02-24 21:44:36
belopolsky
set
messages: +
2008-02-24 21:07:59
christian.heimes
set
nosy: + christian.heimes
messages: +
2008-02-24 21:03:21
belopolsky
set
nosy: + belopolsky
messages: +
2008-02-24 13:19:42
zanella
set
messages: +
2008-02-21 17:02:08
robertwb
set
messages: +
2008-02-21 11:16:28
zanella
set
nosy: + zanella
messages: +
2007-12-01 13:25:53
rhettinger
set
assignee: rhettinger
nosy: + rhettinger
2007-12-01 01:42:03
robertwb
set
messages: +
2007-12-01 01:37:18
josm
set
nosy: + josm
messages: +
2007-12-01 00:28:10
robertwb
create