mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 18:59:04 +08:00
dict iterator __reduce__ to resume from current position (#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>
This commit is contained in:
1 parent
f0708f5829
commit
f8c66d02b6
3 files changed
+34
-20
No files matched your search
Vendored
-1
@@ -6791,7 +6791,6 @@ class TestSyncManagerTypes(unittest.TestCase):
|
||||
obj.clear()
|
||||
case.assertEqual(len(obj), 0)
|
||||
|
||||
@unittest.expectedFailure # TODO: RUSTPYTHON
|
||||
def test_dict(self):
|
||||
o = self.manager.dict()
|
||||
o['foo'] = 5
|
||||
|
||||
@@ -7,11 +7,7 @@ use crate::object::{Traverse, TraverseFn};
|
||||
use crate::{
|
||||
AsObject, Context, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
|
||||
TryFromObject, atomic_func,
|
||||
builtins::{
|
||||
PyTuple,
|
||||
iter::{builtins_iter, builtins_reversed},
|
||||
type_::PyAttributes,
|
||||
},
|
||||
builtins::{PyTuple, iter::builtins_iter, type_::PyAttributes},
|
||||
class::{PyClassDef, PyClassImpl},
|
||||
common::ascii,
|
||||
dict_inner::{self, DictKey},
|
||||
@@ -1118,10 +1114,17 @@ macro_rules! dict_view {
|
||||
let iter = builtins_iter(vm);
|
||||
let internal = self.internal.lock();
|
||||
let entries = match &internal.status {
|
||||
IterStatus::Active(dict) => dict
|
||||
.into_iter()
|
||||
.map(|(key, value)| ($result_fn)(vm, key, value))
|
||||
.collect::<Vec<_>>(),
|
||||
IterStatus::Active(dict) => {
|
||||
let mut position = internal.position;
|
||||
let mut entries = Vec::new();
|
||||
while let Some((next_position, key, value)) =
|
||||
dict.entries.next_entry(position)
|
||||
{
|
||||
entries.push(($result_fn)(vm, key, value));
|
||||
position = next_position;
|
||||
}
|
||||
entries
|
||||
}
|
||||
IterStatus::Exhausted => vec![],
|
||||
};
|
||||
vm.new_tuple((iter, (vm.ctx.new_list(entries),)))
|
||||
@@ -1184,14 +1187,23 @@ macro_rules! dict_view {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
let iter = builtins_reversed(vm);
|
||||
let iter = builtins_iter(vm);
|
||||
let internal = self.internal.lock();
|
||||
// TODO: entries must be reversed too
|
||||
let entries = match &internal.status {
|
||||
IterStatus::Active(dict) => dict
|
||||
.into_iter()
|
||||
.map(|(key, value)| ($result_fn)(vm, key, value))
|
||||
.collect::<Vec<_>>(),
|
||||
IterStatus::Active(dict) => {
|
||||
let mut position = internal.position;
|
||||
let mut entries = Vec::new();
|
||||
while let Some((found_index, key, value)) =
|
||||
dict.entries.prev_entry(position)
|
||||
{
|
||||
entries.push(($result_fn)(vm, key, value));
|
||||
if found_index == 0 {
|
||||
break;
|
||||
}
|
||||
position = found_index - 1;
|
||||
}
|
||||
entries
|
||||
}
|
||||
IterStatus::Exhausted => vec![],
|
||||
};
|
||||
vm.new_tuple((iter, (vm.ctx.new_list(entries),)))
|
||||
@@ -1218,11 +1230,11 @@ macro_rules! dict_view {
|
||||
);
|
||||
}
|
||||
match dict.entries.prev_entry(internal.position) {
|
||||
Some((position, key, value)) => {
|
||||
if internal.position == position {
|
||||
Some((found_index, key, value)) => {
|
||||
if found_index == 0 {
|
||||
internal.status = IterStatus::Exhausted;
|
||||
} else {
|
||||
internal.position = position;
|
||||
internal.position = found_index - 1;
|
||||
}
|
||||
PyIterReturn::Return(($result_fn)(vm, key, value))
|
||||
}
|
||||
|
||||
@@ -861,10 +861,13 @@ impl<T: Clone> Dict<T> {
|
||||
let inner = self.read();
|
||||
loop {
|
||||
let entry = inner.entries.get(position)?;
|
||||
position = position.saturating_sub(1);
|
||||
if let Some(entry) = entry {
|
||||
break Some((position, entry.key.clone(), entry.value.clone()));
|
||||
}
|
||||
if position == 0 {
|
||||
break None;
|
||||
}
|
||||
position -= 1;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user