Issue 19049: itertools.tee uses int for indices (original) (raw)

This issue has been migrated to GitHub: https://github.com/python/cpython/issues/63249

classification

Title: itertools.tee uses int for indices
Type: enhancement Stage: resolved
Components: Versions: Python 3.4

process

Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: pitrou, python-dev, rhettinger, serhiy.storchaka
Priority: low Keywords: patch

Created on 2013-09-19 09:21 by pitrou, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
tee_64b.patch pitrou,2013-09-19 11:37 review
Messages (10)
msg198049 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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) * (Python committer) Date: 2013-09-19 11:37
Simple patch attached.
msg198097 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python triager) 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
History
Date User Action Args
2022-04-11 14:57:51 admin set github: 63249
2013-09-20 21:14:34 rhettinger set assignee: rhettinger
2013-09-20 20:19:45 pitrou set status: open -> closedresolution: fixedstage: patch review -> resolved
2013-09-20 20:19:29 python-dev set nosy: + python-devmessages: +
2013-09-20 09:34:16 pitrou set messages: +
2013-09-20 09:06:32 serhiy.storchaka set messages: +
2013-09-19 18:24:07 pitrou set priority: normal -> lowversions: - Python 3.3title: itertools.tee isn't 64-bit compliant -> itertools.tee uses int for indicesmessages: + type: behavior -> enhancementstage: needs patch -> patch review
2013-09-19 18:07:35 pitrou set messages: +
2013-09-19 18:05:19 serhiy.storchaka set messages: +
2013-09-19 17:57:21 pitrou set messages: +
2013-09-19 17:33:00 serhiy.storchaka set messages: +
2013-09-19 11:37:17 pitrou set files: + tee_64b.patchkeywords: + patchmessages: +
2013-09-19 09:21:08 pitrou create