Skip to content

Preserve __dict__ and __slots__ state in deque.__reduce__#7699

Merged
youknowone merged 1 commit into
RustPython:mainfrom
changjoon-park:fix-deque-reduce-preserve-dict
Apr 27, 2026
Merged

Preserve __dict__ and __slots__ state in deque.__reduce__#7699
youknowone merged 1 commit into
RustPython:mainfrom
changjoon-park:fix-deque-reduce-preserve-dict

Conversation

@changjoon-park

@changjoon-park changjoon-park commented Apr 27, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Background

pickle.dumps(d) calls d.__reduce__(), which returns a 5-tuple (cls, args, state, listiter, dictiter). The state slot is meant to carry whatever __setstate__ (or the default attribute-restore path) needs to reconstruct subclass-specific data: typically __dict__, or (__dict__, slots_dict) when __slots__ is in play.

RustPython's deque.__reduce__ passed None for state, so any attribute set on a deque subclass was lost across a pickle round-trip. CPython's deque___reduce___impl (Modules/_collectionsmodule.c::deque___reduce___impl) calls _PyObject_GetState to get the right value.

Repro

import pickle
from collections import deque

class D(deque): pass
class DS(deque):
    __slots__ = ('x', 'y', '__dict__')

for cls in D, DS:
    d = cls('abc')
    d.x = ['x']  # slot for DS, dict for D
    d.z = ['z']  # always dict
    e = pickle.loads(pickle.dumps(d))

# CPython 3.14 (and now RustPython): e.x and e.z preserved.
# RustPython before this PR: AttributeError on e.x.

Fix

Replace the vm.ctx.none() placeholder with the result of __getstate__() on the instance. object.__getstate__ already handles both __dict__ and the dict/slots tuple form, matching CPython's _PyObject_GetState. No code duplication.

Tests unmasked

  • test_deque.TestSubclass.test_copy_pickle

test_pickle_recursive still expected-fails for an unrelated recursion issue and stays marked.

Verification

  • 10 modules pass with no regressions: test_deque test_collections test_pickle test_copy test_dataclasses test_descr test_class test_typing test_inspect test_functools (2,562 tests)
  • CPython 3.14.4 byte-identical for the repro on both subclass shapes
  • copy.copy(d) continues to drop attributes (matches CPython — the __copy__ path doesn't go through __reduce__)
  • copy.deepcopy(d) preserves attributes via the new __reduce__
  • Plain deque, deque(maxlen=...), and empty deque round-trip unchanged

Summary by CodeRabbit

  • Bug Fixes
    • Improved serialization handling for collection data structures to properly preserve state during pickling and unpickling operations.

@coderabbitai

coderabbitai Bot commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 9e73787e-23d5-40a0-a856-758bab055d45

📥 Commits

Reviewing files that changed from the base of the PR and between 2458408 and 0dea5c4.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_deque.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/vm/src/stdlib/_collections.rs
📝 Walkthrough

Walkthrough

The PyDeque::__reduce__ method now includes the deque's state from __getstate__() in the pickle reduction tuple, replacing the previous None value. This enables proper state serialization during pickling operations.

Changes

Cohort / File(s) Summary
PyDeque Pickle State Serialization
crates/vm/src/stdlib/_collections.rs		 Modified PyDeque::__reduce__ to call __getstate__() and include its result as the third element of the pickle reduction tuple, enabling state preservation during pickling. Error propagation from __getstate__() is now handled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A deque now remembers its precious state,
No more empty pickles to seal its fate!
With __getstate__ whispering what to keep,
Like secrets in my burrow, buried deep! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: replacing None with getstate() result in deque.reduce to preserve dict and slots state during pickling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@github-actions

github-actions Bot commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

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

[ ] lib: cpython/Lib/collections
[x] lib: cpython/Lib/_collections_abc.py
[x] test: cpython/Lib/test/test_collections.py (TODO: 2)
[x] test: cpython/Lib/test/test_deque.py (TODO: 2)
[ ] test: cpython/Lib/test/test_defaultdict.py (TODO: 1)
[x] test: cpython/Lib/test/test_ordered_dict.py (TODO: 8)

dependencies:

  • collections (native: _collections, _weakref, itertools, sys)
    • _collections_abc
    • warnings (native: _contextvars, _thread, _warnings, builtins, sys)
    • _collections_abc, abc, annotationlib, copy, heapq, keyword, operator, reprlib

dependent tests: (302 tests)

  • collections: test_annotationlib test_array test_asyncio test_bisect test_builtin test_c_locale_coercion test_call test_collections test_configparser test_contains test_copy test_csv test_ctypes test_defaultdict test_deque test_descr test_dict test_dictviews test_enum test_exception_group test_file test_fileinput test_fileio test_frame test_funcattrs test_functools test_genericalias test_hash test_httpservers test_inspect test_io test_ipaddress test_iter test_iterlen test_json test_logging test_math test_monitoring test_ordered_dict test_pathlib test_patma test_pickle test_plistlib test_pprint test_pydoc test_random test_reprlib test_richcmp test_set test_shelve test_sqlite3 test_statistics test_string test_struct test_sys test_traceback test_tuple test_types test_typing test_unittest test_urllib test_userdict test_userlist test_userstring test_weakref test_weakset test_with
    • ast: test_ast test_compile test_compiler_codegen test_dis test_fstring test_future_stmt test_site test_ssl test_type_comments test_ucn test_unparse
      • annotationlib: test_type_annotations test_type_params
      • dbm.dumb: test_dbm_dumb
      • inspect: test_abc test_argparse test_asyncgen test_buffer test_code test_coroutines test_decimal test_generators test_grammar test_ntpath test_operator test_posixpath test_signal test_yield_from test_zipimport test_zoneinfo
      • pyclbr: test_pyclbr
      • traceback: test_asyncio test_code_module test_contextlib test_contextlib_async test_dictcomps test_exceptions test_http_cookiejar test_importlib test_listcomps test_pyexpat test_setcomps test_socket test_subprocess test_threadedtempfile test_threading test_unittest
    • asyncio: test_asyncio test_os test_sys_settrace
    • concurrent.futures._base: test_concurrent_futures
    • dbm.sqlite3: test_dbm_sqlite3
    • difflib: test_difflib
    • dis: test__opcode test_compiler_assemble test_dtrace test_opcache test_peepholer test_positional_only_arg
      • bdb: test_bdb
      • modulefinder: test_importlib test_modulefinder
      • trace: test_trace
    • email.feedparser: test_email
    • http.client: test_docxmlrpc test_hashlib test_unicodedata test_urllib2 test_wsgiref test_xmlrpc
      • urllib.request: test_sax test_urllib2_localnet test_urllib2net test_urllibnet
    • importlib.metadata: test_importlib
    • inspect:
      • cmd: test_cmd
      • dataclasses: test__colorize test_ctypes test_regrtest
      • pkgutil: test_pkgutil test_runpy
      • rlcompleter: test_rlcompleter
    • logging: test_support
      • hashlib: test_hmac test_smtplib test_tarfile
      • multiprocessing.util: test_compileall test_concurrent_futures
      • venv: test_venv
    • multiprocessing: test_fcntl test_memoryview test_multiprocessing_main_handling test_re
    • platform: test__locale test__osx_support test_android test_baseexception test_cmath test_ctypes test_mimetypes test_platform test_posix test_shutil test_sysconfig test_time test_winreg
    • pprint: test_htmlparser test_sys_setprofile
      • pickle: test_bool test_bytes test_bz2 test_codecs test_concurrent_futures test_ctypes test_email test_enumerate test_fractions test_http_cookies test_itertools test_list test_lzma test_memoryio test_minidom test_picklebuffer test_pickletools test_range test_slice test_str test_type_aliases test_unittest test_uuid test_xml_dom_minicompat test_xml_etree test_zipfile test_zlib test_zoneinfo
    • queue: test_dummy_thread test_sched
    • selectors: test_selectors
      • socket: test_epoll test_exception_hierarchy test_ftplib test_httplib test_imaplib test_kqueue test_largefile test_mailbox test_mmap test_poplib test_pty test_smtpnet test_socketserver test_stat test_timeout test_urllib_response
      • subprocess: test_atexit test_audit test_cmd_line test_cmd_line_script test_ctypes test_faulthandler test_file_eintr test_gc test_gzip test_json test_launcher test_msvcrt test_osx_env test_poll test_py_compile test_quopri test_repl test_script_helper test_select test_tempfile test_unittest test_utf8_mode test_wait3 test_webbrowser test_zipfile
    • shlex: test_shlex
    • shutil: test_filecmp test_glob test_importlib test_string_literals test_unicode_file
      • ctypes.util: test_ctypes
      • ensurepip: test_ensurepip
      • pathlib: test_importlib test_pathlib test_tomllib test_tools test_winapi test_zipapp test_zstd
      • tempfile: test_doctest test_importlib test_linecache test_pkg test_tabnanny test_termios test_tokenize test_winconsoleio test_zipfile64
      • zipfile: test_zipfile
    • statistics:
      • random: test_complex test_context test_devpoll test_float test_heapq test_int test_long test_numeric_tower test_pow test_queue test_sort test_strtod test_thread
    • string: test_email test_fnmatch test_secrets test_string
    • threading: test_concurrent_futures test_ctypes test_fork1 test_importlib test_robotparser test_super test_syslog test_threading_local
      • dummy_threading: test_dummy_threading
      • sysconfig: test_tools
    • traceback:
      • timeit: test_timeit
    • urllib.parse: test_urlparse
    • wave: test_wave

Legend:

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

@changjoon-park
Preserve __dict__ and __slots__ state in deque.__reduce__
0dea5c4 
deque.__reduce__ passed None as the unpickle state, so a deque
subclass's __dict__ and __slots__ values were dropped across a pickle
round-trip. CPython's deque___reduce___impl
(Modules/_collectionsmodule.c::deque___reduce___impl) calls
_PyObject_GetState, which returns the dict (or a (dict, slots) tuple)
so subclass attributes survive.

Replace the placeholder with the result of __getstate__() on the
instance. object.__getstate__ already implements the dict / dict+slots
protocol, matching _PyObject_GetState.
@changjoon-park changjoon-park force-pushed the fix-deque-reduce-preserve-dict branch from 2458408 to 0dea5c4 Compare April 27, 2026 12:15
ShaharNaveh
ShaharNaveh approved these changes Apr 27, 2026
View reviewed changes

@ShaharNaveh ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@youknowone youknowone enabled auto-merge (squash) April 27, 2026 12:40
@youknowone youknowone merged commit 1d42ee5 into RustPython:main Apr 27, 2026
21 checks passed
@changjoon-park changjoon-park deleted the fix-deque-reduce-preserve-dict branch April 27, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

3 participants

@changjoon-park @ShaharNaveh @youknowone