From 4d40ae7424aa793b68923baf796b172e071f6ebc Mon Sep 17 00:00:00 2001 From: dot Date: Wed, 4 Nov 2026 16:46:35 +0900 Subject: [PATCH 1/6] Align set operand and hashing controls with CPython Convert iterable operands when each operation reaches them, share precise hash-error decoration, and preserve mutable-set keys for fallback lookup. Reuse stored hashes according to each operation and align symmetric difference comparison order while retaining reentrancy-safe table updates. Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/builtins/dict.rs | 18 +- crates/vm/src/builtins/set.rs | 459 ++++++++++++++------------------- crates/vm/src/dict_inner.rs | 42 ++- 3 files changed, 222 insertions(+), 297 deletions(-) diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 959b4983437..aa16ce0890d 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -2401,14 +2401,16 @@ impl AsSequence for PyDictValues { fn set_inner_number_op(a: &PyObject, b: &PyObject, f: F, vm: &VirtualMachine) -> PyResult where - F: FnOnce(PySetInner, ArgIterable) -> PyResult, + F: FnOnce(PySetInner, PyObjectRef) -> PyResult, { let a = PySetInner::from_iter( ArgIterable::try_from_object(vm, a.to_owned())?.iter(vm)?, vm, )?; - let b = ArgIterable::try_from_object(vm, b.to_owned())?; - Ok(PySet { inner: f(a, b)? }.into_pyobject(vm)) + Ok(PySet { + inner: f(a, b.to_owned())?, + } + .into_pyobject(vm)) } pub(crate) fn set_inner_number_subtract( @@ -2443,7 +2445,15 @@ pub(crate) fn set_view_number_and( } pub(crate) fn set_inner_number_xor(a: &PyObject, b: &PyObject, vm: &VirtualMachine) -> PyResult { - set_inner_number_op(a, b, |a, b| a.symmetric_difference(b, vm), vm) + set_inner_number_op( + a, + b, + |a, b| { + a.symmetric_difference_update(core::iter::once(b), vm)?; + Ok(a) + }, + vm, + ) } pub(crate) fn set_item_view_number_xor( diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 8f289d1e2e9..b30f1c96a3a 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -2,8 +2,8 @@ * Builtin set type with a sequence of unique items. */ use super::{ - IterStatus, PositionIterInternal, PyDict, PyGenericAlias, PyTupleRef, PyType, PyTypeRef, - builtins_iter, locked_step, + IterStatus, PositionIterInternal, PyAnyDict, PyBaseExceptionRef, PyDict, PyGenericAlias, + PyTupleRef, PyType, PyTypeRef, builtins_iter, locked_step, }; use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, @@ -18,10 +18,7 @@ use crate::{ }, convert::{ToPyResult, TransmuteFromObject}, dict_inner::{self, DictSize}, - function::{ - ArgIterable, FuncArgs, NameOthers, OptionalArg, PosArgs, PyArithmeticValue, - PyComparisonValue, - }, + function::{FuncArgs, NameOthers, OptionalArg, PosArgs, PyArithmeticValue, PyComparisonValue}, object::{Traverse, TraverseFn}, protocol::{PyIterReturn, PyNumberMethods, PySequenceMethods}, recursion::ReprGuard, @@ -87,27 +84,13 @@ impl PySet { self.inner.elements() } - fn fold_op( - &self, - others: impl core::iter::Iterator, - op: fn(&PySetInner, ArgIterable, &VirtualMachine) -> PyResult, - vm: &VirtualMachine, - ) -> PyResult { - Ok(Self { - inner: self.inner.fold_op(others, op, vm)?, - }) - } - - fn op( + fn union_multi( &self, - other: PyRef, - op: fn(&PySetInner, ArgIterable, &VirtualMachine) -> PyResult, + others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult { Ok(Self { - inner: self - .inner - .fold_op(core::iter::once(other.into_iterable(vm)?), op, vm)?, + inner: self.inner.union_multi(others, vm)?, }) } } @@ -164,28 +147,13 @@ impl PyFrozenSet { self.inner.elements() } - fn fold_op( - &self, - others: impl core::iter::Iterator, - op: fn(&PySetInner, ArgIterable, &VirtualMachine) -> PyResult, - vm: &VirtualMachine, - ) -> PyResult { - Ok(Self { - inner: self.inner.fold_op(others, op, vm)?, - ..Default::default() - }) - } - - fn op( + fn union_multi( &self, - other: PyRef, - op: fn(&PySetInner, ArgIterable, &VirtualMachine) -> PyResult, + others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult { Ok(Self { - inner: self - .inner - .fold_op(core::iter::once(other.into_iterable(vm)?), op, vm)?, + inner: self.inner.union_multi(others, vm)?, ..Default::default() }) } @@ -277,7 +245,7 @@ impl PySetInner { } /// Build a set from an arbitrary object, reusing stored hashes when the - /// source is a set/frozenset/dict. + /// source is a set/frozenset/dict/frozendict. fn from_object(iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult { let set = Self::default(); set.update_internal(iterable, vm)?; @@ -286,32 +254,42 @@ impl PySetInner { /// Elements of `obj` with their stored hashes, or `None` if `obj` keeps /// none and must be iterated generically. Mirrors the `PyAnySet_Check` / - /// `PyDict_CheckExact` fast paths in CPython's `set_update_internal`. + /// `PyAnyDict_CheckExact` fast paths in CPython's `set_update_internal`. fn cached_hashes(obj: &PyObject, vm: &VirtualMachine) -> Option> { if let Some(set) = extract_set(obj) { Some(set.content.keys_with_hashes()) } else { - obj.downcast_ref_if_exact::(vm) - .map(|dict| dict._as_dict_inner().keys_with_hashes()) + Self::exact_dict(obj, vm).map(|dict| dict._as_dict_inner().keys_with_hashes()) + } + } + + fn exact_dict<'a>(obj: &'a PyObject, vm: &VirtualMachine) -> Option<&'a PyDict> { + if obj.class().is(vm.ctx.types.dict_type) || obj.class().is(vm.ctx.types.frozendict_type) { + PyAnyDict::from_object(obj).map(PyAnyDict::as_dict) + } else { + None } } - fn fold_op( + fn union_multi( &self, - others: impl core::iter::Iterator, - op: fn(&Self, O, &VirtualMachine) -> PyResult, + others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult { - let mut res = self.copy(); + let res = self.copy(); for other in others { - res = op(&res, other, vm)?; + if extract_set(&other).is_some_and(|other| PyRc::ptr_eq(&self.content, &other.content)) + { + continue; + } + res.update_internal(other, vm)?; } Ok(res) } fn intersection_multi( &self, - mut others: impl core::iter::Iterator, + mut others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult { let Some(other) = others.next() else { @@ -326,7 +304,7 @@ impl PySetInner { fn difference_multi( &self, - mut others: impl core::iter::Iterator, + mut others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult { let Some(other) = others.next() else { @@ -352,11 +330,10 @@ impl PySetInner { } fn contains(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { - let result = self.retry_op_with_frozenset(needle, vm, |needle, vm| { - let hash = needle.hash(vm)?; - self.content.contains(vm, needle, hash) - }); - Self::wrap_unhashable_error(result, needle, vm) + let hash = needle.hash(vm).or_else(|exc| { + Self::mutable_set_hash(needle, exc, vm).map_err(|exc| Self::hash_error(needle, exc, vm)) + })?; + self.content.contains(vm, needle, hash) } /// Look up a key whose hash is already known, without a frozenset retry. @@ -390,28 +367,17 @@ impl PySetInner { Ok(true) } - pub(super) fn union(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { - let set = self.clone(); - if let Some(elements) = Self::cached_hashes(other.as_object(), vm) { - for (item, hash) in elements { - set.add_known_hash(&item, hash, vm)?; - } - return Ok(set); - } - for item in other.iter(vm)? { - let item = item?; - set.add(&item, vm)?; - } - - Ok(set) + pub(super) fn union(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.update_internal(other, vm)?; + Ok(self.clone()) } - pub(super) fn intersection(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + pub(super) fn intersection(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other_set) = extract_set(other.as_object()) { return self.intersection_set(other_set, vm); } let set = Self::default(); - for item in other.iter(vm)? { + for item in other.get_iter(vm)?.into_iter::(vm) { let obj = item?; let hash = obj.hash(vm)?; if self.contains_known_hash(&obj, hash, vm)? { @@ -442,7 +408,7 @@ impl PySetInner { Ok(set) } - fn difference_new(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn difference_new(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { // Scanning the left side avoids visiting and retaining a much larger // exclusion collection. Match CPython's comparison direction as well. if let Some(other_set) = extract_set(other.as_object()) { @@ -450,7 +416,7 @@ impl PySetInner { return self .difference_by(|key, hash| other_set.contains_known_hash(key, hash, vm), vm); } - } else if let Some(dict) = other.as_object().downcast_ref_if_exact::(vm) + } else if let Some(dict) = Self::exact_dict(other.as_object(), vm) && self.len() >> 2 <= dict._as_dict_inner().len() { return self.difference_by( @@ -462,7 +428,7 @@ impl PySetInner { } // The dict-view caller supplies a private working table. - pub(super) fn difference(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + pub(super) fn difference(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.difference_update(core::iter::once(other), vm)?; Ok(self.clone()) } @@ -483,36 +449,26 @@ impl PySetInner { pub(super) fn symmetric_difference( &self, - other: ArgIterable, + other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - let new_inner = self.clone(); - - if let Some(elements) = Self::cached_hashes(other.as_object(), vm) { - // the source is already duplicate-free - for (item, hash) in elements { - new_inner.content.delete_or_insert(vm, &item, hash, ())?; - } - return Ok(new_inner); - } - - // We want to remove duplicates in other - let other_set = Self::from_iter(other.iter(vm)?, vm)?; - - for (item, hash) in other_set.content.keys_with_hashes() { - new_inner.content.delete_or_insert(vm, &item, hash, ())?; - } - - Ok(new_inner) + // Build the right operand first, so its resident keys receive equality + // comparisons and its iterator can mutate the left operand before use. + let result = match extract_set(&other) { + Some(other) => other.copy(), + None => Self::from_object(other.clone(), vm)?, + }; + result.toggle_entries(self.content.keys_with_hashes(), vm)?; + Ok(result) } - fn issuperset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn issuperset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other_set) = extract_set(other.as_object()) { return self.compare(other_set, PyComparisonOp::Ge, vm); } - for item in other.iter(vm)? { + for item in other.get_iter(vm)?.into_iter::(vm) { let item = item?; - let hash = Self::wrap_unhashable_error(item.hash(vm), &item, vm)?; + let hash = Self::key_hash(&item, vm)?; if !self.contains_known_hash(&item, hash, vm)? { return Ok(false); } @@ -520,14 +476,14 @@ impl PySetInner { Ok(true) } - fn issubset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn issubset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other_set) = extract_set(other.as_object()) { return self.compare(other_set, PyComparisonOp::Le, vm); } Ok(self.intersection(other, vm)?.len() == self.len()) } - pub(super) fn isdisjoint(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + pub(super) fn isdisjoint(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other_set) = extract_set(other.as_object()) { if core::ptr::eq(self, other_set) { return Ok(self.len() == 0); @@ -547,9 +503,9 @@ impl PySetInner { return Ok(true); } } - for item in other.iter(vm)? { + for item in other.get_iter(vm)?.into_iter::(vm) { let item = item?; - let hash = Self::wrap_unhashable_error(item.hash(vm), &item, vm)?; + let hash = Self::key_hash(&item, vm)?; if self.contains_known_hash(&item, hash, vm)? { return Ok(false); } @@ -570,17 +526,13 @@ impl PySetInner { } fn add(&self, item: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - let result = (|| { - let hash = item.hash(vm)?; - self.insert_known_hash(item, hash, vm) - })(); - Self::wrap_unhashable_error(result, item, vm) + let hash = Self::key_hash(item, vm)?; + self.add_known_hash(item, hash, vm) } /// [`Self::add`] with a known hash. fn add_known_hash(&self, item: &PyObject, hash: PyHash, vm: &VirtualMachine) -> PyResult<()> { - let result = self.insert_known_hash(item, hash, vm); - Self::wrap_unhashable_error(result, item, vm) + self.insert_known_hash(item, hash, vm) } /// Insert a key without translating exceptions from its equality callback. @@ -594,19 +546,23 @@ impl PySetInner { } fn remove(&self, item: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - let result = self.retry_op_with_frozenset(item, vm, |item, vm| { - let hash = item.hash(vm)?; - self.content.delete(vm, item, hash) - }); - Self::wrap_unhashable_error(result, item, vm) + if self.discard(item, vm)? { + Ok(()) + } else { + Err(vm.new_key_error(item.to_owned())) + } } fn discard(&self, item: &PyObject, vm: &VirtualMachine) -> PyResult { - let result = self.retry_op_with_frozenset(item, vm, |item, vm| { - let hash = item.hash(vm)?; + self.discard_key(item, vm).or_else(|exc| { + let hash = Self::mutable_set_hash(item, exc, vm)?; self.content.delete_if_exists(vm, item, hash) - }); - Self::wrap_unhashable_error(result, item, vm) + }) + } + + fn discard_key(&self, item: &PyObject, vm: &VirtualMachine) -> PyResult { + let hash = Self::key_hash(item, vm)?; + self.content.delete_if_exists(vm, item, hash) } fn clear(&self) { @@ -631,12 +587,10 @@ impl PySetInner { // check set or frozenset if let Ok(any_set) = PyRef::::try_from_object(vm, iterable.to_owned()) { self.merge_set(any_set, vm) - // check Dict - } else if let Ok(dict) = iterable.to_owned().downcast_exact::(vm) { - self.merge_dict(&dict, vm) + } else if let Some(dict) = Self::exact_dict(&iterable, vm) { + self.merge_dict(dict, vm) } else { - // add iterable that is not a set, frozenset, or dict - for item in iterable.try_into_value::(vm)?.iter(vm)? { + for item in iterable.get_iter(vm)?.into_iter::(vm) { let item = item?; self.add(&item, vm)?; } @@ -645,13 +599,16 @@ impl PySetInner { } fn merge_set(&self, any_set: PyRef, vm: &VirtualMachine) -> PyResult<()> { + if PyRc::ptr_eq(&self.content, &any_set.as_inner().content) { + return Ok(()); + } for (item, hash) in any_set.as_inner().content.keys_with_hashes() { self.add_known_hash(&item, hash, vm)?; } Ok(()) } - fn merge_dict(&self, dict: &Py, vm: &VirtualMachine) -> PyResult<()> { + fn merge_dict(&self, dict: &PyDict, vm: &VirtualMachine) -> PyResult<()> { for (key, hash) in dict._as_dict_inner().keys_with_hashes() { self.add_known_hash(&key, hash, vm)?; } @@ -660,7 +617,7 @@ impl PySetInner { fn intersection_update( &self, - others: impl core::iter::Iterator, + others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult<()> { let temp_inner = self.intersection_multi(others, vm)?; @@ -671,7 +628,7 @@ impl PySetInner { fn difference_update( &self, - others: impl core::iter::Iterator, + others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult<()> { for iterable in others { @@ -690,7 +647,7 @@ impl PySetInner { other_set.content.keys_with_hashes() }) } else { - Self::cached_hashes(iterable.as_object(), vm) + None }; if let Some(elements) = elements { for (item, hash) in elements { @@ -698,102 +655,89 @@ impl PySetInner { } continue; } - for item in iterable.iter(vm)? { + for item in iterable.get_iter(vm)?.into_iter::(vm) { let item = item?; - let hash = item.hash(vm)?; - self.content.delete_if_exists(vm, &*item, hash)?; + self.discard_key(&item, vm)?; } } Ok(()) } - fn symmetric_difference_update( + pub(super) fn symmetric_difference_update( &self, - others: impl core::iter::Iterator, + others: impl core::iter::Iterator, vm: &VirtualMachine, ) -> PyResult<()> { for iterable in others { - if let Some(elements) = Self::cached_hashes(iterable.as_object(), vm) { - // the source is already duplicate-free - for (item, hash) in elements { - self.content.delete_or_insert(vm, &item, hash, ())?; - } + if extract_set(&iterable) + .is_some_and(|other| PyRc::ptr_eq(&self.content, &other.content)) + { + self.clear(); continue; } - // We want to remove duplicates in iterable - let iterable_set = Self::from_iter(iterable.iter(vm)?, vm)?; - for (item, hash) in iterable_set.content.keys_with_hashes() { - self.content.delete_or_insert(vm, &item, hash, ())?; - } + let iterable_set; + let elements = if let Some(elements) = Self::cached_hashes(iterable.as_object(), vm) { + // the source is already duplicate-free + elements + } else { + // We want to remove duplicates in iterable + iterable_set = Self::from_object(iterable.clone(), vm)?; + iterable_set.content.keys_with_hashes() + }; + self.toggle_entries(elements, vm)?; + } + Ok(()) + } + + fn toggle_entries( + &self, + elements: Vec<(PyObjectRef, PyHash)>, + vm: &VirtualMachine, + ) -> PyResult<()> { + for (item, hash) in elements { + self.content.delete_or_insert(vm, &item, hash, ())?; } Ok(()) } fn hash(&self) -> PyHash { - let hasher = self.content.fold_hashes( - hash::FrozenSetHash::new(self.len()), - |mut hasher, element_hash| { - hasher.add(element_hash); - hasher - }, - ); + let hasher = + self.content + .fold_hashes(hash::FrozenSetHash::new, |mut hasher, element_hash| { + hasher.add(element_hash); + hasher + }); hasher.finish() } - // Run operation, on failure, if item is a set/set subclass, convert it - // into a frozenset and try the operation again. Propagates original error - // on failure to convert and restores item in KeyError on failure (remove). - fn retry_op_with_frozenset( - &self, + // Mutable-set lookup borrows the frozenset hash algorithm, not a converted + // key: comparisons must still receive the caller's original object. + fn mutable_set_hash( item: &PyObject, + exc: PyBaseExceptionRef, vm: &VirtualMachine, - op: F, - ) -> PyResult - where - F: Fn(&PyObject, &VirtualMachine) -> PyResult, - { - op(item, vm).or_else(|original_err| { - item.downcast_ref::() - // Keep original error around. - .ok_or(original_err) - .and_then(|set| { - op( - &PyFrozenSet { - inner: set.inner.copy(), - ..Default::default() - } - .into_pyobject(vm), - vm, - ) - // If operation raised KeyError, report original set (set.remove) - .map_err(|op_err| { - if op_err.fast_isinstance(vm.ctx.exceptions.key_error) { - vm.new_key_error(item.to_owned()) - } else { - op_err - } - }) - }) - }) + ) -> PyResult { + if exc.fast_isinstance(vm.ctx.exceptions.type_error) + && let Some(set) = item.downcast_ref::() + { + // Clearing the swallowed exception can mutate the lookup key. + drop(exc); + Ok(set.inner.hash()) + } else { + Err(exc) + } } - fn wrap_unhashable_error( - result: PyResult, + fn key_hash(item: &PyObject, vm: &VirtualMachine) -> PyResult { + item.hash(vm).map_err(|exc| Self::hash_error(item, exc, vm)) + } + + fn hash_error( item: &PyObject, + exc: PyBaseExceptionRef, vm: &VirtualMachine, - ) -> PyResult { - match result { - Err(cause) if cause.fast_isinstance(vm.ctx.exceptions.type_error) => { - let message = cause.as_object().str(vm)?; - let err = vm.new_type_error(format!( - "cannot use '{}' as a set element ({message})", - item.class().name() - )); - err.set_cause(Some(cause)); - Err(err) - } - result => result, - } + ) -> PyBaseExceptionRef { + super::dict::contextualize_hash_error(item, "set", "element", exc, vm) } } @@ -845,11 +789,9 @@ impl PySet { fn __or__(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { - Ok(PyArithmeticValue::Implemented(self.op( - other, - PySetInner::union, - vm, - )?)) + Ok(PyArithmeticValue::Implemented( + self.union_multi(core::iter::once(other.into_object()), vm)?, + )) } else { Ok(PyArithmeticValue::NotImplemented) } @@ -862,7 +804,7 @@ impl PySet { ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { - inner: self.inner.intersection(other.into_iterable(vm)?, vm)?, + inner: self.inner.intersection(other.into_object(), vm)?, })) } else { Ok(PyArithmeticValue::NotImplemented) @@ -876,7 +818,7 @@ impl PySet { ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { - inner: self.inner.difference_new(other.into_iterable(vm)?, vm)?, + inner: self.inner.difference_new(other.into_object(), vm)?, })) } else { Ok(PyArithmeticValue::NotImplemented) @@ -890,9 +832,7 @@ impl PySet { ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { - inner: other - .as_inner() - .difference_new(ArgIterable::try_from_object(vm, zelf.into())?, vm)?, + inner: other.as_inner().difference_new(zelf.into(), vm)?, })) } else { Ok(PyArithmeticValue::NotImplemented) @@ -905,11 +845,9 @@ impl PySet { vm: &VirtualMachine, ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { - Ok(PyArithmeticValue::Implemented(self.op( - other, - PySetInner::symmetric_difference, - vm, - )?)) + Ok(PyArithmeticValue::Implemented(Self { + inner: self.inner.symmetric_difference(other.into_object(), vm)?, + })) } else { Ok(PyArithmeticValue::NotImplemented) } @@ -931,7 +869,7 @@ impl PySet { ) -> PyResult> { if !set.is(zelf.as_object()) { zelf.inner - .intersection_update(core::iter::once(set.into_iterable(vm)?), vm)?; + .intersection_update(core::iter::once(set.into_object()), vm)?; } Ok(zelf) } @@ -945,7 +883,7 @@ impl PySet { zelf.inner.clear(); } else { zelf.inner - .difference_update(set.into_iterable_iter(vm)?, vm)?; + .difference_update(core::iter::once(set.into_object()), vm)?; } Ok(zelf) } @@ -959,7 +897,7 @@ impl PySet { zelf.inner.clear(); } else { zelf.inner - .symmetric_difference_update(set.into_iterable_iter(vm)?, vm)?; + .symmetric_difference_update(core::iter::once(set.into_object()), vm)?; } Ok(zelf) } @@ -1002,16 +940,16 @@ impl Py { #[pymethod] fn union( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult { - self.fold_op(others.into_iter(), PySetInner::union, vm) + self.union_multi(others.into_iter(), vm) } #[pymethod] fn intersection( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult { Ok(PySet { @@ -1022,7 +960,7 @@ impl Py { #[pymethod] fn difference( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult { Ok(PySet { @@ -1031,26 +969,24 @@ impl Py { } #[pymethod] - fn symmetric_difference(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { - self.fold_op( - core::iter::once(other), - PySetInner::symmetric_difference, - vm, - ) + fn symmetric_difference(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + Ok(PySet { + inner: self.inner.symmetric_difference(other, vm)?, + }) } #[pymethod] - fn issubset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn issubset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.inner.issubset(other, vm) } #[pymethod] - fn issuperset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn issuperset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.inner.issuperset(other, vm) } #[pymethod] - fn isdisjoint(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn isdisjoint(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.inner.isdisjoint(other, vm) } @@ -1094,7 +1030,7 @@ impl Py { #[pymethod] fn intersection_update( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult<()> { self.inner.intersection_update(others.into_iter(), vm)?; @@ -1104,14 +1040,14 @@ impl Py { #[pymethod] fn difference_update( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult<()> { self.inner.difference_update(others.into_iter(), vm) } #[pymethod] - fn symmetric_difference_update(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult<()> { + fn symmetric_difference_update(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { self.inner .symmetric_difference_update(core::iter::once(other), vm) } @@ -1409,11 +1345,9 @@ impl PyFrozenSet { fn __or__(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult> { if let Ok(set) = PyRef::::try_from_object(vm, other) { - Ok(PyArithmeticValue::Implemented(self.op( - set, - PySetInner::union, - vm, - )?)) + Ok(PyArithmeticValue::Implemented( + self.union_multi(core::iter::once(set.into_object()), vm)?, + )) } else { Ok(PyArithmeticValue::NotImplemented) } @@ -1426,7 +1360,7 @@ impl PyFrozenSet { ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { - inner: self.inner.intersection(other.into_iterable(vm)?, vm)?, + inner: self.inner.intersection(other.into_object(), vm)?, ..Default::default() })) } else { @@ -1441,7 +1375,7 @@ impl PyFrozenSet { ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { - inner: self.inner.difference_new(other.into_iterable(vm)?, vm)?, + inner: self.inner.difference_new(other.into_object(), vm)?, ..Default::default() })) } else { @@ -1456,9 +1390,7 @@ impl PyFrozenSet { ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { Ok(PyArithmeticValue::Implemented(Self { - inner: other - .as_inner() - .difference_new(ArgIterable::try_from_object(vm, zelf.into())?, vm)?, + inner: other.as_inner().difference_new(zelf.into(), vm)?, ..Default::default() })) } else { @@ -1472,11 +1404,10 @@ impl PyFrozenSet { vm: &VirtualMachine, ) -> PyResult> { if let Ok(other) = PyRef::::try_from_object(vm, other) { - Ok(PyArithmeticValue::Implemented(self.op( - other, - PySetInner::symmetric_difference, - vm, - )?)) + Ok(PyArithmeticValue::Implemented(Self { + inner: self.inner.symmetric_difference(other.into_object(), vm)?, + ..Default::default() + })) } else { Ok(PyArithmeticValue::NotImplemented) } @@ -1522,16 +1453,16 @@ impl Py { #[pymethod] fn union( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult { - self.fold_op(others.into_iter(), PySetInner::union, vm) + self.union_multi(others.into_iter(), vm) } #[pymethod] fn intersection( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult { Ok(PyFrozenSet { @@ -1543,7 +1474,7 @@ impl Py { #[pymethod] fn difference( &self, - others: PosArgs, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult { Ok(PyFrozenSet { @@ -1555,28 +1486,27 @@ impl Py { #[pymethod] fn symmetric_difference( &self, - other: ArgIterable, + other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - self.fold_op( - core::iter::once(other), - PySetInner::symmetric_difference, - vm, - ) + Ok(PyFrozenSet { + inner: self.inner.symmetric_difference(other, vm)?, + ..Default::default() + }) } #[pymethod] - fn issubset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn issubset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.inner.issubset(other, vm) } #[pymethod] - fn issuperset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn issuperset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.inner.issuperset(other, vm) } #[pymethod] - fn isdisjoint(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + fn isdisjoint(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { self.inner.isdisjoint(other, vm) } @@ -1806,17 +1736,6 @@ impl PyRef { fn check(obj: &PyObject, vm: &VirtualMachine) -> bool { PyAnySet::check(obj, vm) } - - fn into_iterable(self, vm: &VirtualMachine) -> PyResult { - self.into_object().try_into_value(vm) - } - - fn into_iterable_iter( - self, - vm: &VirtualMachine, - ) -> PyResult> { - Ok(core::iter::once(self.into_iterable(vm)?)) - } } // SAFETY: PyRef is a transparent PyRef owner. A successful check proves diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index 947bbb91af7..d175bad1b90 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -942,26 +942,18 @@ impl Dict { hash: HashValue, value: T, ) -> PyResult<()> { - // Borrow the value so a deletion leaves it to be dropped unlocked. - let mut value = Some(value); - let _removed = self.with_slot_mut_prepared( - vm, - key, - hash, - Some(|| key.to_owned()), - |inner, (entry, index_index), prepared| { - if let Some(index) = entry.index() { - self.invalidate_keys_version(); - inner.indices[index_index] = IndexEntry::DUMMY; - inner.used -= 1; - return inner.entries[index].take(); - } - let key = prepared.take().unwrap(); - self.invalidate_keys_version(); - inner.unchecked_push(index_index, hash, key, value.take().unwrap(), entry); - None - }, - )?; + // Set symmetric difference performs a discard followed by an add. + // The second lookup is observable through equality callbacks. + let removed = self.with_slot_mut(vm, key, hash, |inner, (entry, index_index)| { + let index = entry.index()?; + self.invalidate_keys_version(); + inner.indices[index_index] = IndexEntry::DUMMY; + inner.used -= 1; + inner.entries[index].take() + })?; + if removed.is_none() { + self.insert(vm, key, hash, value)?; + } Ok(()) } @@ -1171,12 +1163,16 @@ impl Dict { .collect() } - /// Fold stored hashes under the read lock; the callback must not run Python. - pub(crate) fn fold_hashes(&self, init: Acc, f: Fold) -> Acc + /// Fold the length and stored hashes under one read lock; neither callback + /// may run Python or release owned Python references. + pub(crate) fn fold_hashes(&self, init: Init, f: Fold) -> Acc where + Init: FnOnce(usize) -> Acc, Fold: FnMut(Acc, HashValue) -> Acc, { - self.read() + let inner = self.read(); + let init = init(inner.used); + inner .entries .iter() .filter_map(|v| v.as_ref().map(|v| v.hash)) From cb2955430f326f4e44f3fcf0bc336231a78ce1d2 Mon Sep 17 00:00:00 2001 From: dot Date: Wed, 4 Nov 2026 16:59:59 +0900 Subject: [PATCH 2/6] Keep legacy dictionary deletion wrapper test-only Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/dict_inner.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index d175bad1b90..fcc00e67f42 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -860,6 +860,7 @@ impl Dict { } /// Delete a key. Same hash contract as [`Self::insert`]. + #[cfg(test)] pub(crate) fn delete(&self, vm: &VirtualMachine, key: &K, hash: HashValue) -> PyResult<()> where K: DictKey + ?Sized, From 52066f05d26ff76ff43d25dda9376ad753148473 Mon Sep 17 00:00:00 2001 From: dot Date: Wed, 4 Nov 2026 17:40:14 +0900 Subject: [PATCH 3/6] Align dictionary view and native copy controls Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/builtins/dict.rs | 157 +++++++++++++++++++++++++-------- crates/vm/src/builtins/set.rs | 30 +++---- crates/vm/src/dict_inner.rs | 6 -- 3 files changed, 133 insertions(+), 60 deletions(-) diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index aa16ce0890d..b1bce4e9c72 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -228,7 +228,11 @@ impl PyAnyDict { self.as_dict().is_empty() } pub fn copy_as_dict(&self, vm: &VirtualMachine) -> PyResult { - if self.is_empty() || self.uses_builtin_iter(vm) { + let size = self.size(); + // Sparse copies go through merge, where equality between colliding + // keys remains observable. Dense native tables can be cloned directly. + if size.used == 0 || (self.uses_builtin_iter(vm) && size.used >= size.entries_size * 2 / 3) + { return Ok(self.as_dict().copy()); } let copied = PyDict::default(); @@ -585,18 +589,25 @@ impl PyDict { vm: &VirtualMachine, ) -> PyResult<()> { let dict = &self.entries; - let dict_size = &dict_other.size(); + let source_len = dict_other.__len__(); let mut position = 0; while let Some((next, key, value, hash)) = dict_other.entries.next_entry_with_hash(position) { position = next; - if !override_existing && dict.contains(vm, &*key, hash)? { - continue; + let outcome: PyResult<()> = (|| { + if override_existing || !dict.contains(vm, &*key, hash)? { + dict.insert(vm, &*key, hash, value.clone())?; + } + Ok(()) + })(); + // Release current references before checking mutations from + // comparisons or finalizers, including a skipped existing key. + drop(value); + drop(key); + outcome?; + if dict_other.__len__() != source_len { + return Err(vm.new_runtime_error("dict mutated during update")); } - dict.insert(vm, &*key, hash, value)?; - } - if dict_other.entries.has_changed_size(dict_size) { - return Err(vm.new_runtime_error("dict mutated during update")); } Ok(()) } @@ -2171,11 +2182,16 @@ dict_view! { vm.new_tuple((key, value)).into() } +fn set_view_dict(obj: &PyObject) -> Option<&PyAnyDict> { + obj.downcast_ref::() + .map(|view| view.dict()) + .or_else(|| obj.downcast_ref::().map(|view| view.dict())) +} + fn is_set_or_dict_view(obj: &PyObject) -> bool { obj.downcast_ref::().is_some() || obj.downcast_ref::().is_some() - || obj.downcast_ref::().is_some() - || obj.downcast_ref::().is_some() + || set_view_dict(obj).is_some() } // Set operations defined on set-like views of the dictionary. @@ -2399,14 +2415,29 @@ impl AsSequence for PyDictValues { } } +fn dict_view_to_set(obj: &PyObject, vm: &VirtualMachine) -> PyResult { + if let Some(keys) = obj.downcast_ref::() { + let dict = keys.dict(); + if dict.as_object().class().is(vm.ctx.types.dict_type) + || dict.as_object().class().is(vm.ctx.types.frozendict_type) + { + // Exact dictionary keys are already hashed. Subclasses instead + // go through the view iterator, including OrderedDict views. + let result = PySetInner::default(); + result.merge_dict(dict.as_dict(), vm)?; + return Ok(result); + } + } + // Reflected operations can put a set on the left. Reuse the set + // constructor's native paths rather than calling an overridden iterator. + PySetInner::from_object(obj.to_owned(), vm) +} + fn set_inner_number_op(a: &PyObject, b: &PyObject, f: F, vm: &VirtualMachine) -> PyResult where F: FnOnce(PySetInner, PyObjectRef) -> PyResult, { - let a = PySetInner::from_iter( - ArgIterable::try_from_object(vm, a.to_owned())?.iter(vm)?, - vm, - )?; + let a = dict_view_to_set(a, vm)?; Ok(PySet { inner: f(a, b.to_owned())?, } @@ -2422,9 +2453,11 @@ pub(crate) fn set_inner_number_subtract( } pub(crate) fn set_inner_number_and(a: &PyObject, b: &PyObject, vm: &VirtualMachine) -> PyResult { - let a_is_view = - a.downcast_ref::().is_some() || a.downcast_ref::().is_some(); - let (view, other) = if a_is_view { (a, b) } else { (b, a) }; + let (view, other) = if set_view_dict(a).is_some() { + (a, b) + } else { + (b, a) + }; set_view_number_and(view, other, vm) } @@ -2433,6 +2466,18 @@ pub(crate) fn set_view_number_and( other: &PyObject, vm: &VirtualMachine, ) -> PyResult { + let view_len = set_view_dict(view).unwrap().__len__(); + if other.class().is(vm.ctx.types.set_type) && view_len <= other.length(vm)? { + // The set path hashes each candidate once and can stop when the + // result has reached the set's size. It also preserves the key from + // the view, rather than the equal key resident in the set. + return vm.call_method(other, "intersection", (view.to_owned(),)); + } + let (view, other) = if set_view_dict(other).is_some_and(|dict| dict.__len__() > view_len) { + (other, view) + } else { + (view, other) + }; let other = ArgIterable::::try_from_object(vm, other.to_owned())?; let result = PySet::default(); for item in other.iter(vm)? { @@ -2456,36 +2501,70 @@ pub(crate) fn set_inner_number_xor(a: &PyObject, b: &PyObject, vm: &VirtualMachi ) } -pub(crate) fn set_item_view_number_xor( - a: &PyObject, - b: &PyObject, - both_are_item_views: bool, - vm: &VirtualMachine, -) -> PyResult { - if !both_are_item_views { - return set_inner_number_xor(a, b, vm); - } +fn set_item_view_number_xor(a: &PyDictItems, b: &PyDictItems, vm: &VirtualMachine) -> PyResult { + // Membership comparisons may mutate either original dictionary. Keep + // unmatched left entries in a private copy, as in dictitems_xor. + let remaining = a.dict().copy_as_dict(vm)?; let result = PySet::default(); - let a_iterable = ArgIterable::::try_from_object(vm, a.to_owned())?; - for item in a_iterable.iter(vm)? { - let item = item?; - if !b.sequence_unchecked().contains(&item, vm)? { - result.add(item, vm)?; + let mut current_key = None; + let mut current_value = None; + let outcome = (|| -> PyResult<()> { + // Read the right dictionary live, without a public iterator's size + // check. Release each table guard before calling value comparisons. + let mut position = 0; + while let Some((next_position, key, value, hash)) = + b.dict().as_dict().entries.next_entry_with_hash(position) + { + position = next_position; + current_key = Some(key); + current_value = Some(value); + let key = current_key.as_ref().unwrap().as_object(); + // Only unmatched pairs need to be hashable, and stored key hashes + // must not invoke __hash__ again during lookup or deletion. + let remove = match remaining.entries.get(vm, key, hash)? { + Some(left_value) => { + vm.identical_or_equal(&left_value, current_value.as_ref().unwrap())? + } + None => false, + }; + if remove { + drop(current_value.take()); + remaining.entries.delete(vm, key, hash)?; + drop(current_key.take()); + } else { + let pair = + vm.new_tuple((current_key.take().unwrap(), current_value.take().unwrap())); + result.add(pair.into(), vm)?; + } } - } - let b_iterable = ArgIterable::::try_from_object(vm, b.to_owned())?; - for item in b_iterable.iter(vm)? { - let item = item?; - if !a.sequence_unchecked().contains(&item, vm)? { - result.add(item, vm)?; + // Append the entries still present in the private copy. + let mut position = 0; + while let Some((next_position, key, value)) = remaining.entries.next_entry(position) { + position = next_position; + result.add(vm.new_tuple((key, value)).into(), vm)?; } + Ok(()) + })(); + // On failure, CPython releases the private dictionary and result before + // the current right item. Their finalizers may observe the distinction. + drop(remaining); + if let Err(exc) = outcome { + drop(result); + drop(current_key); + drop(current_value); + return Err(exc); } Ok(result.into_pyobject(vm)) } fn dict_item_view_number_xor(a: &PyObject, b: &PyObject, vm: &VirtualMachine) -> PyResult { - let is_native_item_view = |obj: &PyObject| obj.downcast_ref::().is_some(); - set_item_view_number_xor(a, b, is_native_item_view(a) && is_native_item_view(b), vm) + match ( + a.downcast_ref::(), + b.downcast_ref::(), + ) { + (Some(a), Some(b)) => set_item_view_number_xor(a, b, vm), + _ => set_inner_number_xor(a, b, vm), + } } pub(crate) fn set_inner_number_or(a: &PyObject, b: &PyObject, vm: &VirtualMachine) -> PyResult { diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index b30f1c96a3a..763e7d23a09 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -232,21 +232,14 @@ unsafe impl Traverse for PySetInner { } impl PySetInner { - pub(super) fn from_iter(iter: T, vm: &VirtualMachine) -> PyResult - where - T: IntoIterator>, - { - let set = Self::default(); - for item in iter { - let item = item?; - set.add(&item, vm)?; - } - Ok(set) - } - /// Build a set from an arbitrary object, reusing stored hashes when the /// source is a set/frozenset/dict/frozendict. - fn from_object(iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult { + pub(super) fn from_object(iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if let Some(source) = extract_set(&iterable) { + // A fresh table can copy resident entries without comparing + // colliding keys or invoking a subclass's iterator. + return Ok(source.copy()); + } let set = Self::default(); set.update_internal(iterable, vm)?; Ok(set) @@ -608,8 +601,15 @@ impl PySetInner { Ok(()) } - fn merge_dict(&self, dict: &PyDict, vm: &VirtualMachine) -> PyResult<()> { - for (key, hash) in dict._as_dict_inner().keys_with_hashes() { + pub(super) fn merge_dict(&self, dict: &PyDict, vm: &VirtualMachine) -> PyResult<()> { + // Equality between colliding keys can change the source dictionary. + // Match the live _PyDict_Next traversal used by set_update_dict. + let mut position = 0; + while let Some((next_position, key, value, hash)) = + dict._as_dict_inner().next_entry_with_hash(position) + { + position = next_position; + drop(value); self.add_known_hash(&key, hash, vm)?; } Ok(()) diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index fcc00e67f42..326ee0da003 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -860,7 +860,6 @@ impl Dict { } /// Delete a key. Same hash contract as [`Self::insert`]. - #[cfg(test)] pub(crate) fn delete(&self, vm: &VirtualMachine, key: &K, hash: HashValue) -> PyResult<()> where K: DictKey + ?Sized, @@ -1107,11 +1106,6 @@ impl Dict { self.read().entries.len().saturating_sub(position) } - pub(crate) fn has_changed_size(&self, old: &DictSize) -> bool { - let current = self.read().size(); - current != *old - } - /// Inspect keys without allocating or cloning their references. /// The predicate must not run Python or mutate this table. pub(crate) fn any_key(&self, mut predicate: impl FnMut(&PyObject) -> bool) -> bool { From 3ad04323411774db92d3ed67ec9b49b7b801bcae Mon Sep 17 00:00:00 2001 From: dot Date: Wed, 4 Nov 2026 17:44:57 +0900 Subject: [PATCH 4/6] Reuse set construction for dictionary view operands Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/builtins/dict.rs | 7 +++---- crates/vm/src/builtins/set.rs | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index b1bce4e9c72..cba58bcd933 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -2416,6 +2416,7 @@ impl AsSequence for PyDictValues { } fn dict_view_to_set(obj: &PyObject, vm: &VirtualMachine) -> PyResult { + let mut source = obj; if let Some(keys) = obj.downcast_ref::() { let dict = keys.dict(); if dict.as_object().class().is(vm.ctx.types.dict_type) @@ -2423,14 +2424,12 @@ fn dict_view_to_set(obj: &PyObject, vm: &VirtualMachine) -> PyResult { // Exact dictionary keys are already hashed. Subclasses instead // go through the view iterator, including OrderedDict views. - let result = PySetInner::default(); - result.merge_dict(dict.as_dict(), vm)?; - return Ok(result); + source = dict.as_object(); } } // Reflected operations can put a set on the left. Reuse the set // constructor's native paths rather than calling an overridden iterator. - PySetInner::from_object(obj.to_owned(), vm) + PySetInner::from_object(source.to_owned(), vm) } fn set_inner_number_op(a: &PyObject, b: &PyObject, f: F, vm: &VirtualMachine) -> PyResult diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 763e7d23a09..c3e9824276e 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -601,7 +601,7 @@ impl PySetInner { Ok(()) } - pub(super) fn merge_dict(&self, dict: &PyDict, vm: &VirtualMachine) -> PyResult<()> { + fn merge_dict(&self, dict: &PyDict, vm: &VirtualMachine) -> PyResult<()> { // Equality between colliding keys can change the source dictionary. // Match the live _PyDict_Next traversal used by set_update_dict. let mut position = 0; From bb6a02e00563414ed85237caaa83d63a2a589bed Mon Sep 17 00:00:00 2001 From: dot Date: Thu, 5 Nov 2026 03:13:59 +0900 Subject: [PATCH 5/6] Preserve intersection failure cleanup order Retain the current dictionary-view item until its iterator and partial result have been released, including consuming set insertion failures. Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/builtins/dict.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index cba58bcd933..264b39a5eda 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -2479,11 +2479,26 @@ pub(crate) fn set_view_number_and( }; let other = ArgIterable::::try_from_object(vm, other.to_owned())?; let result = PySet::default(); - for item in other.iter(vm)? { - let item = item?; - if view.sequence_unchecked().contains(&item, vm)? { - result.add(item, vm)?; + let mut iterator = other.iter(vm)?; + let mut current = None; + let outcome = (|| -> PyResult<()> { + for item in iterator.by_ref() { + current = Some(item?); + let item = current.as_ref().unwrap(); + if view.sequence_unchecked().contains(item, vm)? { + result.add(item.clone(), vm)?; + } + drop(current.take()); } + Ok(()) + })(); + // On failure, release the iterator and partial result before the current + // item. Retain its owner through both membership and result insertion. + drop(iterator); + if let Err(exc) = outcome { + drop(result); + drop(current); + return Err(exc); } Ok(result.into_pyobject(vm)) } From d5b0314e64a121c9d921f4595151931556ca4648 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 6 Nov 2026 12:33:34 +0900 Subject: [PATCH 6/6] Restore typed set predicate arguments Keep single-operand set and frozenset predicates declarative after the shared binder checks. Borrow their original sources while retaining native-set dispatch and validated iterator construction, including through issubset final size comparison. Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/builtins/set.rs | 47 +++++++++++++++------------ extra_tests/snippets/builtin_set.py | 50 +++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index c3e9824276e..50f1f27a7e3 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -18,7 +18,10 @@ use crate::{ }, convert::{ToPyResult, TransmuteFromObject}, dict_inner::{self, DictSize}, - function::{FuncArgs, NameOthers, OptionalArg, PosArgs, PyArithmeticValue, PyComparisonValue}, + function::{ + ArgIterable, FuncArgs, NameOthers, OptionalArg, PosArgs, PyArithmeticValue, + PyComparisonValue, + }, object::{Traverse, TraverseFn}, protocol::{PyIterReturn, PyNumberMethods, PySequenceMethods}, recursion::ReprGuard, @@ -455,8 +458,8 @@ impl PySetInner { Ok(result) } - fn issuperset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other_set) = extract_set(other.as_object()) { + fn issuperset(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult { + if let Some(other_set) = extract_set(other) { return self.compare(other_set, PyComparisonOp::Ge, vm); } for item in other.get_iter(vm)?.into_iter::(vm) { @@ -469,19 +472,21 @@ impl PySetInner { Ok(true) } - fn issubset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other_set) = extract_set(other.as_object()) { + fn issubset(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult { + if let Some(other_set) = extract_set(other) { return self.compare(other_set, PyComparisonOp::Le, vm); } - Ok(self.intersection(other, vm)?.len() == self.len()) + // The binding owns the source until after the size comparison. Its + // finalizer must not mutate self between intersection and comparison. + Ok(self.intersection(other.to_owned(), vm)?.len() == self.len()) } - pub(super) fn isdisjoint(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other_set) = extract_set(other.as_object()) { + pub(super) fn isdisjoint(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult { + if let Some(other_set) = extract_set(other) { if core::ptr::eq(self, other_set) { return Ok(self.len() == 0); } - let other_type = other.as_object().class(); + let other_type = other.class(); if other_type.is(vm.ctx.types.set_type) || other_type.is(vm.ctx.types.frozenset_type) { let (target, source) = if self.len() < other_set.len() { (other_set, self) @@ -976,18 +981,18 @@ impl Py { } #[pymethod] - fn issubset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.inner.issubset(other, vm) + fn issubset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + self.inner.issubset(other.as_object(), vm) } #[pymethod] - fn issuperset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.inner.issuperset(other, vm) + fn issuperset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + self.inner.issuperset(other.as_object(), vm) } #[pymethod] - fn isdisjoint(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.inner.isdisjoint(other, vm) + fn isdisjoint(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + self.inner.isdisjoint(other.as_object(), vm) } #[pymethod] @@ -1496,18 +1501,18 @@ impl Py { } #[pymethod] - fn issubset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.inner.issubset(other, vm) + fn issubset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + self.inner.issubset(other.as_object(), vm) } #[pymethod] - fn issuperset(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.inner.issuperset(other, vm) + fn issuperset(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + self.inner.issuperset(other.as_object(), vm) } #[pymethod] - fn isdisjoint(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.inner.isdisjoint(other, vm) + fn isdisjoint(&self, other: ArgIterable, vm: &VirtualMachine) -> PyResult { + self.inner.isdisjoint(other.as_object(), vm) } #[pymethod] diff --git a/extra_tests/snippets/builtin_set.py b/extra_tests/snippets/builtin_set.py index 05edaaebeef..7ccd72c7ccc 100644 --- a/extra_tests/snippets/builtin_set.py +++ b/extra_tests/snippets/builtin_set.py @@ -498,6 +498,56 @@ def __iter__(self): assert_raises(RuntimeError, set_type([3]).isdisjoint, subclass) +# A disabled iterator does not disable the predicates' native set paths. +for set_type in (set, frozenset): + + class DisabledIterator(set_type): + __iter__ = None + + subclass = DisabledIterator([1]) + assert set_type([1]).issubset(subclass) + assert set_type([1, 2]).issuperset(subclass) + assert not subclass.isdisjoint(subclass) + empty_subclass = DisabledIterator() + assert empty_subclass.isdisjoint(empty_subclass) + assert_raises(TypeError, set_type([2]).isdisjoint, subclass) + + +class InvalidPredicateIterator: + def __iter__(self): + return [] + + +# Typed bindings must preserve both binder errors and GetIter validation. +with assert_raises(TypeError) as invalid_iterator: + iter(InvalidPredicateIterator()) +for set_type in (set, frozenset): + for method_name in ("issubset", "issuperset", "isdisjoint"): + predicate = getattr(set_type([1]), method_name) + with assert_raises(TypeError) as rejected_keyword: + predicate(42, unexpected=True) + assert "takes no keyword arguments" in str(rejected_keyword.exception) + with assert_raises(TypeError) as rejected_iterator: + predicate(InvalidPredicateIterator()) + assert str(rejected_iterator.exception) == str(invalid_iterator.exception) + + +# A temporary source remains alive through issubset's final size comparison. +subset_target = {1} + + +class TemporarySubsetSource: + def __iter__(self): + return iter([1]) + + def __del__(self): + subset_target.clear() + + +assert subset_target.issubset(TemporarySubsetSource()) +assert subset_target == set() + + # Intersections retain the key from the smaller operand, or RHS on a tie. small_key = float("1") small = {small_key}