Issue28376
Created on 2016-10-06 12:00 by Oren Milman, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| CPythonTestOutput.txt | Oren Milman, 2016-10-06 12:00 | test output of CPython without my patches (tested on my PC) | ||
| patchedCPythonTestOutput_ver1.txt | Oren Milman, 2016-10-06 12:00 | test output of CPython with my patches (tested on my PC) - ver1 | ||
| issue28376_ver1.diff | Oren Milman, 2016-10-06 12:02 | proposed patches diff file - ver1 | review | |
| issue28376_CPython35_ver2.diff | Oren Milman, 2016-10-07 08:51 | proposed patches diff file for 3.5 - ver2 | review | |
| issue28376_CPython36_ver2.diff | Oren Milman, 2016-10-07 08:53 | proposed patches diff file for 3.6 - ver2 | review | |
| issue28376_CPython37_ver2.diff | Oren Milman, 2016-10-07 08:53 | proposed patches diff file for 3.7 - ver2 | review | |
| patchedCPython35TestOutput_ver2.txt | Oren Milman, 2016-10-07 08:54 | test output of CPython3.5 with my patches (tested on my PC) - ver2 | ||
| patchedCPython36TestOutput_ver2.txt | Oren Milman, 2016-10-07 08:54 | test output of CPython3.6 with my patches (tested on my PC) - ver2 | ||
| patchedCPython37TestOutput_ver2.txt | Oren Milman, 2016-10-07 08:55 | test output of CPython3.7 with my patches (tested on my PC) - ver2 | ||
| issue28376_CPython36_ver3.diff | Oren Milman, 2016-10-08 16:25 | proposed patches diff file for 3.6 - ver3 | review | |
| issue28376_CPython37_ver3.diff | Oren Milman, 2016-10-08 16:25 | proposed patches diff file for 3.7 - ver3 | review | |
| patchedCPython36TestOutput_ver3.txt | Oren Milman, 2016-10-08 16:26 | test output of CPython3.6 with my patches (tested on my PC) - ver3 | ||
| patchedCPython37TestOutput_ver3.txt | Oren Milman, 2016-10-08 16:26 | test output of CPython3.7 with my patches (tested on my PC) - ver3 | ||
| issue28376_CPython36_ver4.diff | Oren Milman, 2016-10-08 17:51 | proposed patches diff file for 3.6 - ver4 | review | |
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 552 | closed | dstufft, 2017-03-31 16:36 | |
| Messages (14) | |||
|---|---|---|---|
| msg278187 - (view) | Author: Oren Milman (Oren Milman) * | Date: 2016-10-06 12:00 | |
------------ current state ------------ An assertion in Objects/rangeobject.c might fail: >>> type(iter(range(0))) <class 'range_iterator'> >>> type(iter(range(0)))(1, 1, 0) Assertion failed: step != 0, file ..\Objects\rangeobject.c, line 895 This is caused by the lack of a check whether 'step' is zero, during the creation of a range_iterator object, in rangeiter_new. Note that during the creation of a range object, the function range_new does check that, by calling validate_step, which leads to the following behavior: >>> range(1, 1, 0) Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: range() arg 3 must not be zero >>> ------------ proposed changes ------------ 1. In Objects/rangeobject.c in rangeiter_new: - in case 'step' is zero, raise a ValueError - in error messages, replace 'rangeiter' with 'range_iterator', as the latter is the name of the type in Python code 2. In Lib/test/test_range.py, add tests for calling the range_iterator type (i.e. creating a range_iterator object) ------------ diff ------------ The proposed patches diff file is attached. ------------ 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_range passed.) The outputs of both runs are attached. |
|||
| msg278188 - (view) | Author: Anilyka Barry (abarry) * | Date: 2016-10-06 12:31 | |
I'm not able to trigger an assertion error when running the latest trunk in debug mode. I get a "valid" range_iterator object though, and using __reduce__ gives me direct access to `range(0, 0, 0)` (which is completely worthless). Error or not, this should be fixed, and your patch LGTM. Thanks! (I changed the type to 'behaviour' since I can't reproduce the assertion, but it doesn't make much of a difference) |
|||
| msg278189 - (view) | Author: Inada Naoki (methane) * | Date: 2016-10-06 12:46 | |
patch is LGTM. But there is one hidden inconsistency: >>> r = range(2**100) >>> type(iter(r)) <class 'longrange_iterator'> >>> type(iter(r))(1, 1, 0) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: cannot create 'longrange_iterator' instances Should we have same tp_new method for longrange_iterator? |
|||
| msg278197 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-10-06 17:08 | |
Good point Naoki. I think we can remove tp_new method from range_iterator in 3.7. Seems it is never used. The patch LGTM for 3.5-3.6, but the test should be marked as CPython implementation detail (@support.cpython_only). |
|||
| msg278232 - (view) | Author: Oren Milman (Oren Milman) * | Date: 2016-10-07 08:51 | |
The diff files for 3.5 and 3.6 are attached (I only added @test.support.cpython_only, as you suggested).
The diff file for 3.7 is also attached. It contains:
- removing rangeiter_new
- tests to verify one cannot create instances of range_iterator or longrange_iterator (in CPython)
- added 'Iterator.register(longrange_iterator)' in Lib/_collections_abc.py
The output of 'python_d.exe -m test -j3' for each version is also attached (they were all successful).
P.S. If we remove the ability to create instances of range_iterator in 3.7, shouldn't we raise a DeprecationWarning in rangeiter_new in 3.5 and 3.6?
|
|||
| msg278234 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-10-07 11:49 | |
Raising a DeprecationWarning in 3.6 doesn't hurt. |
|||
| msg278237 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-10-07 12:22 | |
rangeiter_new() was added in r55167. |
|||
| msg278238 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-10-07 12:23 | |
And merged in a6eb6acfe04a. |
|||
| msg278307 - (view) | Author: Oren Milman (Oren Milman) * | Date: 2016-10-08 16:25 | |
The diff files for 3.6 and 3.7 are attached (named '*ver3.diff').
Changes:
- in 3.6, added:
* raising a DeprecationWarning in rangeiter_new
* a test to verify the DeprecationWarning is raised
- in 3.7:
* changed the tests so they would only verify a TypeError is raised when calling either range_iterator or longrange_iterator
* removed the test.support.cpython_only decorator of the test
The output of 'python_d.exe -m test -j3' for each version is also attached (they were both successful).
|
|||
| msg278309 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-10-08 16:44 | |
Thanks Oren. Patches for 3.5 and 3.7 LGTM, the patch for 3.6 should be fixed. Tests should pass with -Wa and -We. No need to attach test output. Just check that your patch doesn't add a regression. |
|||
| msg278313 - (view) | Author: Oren Milman (Oren Milman) * | Date: 2016-10-08 17:51 | |
Alright, just added 'with test.support.check_warnings(('', DeprecationWarning)):'.
*ver4.diff is attached.
I ran the tests again (this time using 'python_d.exe -We -m test -j3'), and they were successful :)
That was totally my bad, of course, but anyway, ISTM that https://docs.python.org/devguide/index.html should say './python -We -m test -j3' in section 4 (Run the tests).
|
|||
| msg278315 - (view) | Author: Roundup Robot (python-dev) | Date: 2016-10-08 19:08 | |
New changeset ee049edc3fff by Serhiy Storchaka in branch '3.5': Issue #28376: Fixed typos. https://hg.python.org/cpython/rev/ee049edc3fff New changeset e486f3d30e0e by Serhiy Storchaka in branch '3.5': Issue #28376: The constructor of range_iterator now checks that step is not 0. https://hg.python.org/cpython/rev/e486f3d30e0e New changeset 06f065a59751 by Serhiy Storchaka in branch '3.6': Issue #28376: Creating instances of range_iterator by calling range_iterator https://hg.python.org/cpython/rev/06f065a59751 New changeset e099583400f3 by Serhiy Storchaka in branch 'default': Issue #28376: Creating instances of range_iterator by calling range_iterator https://hg.python.org/cpython/rev/e099583400f3 New changeset ce4af7593e45 by Serhiy Storchaka in branch '3.5': Issue #28376: The type of long range iterator is now registered as Iterator. https://hg.python.org/cpython/rev/ce4af7593e45 |
|||
| msg278316 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-10-08 19:09 | |
Thank you for your contribution Oren. |
|||
| msg278352 - (view) | Author: Oren Milman (Oren Milman) * | Date: 2016-10-09 09:48 | |
Thanks for the reviews and patience :) |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2017-03-31 16:36:37 | dstufft | set | pull_requests: + pull_request1098 |
| 2016-10-09 09:48:43 | Oren Milman | set | messages: + msg278352 |
| 2016-10-08 19:09:37 | serhiy.storchaka | set | status: open -> closed messages: + msg278316 assignee: serhiy.storchaka |
| 2016-10-08 19:08:34 | python-dev | set | nosy:
+ python-dev messages: + msg278315 |
| 2016-10-08 17:51:49 | Oren Milman | set | files:
+ issue28376_CPython36_ver4.diff messages: + msg278313 |
| 2016-10-08 16:44:09 | serhiy.storchaka | set | messages: + msg278309 |
| 2016-10-08 16:26:49 | Oren Milman | set | files: + patchedCPython37TestOutput_ver3.txt |
| 2016-10-08 16:26:16 | Oren Milman | set | files: + patchedCPython36TestOutput_ver3.txt |
| 2016-10-08 16:25:40 | Oren Milman | set | files: + issue28376_CPython37_ver3.diff |
| 2016-10-08 16:25:17 | Oren Milman | set | files:
+ issue28376_CPython36_ver3.diff messages: + msg278307 |
| 2016-10-07 12:23:42 | serhiy.storchaka | set | messages: + msg278238 |
| 2016-10-07 12:22:26 | serhiy.storchaka | set | messages: + msg278237 |
| 2016-10-07 11:49:58 | serhiy.storchaka | set | messages: + msg278234 |
| 2016-10-07 08:55:13 | Oren Milman | set | files: + patchedCPython37TestOutput_ver2.txt |
| 2016-10-07 08:54:36 | Oren Milman | set | files: + patchedCPython36TestOutput_ver2.txt |
| 2016-10-07 08:54:09 | Oren Milman | set | files: + patchedCPython35TestOutput_ver2.txt |
| 2016-10-07 08:53:29 | Oren Milman | set | files: + issue28376_CPython37_ver2.diff |
| 2016-10-07 08:53:11 | Oren Milman | set | files: + issue28376_CPython36_ver2.diff |
| 2016-10-07 08:51:46 | Oren Milman | set | files:
+ issue28376_CPython35_ver2.diff messages: + msg278232 |
| 2016-10-06 17:08:22 | serhiy.storchaka | set | messages: + msg278197 |
| 2016-10-06 14:38:28 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka versions: + Python 3.5, Python 3.6 |
| 2016-10-06 12:46:20 | methane | set | nosy:
+ methane messages: + msg278189 |
| 2016-10-06 12:31:26 | abarry | set | nosy:
+ abarry title: assertion failure in rangeobject.c -> rangeiter_new fails when creating a range of step 0 messages: + msg278188 type: crash -> behavior |
| 2016-10-06 12:02:33 | Oren Milman | set | files:
+ issue28376_ver1.diff keywords: + patch |
| 2016-10-06 12:00:51 | Oren Milman | set | files: + patchedCPythonTestOutput_ver1.txt |
| 2016-10-06 12:00:27 | Oren Milman | create | |