gh-126618: Fix representation of itertools.count(sys.maxsize)#126620
gh-126618: Fix representation of itertools.count(sys.maxsize)#126620picnixz wants to merge 8 commits intopython:mainfrom
itertools.count(sys.maxsize)#126620Conversation
picnixz
added
needs backport to 3.12
picnixz
changed the title
itertools.count.__repr__ when at sys.maxsizeitertools.count(sys.maxsize).__repr__
picnixz
changed the title
itertools.count(sys.maxsize).__repr__itertools.count(sys.maxsize)
|
Converting to draft until the CI failures are fixed by #126617. |
skirpichev
left a comment
skirpichev
left a comment
There was a problem hiding this comment.
Here is a more simple workaround:
diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c
index 1201fa0949..a00b867d73 100644
--- a/Modules/itertoolsmodule.c
+++ b/Modules/itertoolsmodule.c
@@ -3415,7 +3418,7 @@ count_next(countobject *lz)
static PyObject *
count_repr(countobject *lz)
{
- if (lz->cnt != PY_SSIZE_T_MAX)
+ if (lz->long_cnt == NULL)
return PyUnicode_FromFormat("%s(%zd)",
_PyType_Name(Py_TYPE(lz)), lz->cnt);
But, perhaps, your solution is better, as it ensure that either:
assert(cnt != PY_SSIZE_T_MAX && long_cnt == NULL && long_step==1)
or
assert(cnt == PY_SSIZE_T_MAX && long_cnt != NULL && long_step != NULL)
kept.
|
Yea, I wanted to keep those invariants but if performances matter more, we may use your solution (it would reduce the number of operations to perform in general). |
|
Is there a simpler fix? I was expecting a much more minor edit to just the repr code. |
Sergey's patch could be the solution if you don't mind having the invariants invalid. |
|
Perhaps, here is slightly another alternative to this fix and #126617: Detailsdiff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c
index 78fbdcdf77..6ca93294b6 100644
--- a/Modules/itertoolsmodule.c
+++ b/Modules/itertoolsmodule.c
@@ -3291,9 +3291,6 @@ itertools_count_impl(PyTypeObject *type, PyObject *long_cnt,
PyErr_Clear();
fast_mode = 0;
}
- else if (cnt == PY_SSIZE_T_MAX) {
- fast_mode = 0;
- }
}
} else {
cnt = 0;
@@ -3325,7 +3322,7 @@ itertools_count_impl(PyTypeObject *type, PyObject *long_cnt,
else
cnt = PY_SSIZE_T_MAX;
- assert((cnt != PY_SSIZE_T_MAX && long_cnt == NULL && fast_mode) ||
+ assert((long_cnt == NULL && fast_mode) ||
(cnt == PY_SSIZE_T_MAX && long_cnt != NULL && !fast_mode));
assert(!fast_mode ||
(PyLong_Check(long_step) && PyLong_AS_LONG(long_step) == 1));
@@ -3418,7 +3415,7 @@ count_next(countobject *lz)
static PyObject *
count_repr(countobject *lz)
{
- if (lz->cnt != PY_SSIZE_T_MAX)
+ if (lz->long_cnt == NULL)
return PyUnicode_FromFormat("%s(%zd)",
_PyType_Name(Py_TYPE(lz)), lz->cnt);
|
skirpichev
left a comment
skirpichev
left a comment
There was a problem hiding this comment.
It seems, CI failure is related to the patch.
Alternative solution (from the thread) is presented in #127048.
|
The CI failure is in importlib so I don't really know but I'll have a look at your patch when I am back. |
Yes, it's not obvious how this failure is triggered. But your code alters free-threading code - and that failure clearly belongs to it, I did several updates. |
|
Closing in favor of #127048. |
3 participants
Backports for 3.12 will be manual since the free-threaded code will not be needed.
itertools.count(sys.maxsize)#126618