Skip to content

dict iterator __reduce__ to resume from current position - #8384

Merged
youknowone merged 3 commits into
RustPython:mainfrom
zzarbttoo:fix-dict-iter-reduce-position
Jul 27, 2026
Merged

dict iterator __reduce__ to resume from current position#8384
youknowone merged 3 commits into
RustPython:mainfrom
zzarbttoo:fix-dict-iter-reduce-position

Conversation

@zzarbttoo

@zzarbttoo zzarbttoo commented Jul 26, 2026 •
edited by moreal
Loading

Copy link
Copy Markdown
Contributor

Summary

Pickling a partially-consumed dict / dict-view iterator restarted from the beginning because reduce materialized every entry and ignored the iterator's position. Walk from the current position (mirroring next) so only the not-yet-yielded entries are captured, matching CPython, which reduces both directions to iter(remaining).

Fixing the reverse iterator also uncovered two pre-existing bugs in reverse iteration itself, both from prev_entry saturating at 0: a hole just above index 0 made the entry at 0 yield twice, and deleting the first-inserted key made reversed() loop forever. prev_entry now returns the found entry's actual index and stops cleanly at index 0, and the reverse iterator/reduce detect exhaustion from that index.

Verified against CPython 3.14 across all pickle protocols, dict states (including deletions), iterator kinds, and consumption counts. No regressions in test_dict, test_dictviews, test_ordered_dict, test_collections, test_userdict, test_iter, or test_copy.

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

Summary by CodeRabbit

  • Bug Fixes
    • Improved reverse dictionary iteration so reduced views traverse entries correctly from end to start.
    • Fixed reverse iteration termination when encountering empty positions near the beginning, preventing repeats or premature stopping.
    • Enhanced dictionary iterator state handling for both forward and reverse traversal to ensure consistency across reductions and subsequent iteration.

@zzarbttoo @claude
Fix dict iterator __reduce__ to resume from current position
75d60e7 
Pickling a partially-consumed dict / dict-view iterator restarted from the
beginning because __reduce__ materialized every entry and ignored the
iterator's position. Walk from the current position (mirroring next) so
only the not-yet-yielded entries are captured, matching CPython, which
reduces both directions to iter(remaining).

Fixing the reverse iterator also uncovered two pre-existing bugs in
reverse iteration itself, both from prev_entry saturating at 0: a hole
just above index 0 made the entry at 0 yield twice, and deleting the
first-inserted key made reversed() loop forever. prev_entry now returns
the found entry's actual index and stops cleanly at index 0, and the
reverse iterator/reduce detect exhaustion from that index.

Verified against CPython 3.14 across all pickle protocols, dict states
(including deletions), iterator kinds, and consumption counts. No
regressions in test_dict, test_dictviews, test_ordered_dict,
test_collections, test_userdict, test_iter, or test_copy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Dict and dict-view iterator reduction now serializes remaining entries from the stored position. Reverse iterators traverse dictionary storage directly, update positions from returned indices, and terminate correctly when probing reaches an empty slot at index zero.

Changes

Dict iterator traversal

Layer / File(s) Summary
Reverse entry boundary handling
crates/vm/src/dict_inner.rs		 Dict::prev_entry explicitly terminates at an empty slot at position zero and decrements positions otherwise.
Directional iterator reduction
crates/vm/src/builtins/dict.rs		 Forward and reverse __reduce__ implementations materialize remaining entries using directional dictionary traversal; reverse next updates its position from the returned index.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #8376 by preserving dict and dict-view iterator position in reduce and fixing reverse iteration behavior.
Out of Scope Changes check ✅ Passed The edits are confined to dict iterator reduction and reverse traversal fixes, with no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: dict iterator reduce now resumes from the current position.
✨ 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.

zzarbttoo and others added 2 commits July 26, 2026 16:54
@zzarbttoo @claude
Apply cargo fmt to dict.rs imports
dee77d0 
Collapse the builtins import block left multi-line after removing the
now-unused builtins_reversed import.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zzarbttoo @claude
Unmark TestSyncManagerTypes.test_dict as expectedFailure
ec133b1 
The dict iterator __reduce__ fix makes SyncManager dict proxies pickle
correctly, so this test now passes under spawn and forkserver. Remove the
stale expectedFailure marker that was causing an UNEXPECTED SUCCESS CI
failure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_float.py (TODO: 3)
[x] test: cpython/Lib/test/test_strtod.py (TODO: 2)

dependencies:

dependent tests: (no tests depend on float)

[x] test: cpython/Lib/test/test_termios.py (TODO: 5)

dependencies:

dependent tests: (8 tests)

  • termios: test_getpass test_pyrepl
    • getpass:
      • imaplib: test_imaplib
    • tty: test_asyncio test_pty test_sundry
      • pty: test_builtin test_repl

[x] test: cpython/Lib/test/test_format.py (TODO: 6)

dependencies:

dependent tests: (no tests depend on format)

[x] lib: cpython/Lib/threading.py
[x] lib: cpython/Lib/_threading_local.py
[ ] test: cpython/Lib/test/test_threading.py (TODO: 17)
[x] test: cpython/Lib/test/test_threadedtempfile.py
[ ] test: cpython/Lib/test/test_threading_local.py (TODO: 3)

dependencies:

  • threading

dependent tests: (163 tests)

  • threading: test_android test_asyncio test_bytes test_bz2 test_code test_concurrent_futures test_context test_contextlib test_ctypes test_decimal test_docxmlrpc test_email test_enum test_external_inspection test_fork1 test_frame test_ftplib test_functools test_gc test_hashlib test_httplib test_httpservers test_imaplib test_importlib test_inspect test_io test_ioctl test_itertools test_largefile test_linecache test_logging test_memoryview test_opcache test_pathlib test_poll test_poplib test_pyrepl test_queue test_robotparser test_sched test_signal test_smtplib test_socket test_socketserver test_sqlite3 test_ssl test_subprocess test_super test_sys test_syslog test_termios test_threadedtempfile test_threading test_threading_local test_time test_urllib2_localnet test_weakref test_winreg test_wsgiref test_xmlrpc test_zstd
    • asyncio: test_asyncio test_os test_pdb test_unittest
    • bdb: test_bdb
    • concurrent.futures._base: test_concurrent_futures
    • concurrent.futures.process: test_compileall test_concurrent_futures
    • concurrent.futures.thread: test_genericalias
    • dummy_threading: test_dummy_threading
    • http.cookiejar: test_http_cookiejar test_urllib2
      • urllib.request: test_pathlib test_pydoc test_sax test_site test_urllib test_urllib2net test_urllibnet
    • importlib.util: test_asdl_parser test_ctypes test_doctest test_importlib test_pkgutil test_py_compile test_reprlib test_runpy test_zipfile test_zipimport
      • pkgutil: test_pyrepl
      • py_compile: test_argparse test_cmd_line_script test_importlib test_modulefinder test_multiprocessing_main_handling
      • pyclbr: test_pyclbr
      • sysconfig: test_c_locale_coercion test_cmd_line test_dtrace test_embed test_launcher test_osx_env test_peg_generator test_posix test_pyexpat test_regrtest test_support test_sysconfig test_tools test_venv
      • zipfile: test_shutil test_zipapp test_zipfile test_zipfile64 test_zipimport_support
    • logging: test_unittest
      • hashlib: test_hmac test_tarfile test_unicodedata
    • multiprocessing: test_fcntl test_re
    • queue: test_dummy_thread
    • subprocess: test_atexit test_audit test_ctypes test_faulthandler test_file_eintr test_gzip test_json test_msvcrt test_ntpath test_platform test_plistlib test_pyrepl test_quopri test_repl test_script_helper test_select test_sys_settrace test_tempfile test_traceback test_unittest test_utf8_mode test_wait3 test_webbrowser test_xpickle
      • ctypes.util: test_ctypes
      • ensurepip: test_ensurepip
      • platform: test__locale test__osx_support test_baseexception test_builtin test_cmath test_math test_mimetypes test_strptime
    • sysconfig:
      • trace: test_trace
    • zipfile:
      • shutil: test_filecmp test_glob test_string_literals test_unicode_file test_zoneinfo

[x] lib: cpython/Lib/pty.py
[x] test: cpython/Lib/test/test_pty.py (TODO: 1)

dependencies:

  • pty

dependent tests: (4 tests)

  • pty: test_builtin test_pty test_pyrepl test_repl

[x] lib: cpython/Lib/sqlite3
[x] test: cpython/Lib/test/test_sqlite3 (TODO: 70)

dependencies:

  • sqlite3

dependent tests: (2 tests)

  • sqlite3: test_dbm_sqlite3 test_sqlite3

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

youknowone
youknowone approved these changes Jul 27, 2026
View reviewed changes

@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.

thank you! we are getting more and more loop in dict. we probably need to redesign dict one day...

@youknowone
youknowone merged commit f8c66d0 into RustPython:main Jul 27, 2026
27 checks passed
youknowone pushed a commit to youknowone/RustPython that referenced this pull request Jul 29, 2026
@zzarbttoo @claude @youknowone
dict iterator __reduce__ to resume from current position (RustPython…
ab6f1cd 
…#8384)

* Fix dict iterator __reduce__ to resume from current position

Pickling a partially-consumed dict / dict-view iterator restarted from the
beginning because __reduce__ materialized every entry and ignored the
iterator's position. Walk from the current position (mirroring next) so
only the not-yet-yielded entries are captured, matching CPython, which
reduces both directions to iter(remaining).

Fixing the reverse iterator also uncovered two pre-existing bugs in
reverse iteration itself, both from prev_entry saturating at 0: a hole
just above index 0 made the entry at 0 yield twice, and deleting the
first-inserted key made reversed() loop forever. prev_entry now returns
the found entry's actual index and stops cleanly at index 0, and the
reverse iterator/reduce detect exhaustion from that index.

Verified against CPython 3.14 across all pickle protocols, dict states
(including deletions), iterator kinds, and consumption counts. No
regressions in test_dict, test_dictviews, test_ordered_dict,
test_collections, test_userdict, test_iter, or test_copy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Apply cargo fmt to dict.rs imports

Collapse the builtins import block left multi-line after removing the
now-unused builtins_reversed import.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Unmark TestSyncManagerTypes.test_dict as expectedFailure

The dict iterator __reduce__ fix makes SyncManager dict proxies pickle
correctly, so this test now passes under spawn and forkserver. Remove the
stale expectedFailure marker that was causing an UNEXPECTED SUCCESS CI
failure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai coderabbitai Bot mentioned this pull request Aug 4, 2026
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dict / dict-view iterators lose position when pickled (__reduce__ ignores consumed items)

2 participants

@zzarbttoo @youknowone