mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 10:44:36 +08:00
Track live entries in reverse dictionary iterators (#9046)
* Track live entries in reverse dictionary iterators Extract the Python 3.14-compatible remaining-count and same-size storage handling from #8954. Preserve the older deferred exhaustion and extra-yield contract, using explicit wrapping arithmetic rather than the Python 3.15 extra-yield error and cursor clamp. Cover deleted-slot hints and mutation after the last yield in the existing Python snippet. Assisted-by: OpenAI Codex:model identifier unavailable * Enable the repaired reverse-dict restoration regression The patched native interpreter unexpectedly passes the existing reverse-dict clear/restore test. Remove only its obsolete expected-failure decorator, preserving the canonical assertions and all other markers. Assisted-by: OpenAI Codex:model identifier unavailable * Preserve reverse iterator invalidation on atomic relocation Assisted-by: OpenAI Codex:model identifier unavailable
This commit is contained in:
1 parent
b63c3b0134
commit
1889ee627f
4 files changed
+56
-18
No files matched your search
Vendored
-1
@@ -1405,7 +1405,6 @@ class DictTest(unittest.TestCase):
|
||||
self.assertEqual(list(reversed(A(1, 0).__dict__)), ['x'])
|
||||
self.assertEqual(list(reversed(A(0, 1).__dict__)), ['y'])
|
||||
|
||||
@unittest.expectedFailure # TODO: RUSTPYTHON; RuntimeError: dictionary changed size during iteration
|
||||
def test_reversed_dict_after_clear_and_restore(self):
|
||||
d = {}
|
||||
for i in range(1000):
|
||||
|
||||
@@ -1464,6 +1464,7 @@ macro_rules! dict_view {
|
||||
pub(crate) size: dict_inner::DictSize,
|
||||
/// As in `$iter_name`.
|
||||
changed: PyAtomic<bool>,
|
||||
remaining: PyAtomic<usize>,
|
||||
internal: PyMutex<PositionIterInternal<PyDictRef>>,
|
||||
}
|
||||
|
||||
@@ -1477,8 +1478,10 @@ macro_rules! dict_view {
|
||||
impl $reverse_iter_name {
|
||||
fn new(dict: PyDictRef) -> Self {
|
||||
let size = dict.size();
|
||||
let position = size.entries_size.saturating_sub(1);
|
||||
// usize::MAX represents the position before the first entry.
|
||||
let position = size.entries_size.wrapping_sub(1);
|
||||
$reverse_iter_name {
|
||||
remaining: Radium::new(size.used),
|
||||
size,
|
||||
changed: Radium::new(false),
|
||||
internal: PyMutex::new(PositionIterInternal::new(dict, position)),
|
||||
@@ -1520,8 +1523,10 @@ macro_rules! dict_view {
|
||||
}
|
||||
let internal = self.internal.lock();
|
||||
match &internal.status {
|
||||
IterStatus::Active(dict) if dict.size() == self.size => {
|
||||
internal.rev_length_hint(|_| self.size.entries_size)
|
||||
IterStatus::Active(dict)
|
||||
if dict.size().has_same_used_and_relocations(&self.size) =>
|
||||
{
|
||||
self.remaining.load(Ordering::Relaxed)
|
||||
}
|
||||
_ => 0,
|
||||
}
|
||||
@@ -1553,13 +1558,13 @@ macro_rules! dict_view {
|
||||
(Err(mutated()), None)
|
||||
}
|
||||
Ok(Some((found_index, item))) => {
|
||||
let released = if found_index == 0 {
|
||||
internal.exhaust()
|
||||
} else {
|
||||
internal.position = found_index - 1;
|
||||
None
|
||||
};
|
||||
(Ok(PyIterReturn::Return(($result_fn)(vm, item))), released)
|
||||
// CPython 3.14 exposes the remaining count as size_t,
|
||||
// even after same-size key replacement yields extra entries.
|
||||
let remaining = zelf.remaining.load(Ordering::Relaxed);
|
||||
zelf.remaining
|
||||
.store(remaining.wrapping_sub(1), Ordering::Relaxed);
|
||||
internal.position = found_index.wrapping_sub(1);
|
||||
(Ok(PyIterReturn::Return(($result_fn)(vm, item))), None)
|
||||
}
|
||||
Ok(None) => (Ok(PyIterReturn::StopIteration(None)), internal.exhaust()),
|
||||
}
|
||||
|
||||
@@ -184,6 +184,8 @@ impl IndexEntry {
|
||||
struct DictInner<T> {
|
||||
used: usize,
|
||||
filled: usize,
|
||||
// Only private atomic relocations invalidate same-size reverse iteration.
|
||||
relocation_epoch: u64,
|
||||
indices: Vec<IndexEntry>,
|
||||
entries: Vec<Option<DictEntry<T>>>,
|
||||
}
|
||||
@@ -210,7 +212,7 @@ impl<T: Clone> Clone for Dict<T> {
|
||||
}
|
||||
// Keep dense copies cheap, but do not carry mostly empty storage into
|
||||
// a new dictionary. pop_back can leave oversized indices without holes.
|
||||
let inner = if inner.used >= inner.entries.len() - inner.entries.len() / 3
|
||||
let mut inner = if inner.used >= inner.entries.len() - inner.entries.len() / 3
|
||||
&& (inner.indices.len() <= 8 || inner.used >= inner.indices.len() / 4)
|
||||
{
|
||||
inner.clone()
|
||||
@@ -218,6 +220,7 @@ impl<T: Clone> Clone for Dict<T> {
|
||||
let mut copy = DictInner {
|
||||
used: inner.used,
|
||||
filled: inner.used,
|
||||
relocation_epoch: 0,
|
||||
indices: Vec::new(),
|
||||
entries: Vec::with_capacity(inner.used),
|
||||
};
|
||||
@@ -226,6 +229,7 @@ impl<T: Clone> Clone for Dict<T> {
|
||||
copy.resize(inner.used * 2);
|
||||
copy
|
||||
};
|
||||
inner.relocation_epoch = 0;
|
||||
Self {
|
||||
inner: PyRwLock::new(inner),
|
||||
keys_version: AtomicU32::new(0),
|
||||
@@ -239,6 +243,7 @@ impl<T> Default for Dict<T> {
|
||||
inner: PyRwLock::new(DictInner {
|
||||
used: 0,
|
||||
filled: 0,
|
||||
relocation_epoch: 0,
|
||||
indices: vec![IndexEntry::FREE; 8],
|
||||
entries: Vec::new(),
|
||||
}),
|
||||
@@ -262,9 +267,16 @@ pub struct DictSize {
|
||||
pub entries_size: usize,
|
||||
pub used: usize,
|
||||
filled: usize,
|
||||
relocation_epoch: u64,
|
||||
}
|
||||
|
||||
/// The dict was resized under an iterator holding an older [`DictSize`].
|
||||
impl DictSize {
|
||||
pub(crate) fn has_same_used_and_relocations(&self, other: &Self) -> bool {
|
||||
self.used == other.used && self.relocation_epoch == other.relocation_epoch
|
||||
}
|
||||
}
|
||||
|
||||
/// The dict changed under an iterator holding an older [`DictSize`].
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct DictChanged;
|
||||
|
||||
@@ -387,6 +399,7 @@ impl<T> DictInner<T> {
|
||||
entries_size: self.entries.len(),
|
||||
used: self.used,
|
||||
filled: self.filled,
|
||||
relocation_epoch: self.relocation_epoch,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -856,10 +869,12 @@ impl<T: Clone> Dict<T> {
|
||||
|
||||
/// Install an owned table, releasing the old entries after unlocking.
|
||||
pub(crate) fn replace_contents(&self, other: Self) {
|
||||
let replacement = other.inner.into_inner();
|
||||
let mut replacement = other.inner.into_inner();
|
||||
let _removed = {
|
||||
let mut inner = self.write();
|
||||
self.invalidate_keys_version();
|
||||
// Relocation history belongs to this dict, not the incoming table.
|
||||
replacement.relocation_epoch = inner.relocation_epoch;
|
||||
core::mem::replace(&mut *inner, replacement)
|
||||
};
|
||||
}
|
||||
@@ -1082,7 +1097,8 @@ impl<T: Clone> Dict<T> {
|
||||
}
|
||||
}
|
||||
|
||||
/// [`Self::next_entry_checked`] in reverse.
|
||||
/// Step backward, validating the live size and private relocation history
|
||||
/// under the same read guard. Ordinary same-size mutations may change layout.
|
||||
pub(crate) fn prev_entry_checked<R>(
|
||||
&self,
|
||||
mut position: EntryIndex,
|
||||
@@ -1090,7 +1106,7 @@ impl<T: Clone> Dict<T> {
|
||||
project: impl FnOnce(&PyObject, &T) -> R,
|
||||
) -> Result<Option<(usize, R)>, DictChanged> {
|
||||
let inner = self.read();
|
||||
if inner.size() != *old {
|
||||
if !inner.size().has_same_used_and_relocations(old) {
|
||||
return Err(DictChanged);
|
||||
}
|
||||
loop {
|
||||
@@ -1471,6 +1487,7 @@ impl<T: Clone> Dict<T> {
|
||||
// this critical section.
|
||||
inner.entries.reserve(1);
|
||||
self.invalidate_keys_version();
|
||||
inner.relocation_epoch = inner.relocation_epoch.wrapping_add(1);
|
||||
let entry = inner.entries[entry_index].take().unwrap();
|
||||
let new_index = inner.entries.len();
|
||||
inner.indices[entry.index] = unsafe {
|
||||
@@ -1482,8 +1499,6 @@ impl<T: Clone> Dict<T> {
|
||||
if holes >= 2 && holes >= inner.used {
|
||||
// Moves do not increase `filled`, so ordinary insertion's
|
||||
// resize threshold cannot bound their accumulated holes.
|
||||
// Two holes also ensure this operation changes DictSize,
|
||||
// allowing existing iterators to detect the relocation.
|
||||
let indices_size = inner.indices.len();
|
||||
inner.resize(indices_size);
|
||||
}
|
||||
|
||||
@@ -378,6 +378,25 @@ assert it.__length_hint__() == 0
|
||||
assert_raises(StopIteration, next, it)
|
||||
assert it.__length_hint__() == 0
|
||||
|
||||
# Reverse hints count live entries, including when storage has deleted slots.
|
||||
for reverse_iter in (
|
||||
reversed,
|
||||
lambda d: reversed(d.keys()),
|
||||
lambda d: reversed(d.values()),
|
||||
lambda d: reversed(d.items()),
|
||||
):
|
||||
d = {0: 0, 1: 1, 2: 2, 3: 3}
|
||||
del d[1]
|
||||
it = reverse_iter(d)
|
||||
for remaining in (3, 2, 1):
|
||||
assert it.__length_hint__() == remaining
|
||||
next(it)
|
||||
assert it.__length_hint__() == 0
|
||||
# Exhaustion is observed by next(), rather than the final successful yield.
|
||||
d[4] = 4
|
||||
assert_raises(RuntimeError, next, it)
|
||||
assert it.__length_hint__() == 0
|
||||
|
||||
# Test dictionary unpacking with non-mapping objects
|
||||
# This should raise TypeError for non-mapping objects
|
||||
with assert_raises(TypeError) as cm:
|
||||
|
||||
Reference in new issue
Block a user