Repository navigation
Check n before iter() in itertools.tee() and re-check the iterator - #9518
Conversation
tee() bound its iterable as an iterator, so iter() ran during argument binding: a negative n was reported only after __iter__ succeeded, and n == 0 still called it. tee() also used an iterator's own __copy__ instead of buffering it, and never called iter() on the iterator it buffered, so an iterator whose __iter__ returns a non-iterator went unnoticed. Check n first and return an empty tuple for zero, then always buffer the iterator in a tee object, which calls iter() on it again. Assisted-by: Claude Code:claude-opus-5-5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Input handling and result creation crates/vm/src/stdlib/itertools.rs, extra_tests/snippets/stdlib_itertools.py |
tee accepts a Python object, returns an empty tuple for n == 0, and obtains an iterator for positive counts. PyItertoolsTee::from_iter validates its input with get_iter. Tests cover count handling, invalid iterators, and __copy__ failures. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related PRs
- RustPython/RustPython#9322: It changes shared tee buffer storage, cursor advancement, and tee copying in the same implementation.
Suggested reviewers: youknowone
Merge Risk: 
c2b8a
-
No concrete merge-blocking issue is established for the tee changes; mergeable subject to normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main changes: validating nbefore callingiter()and re-checking the iterator initertools.tee().Linked Issues check The changes satisfy the coding requirements in direct issue #9512.TeeArgsnow keeps the iterable asPyObjectRef, sotee()can validate negativenbeforeiter(). The implementation returns t…Out of Scope Changes check The diff stays within issue #9512. The Rust changes implement the requireditertools.tee()behavior. The Python changes add focused regression tests for those behaviors. No unrelated production beha…Full details: Linked Issues check
-
Explanation
The changes satisfy the coding requirements in direct issue
#9512.TeeArgsnow keeps the iterable asPyObjectRef, sotee()can validate negativenbeforeiter(). The implementation returns the empty tuple forn == 0before iterator conversion. For positiven, it callsget_iter()on the iterable and again inPyItertoolsTee::from_iterbefore buffering. It removes the direct input__copy__shortcut while retaining copying for tee objects. The added tests cover negative and zero counts, invalid iterator conversion, and an iterator whose__copy__raises.
Full details: Out of Scope Changes check
-
Explanation
The diff stays within issue
#9512. The Rust changes implement the requireditertools.tee()behavior. The Python changes add focused regression tests for those behaviors. No unrelated production behavior or unrelated tests are changed.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Merging this PR will not alter performance
Comparing 
Footnotes
|
itertools.tee()callsiter()before checkingn, and never calls it on the iterator it buffers #9512Summary
TeeArgsbindsiterableasPyIter, soiter()runs while the arguments are bound, beforeteechecksn.teethen copies an iterator that has__copy__directly, andPyItertoolsTee::from_iterstores any other iterator without callingiter()on it. In CPython the argument clinic rejects a negativen, anditertools_tee_implreturns()for zero before callingiter(), then buffers the iterator in a tee object, whosetee_fromiterablecallsiter()on it again.AS-IS
TO-BE
(matching CPython's exception types; CPython 3.15 words the first message
n cannot be negative)Changes
crates/vm/src/stdlib/itertools.rs, binditerableasPyObjectRef.teechecksn, returns an empty tuple for zero, and only then callsiter().__copy__shortcut, so every iterator goes throughPyItertoolsTee::from_iter, which now callsiter()on the iterator it is given. A tee object is still copied rather than wrapped. An iterator's own__copy__is no longer called, as in CPython since gh-123884.extra_tests/snippets/stdlib_itertools.pywith a negative and a zeronon a broken iterable, an iterator whose__iter__returns a non-iterator, and an iterator whose__copy__raises.Test plan
Built and run on macOS aarch64 against upstream/main 713e14c, using the CI feature set
--no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env,doc,ucd_3_2_0, plusthreading, compared with CPython 3.15.2.extra_tests/snippets/stdlib_itertools.pyfails on an unpatched build of that commit (attee(NoneIter(), -1)) and passes with the patch and under CPython 3.15.2.rustpython -m test test_itertools: 137 run, 22 skipped, all pass.pytest -k rustpythoninextra_tests: 248 pass.cargo fmt --checkclean,cargo clippy -p rustpython-vm -- -Dwarningsreports nothing, and the pre-commit hooks pass on both changed files.I used Claude Code (claude-opus-5-5) to draft the analysis, the patch and this description; I reviewed the change and ran every command above myself.
Summary by CodeRabbit