Fix marshal recursive reference loading - #8501
Conversation
Create reference-tracked containers before reading their children so recursive list, dict, set, and tuple graphs can be unmarshaled. Preserve interned string markers through the runtime bag and add an initialization-only tuple construction path. Assisted-by: Codex:gpt-5
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_marshal.py (TODO: 8) dependencies: dependent tests: (25 tests)
Legend:
|
Keep Python exceptions raised while constructing unmarshaled sets, frozensets, and dictionaries instead of collapsing them into ValueError. This makes abnormal recursive hash-container streams report TypeError like CPython and removes the remaining test_marshal expected failure. Assisted-by: Codex:gpt-5
Summary
Create reference-tracked containers before unmarshalling their children, matching CPython's
r_object()construction order. This restores recursive list and dict graphs as well as indirect tuple cycles.The compiler-core decoder exposes optional placeholder/fill hooks, while the VM bag supplies RustPython object construction. Tuple storage gains an initialization-only mutation path corresponding to
PyTuple_Newfollowed byPyTuple_SET_ITEM; tuples remain immutable after decoding. Interned marshal string markers are also preserved through the runtime bag.Python exceptions raised while inserting unmarshaled set and dictionary members are retained in a call-local pending-error slot. Abnormal self-referential hash containers therefore raise
TypeErrorlike CPython instead of being collapsed into a genericValueError; immutable reference loops continue to raiseValueError. This also removes the remainingtest_loads_abnormal_reference_loopsexpected failure and replaces insertion-pathunwrap()calls with ordinary error propagation.AI assistance disclosure: Codex (gpt-5) assisted with implementation, test execution, and drafting this pull request. The changes were exercised with the RustPython interpreter and full project test commands listed below.
Testing
cargo fmt --checkprek run --all-filescargo run --release -- -m test test_marshalcargo test -p rustpython-compiler-core -p rustpython-vmcargo clippy -p rustpython-compiler-core -p rustpython-vm --all-targetscargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi(cd crates/capi && cargo test)cargo check --offline -p pyre-interpreter --features dynasm.