Skip to content

Rebase range iterator __reduce__ to match CPython - #8424

Open
devyubin wants to merge 1 commit into
RustPython:mainfrom
devyubin:fix-range-iter-reduce
Open

Rebase range iterator __reduce__ to match CPython#8424
devyubin wants to merge 1 commit into
RustPython:mainfrom
devyubin:fix-range-iter-reduce

Conversation

@devyubin

@devyubin devyubin commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

range_iterator.__reduce__ (and longrange_iterator.__reduce__) returned the original range plus the current index as the pickle state, whereas CPython returns the remaining range rebased to the current position with a None state. The round-trip result was already correct on both (RustPython restored the index via __setstate__), so this is a representational / cross-compatibility divergence, not data loss.

it = iter(range(10)); next(it); next(it); next(it)
it.__reduce__()
# before: (<built-in function iter>, (range(0, 10),), 3)
# after:  (<built-in function iter>, (range(3, 10),), None)   # matches CPython

Cause

range_iter_reduce embedded the full original range and passed index as the third tuple element. Both PyRangeIterator::__reduce__ and PyLongRangeIterator::__reduce__ go through it.

Fix

Rebase the range start by index * step and emit None for the state. The index is clamped to the length first, because RustPython's iterator increments the index unconditionally, so it can run past the length after exhaustion (unlike CPython, which stops at the length). __setstate__ is left intact, so pickles that carry an integer state still load.

Test

Verified against CPython 3.14.6 — __reduce__ output now matches for mid-iteration, fresh, reversed, exhausted (range(2, 2)), step > 1, empty, single-element, negative-step, longrange, and __setstate__-advanced iterators. pickle.loads(pickle.dumps(it)) round-trips correctly in all cases.

  • test_range: SUCCESS (29 run, 2 skipped — the two skips are a separate, pre-existing __setstate__ crash, unrelated to this change).
  • cargo build / cargo clippy -p rustpython-vm / cargo fmt --check: clean.

No @expectedFailure marker flips here: CPython's own test_range has no test asserting the __reduce__ shape, so this is a representational fix, covered for regressions by the existing pickle round-trip tests.

Summary by CodeRabbit

  • Bug Fixes
    • Improved range iterator state handling when reducing or serializing iterators.
    • Prevented iterator positions from exceeding the range length.

range_iterator.__reduce__ (and longrange_iterator) returned the original
range plus the current index as pickle state; CPython returns the range
rebased to the current position with a None state. Rebase start by
index * step (clamped to the length) and emit None. __setstate__ is kept
so pickles carrying an integer state still load.

Assisted-by: Claude Code:claude-opus-4-8
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 429bd77d-bb7e-4200-9cbc-10e7db1a9d31

📥 Commits

Reviewing files that changed from the base of the PR and between dc1cae4 and ac0268f.

📒 Files selected for processing (1)
  • crates/vm/src/builtins/range.rs

📝 Walkthrough

Walkthrough

range_iter_reduce now serializes the remaining range from the current iterator position and uses None for pickle state.

Changes

Range iterator reduction

Layer / File(s) Summary
Rebase serialized range output
crates/vm/src/builtins/range.rs
range_iter_reduce clamps the index to the iterator length, advances the serialized range start, and emits None instead of the raw index.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • RustPython/RustPython#8384: Updates iterator reduction state to serialize remaining entries while preserving the current position.

Suggested reviewers: shaharnaveh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes rebasing range iterator reduce to match CPython, which is the primary change.
Linked Issues check ✅ Passed The change rebases the remaining range, clamps the index, and emits None state, meeting issue #8377 objectives.
Out of Scope Changes check ✅ Passed The changes are limited to range iterator reduction behavior and directly support issue #8377.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@moreal moreal added the z-ca-2026 Tag to track Contribution Academy 2026 label Aug 1, 2026
@devyubin
devyubin marked this pull request as ready for review August 4, 2026 11:35

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a test about this. copying the example code from original issue to extra_tests/snippets/builtin_range.py or stdlib_pickle.py will be good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

range_iterator.__reduce__ uses original range + index state instead of rebased range + None

3 participants