Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-126618: Fix representation ofitertools.count(sys.maxsize)#126620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
itertools.count.__repr__ when atsys.maxsizeitertools.count(sys.maxsize).__repr__itertools.count(sys.maxsize).__repr__itertools.count(sys.maxsize)picnixz commentedNov 9, 2024
Converting to draft until the CI failures are fixed by#126617. |
skirpichev left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Here is a more simple workaround:
diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.cindex 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.
picnixz commentedNov 10, 2024
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). |
rhettinger commentedNov 12, 2024
Is there a simpler fix? I was expecting a much more minor edit to just the repr code. |
picnixz commentedNov 12, 2024
Sergey's patch could be the solution if you don't mind having the invariants invalid. |
skirpichev commentedNov 13, 2024
Perhaps, here is slightly another alternative to this fix and#126617: Detailsdiff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.cindex 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It seems, CI failure is related to the patch.
Alternative solution (from the thread) is presented in#127048.
picnixz commentedNov 20, 2024
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. |
skirpichev commentedNov 20, 2024
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. |
picnixz commentedNov 30, 2024
Closing in favor of#127048. |
Uh oh!
There was an error while loading.Please reload this page.
Backports for 3.12 will be manual since the free-threaded code will not be needed.
itertools.count(sys.maxsize)#126618