msg198049 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-19 09:21 |
itertools.tee uses ints rather than Py_ssize_t for indices. Using Py_ssize_t would not make the object bigger, since other fields will already be pointer-aligned. |
|
|
msg198059 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-19 11:37 |
Simple patch attached. |
|
|
msg198097 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-09-19 17:33 |
I don't think we need Py_ssize_t for integers from 0 to 57. |
|
|
msg198099 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-19 17:57 |
> I don't think we need Py_ssize_t for integers from 0 to 57. IMO it's better to use Py_ssize_t there, since it can get combined with other Py_ssize_t values. Avoiding spurious conversions eliminates a common source of errors. We don't gain anything by keeping it as an int. |
|
|
msg198100 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-09-19 18:05 |
We gain stability. I doubt we should change this. However in any case you forgot about tee_reduce(). |
|
|
msg198103 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-19 18:07 |
> I doubt we should change this. Currently itertools.tee() will break if more than 2**31 elements are saved in the inner structures. I certainly think it should be fixed, rather than bite someone by surprise one day. > However in any case you forgot about tee_reduce(). Will take a look. |
|
|
msg198109 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-19 18:24 |
Ah, my bad, it seems I misunderstood the tee() implementation. So apparenly even index cannot get past 57. It's more of a consistency patch then, and I agree it isn't very important. |
|
|
msg198135 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-09-20 09:06 |
There are disadvantages in the changing int to Py_ssize_t. Converting Py_ssize_t to/from Python int is a little harder than converting C int or long. This change (as any other change) has a risk of introduce new bugs (as you can see on example of your patch). I suggest just add (yet one) explicit comment. int index; /* 0 <= index <= LINKCELLS */ |
|
|
msg198137 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2013-09-20 09:34 |
> I suggest just add (yet one) explicit comment. > > int index; /* 0 <= index <= LINKCELLS */ Ok, I think a comment is good enough here. |
|
|
msg198165 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-09-20 20:19 |
New changeset 900bf633b7f4 by Antoine Pitrou in branch 'default': Add a comment making it explicit that itertools.tee() is already 64bit-safe (issue #19049) http://hg.python.org/cpython/rev/900bf633b7f4 |
|
|