⚡ Sangsakala Proxy 🏠 Home
Skip to content

Align OrderedDict update and serialization controls #9682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
youknowdot wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Align OrderedDict update and serialization controls #9682

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50b59fa
Align OrderedDict update and reduction controls
youknowdot Nov 4, 2026
f97290c
Make OrderedDict outer source lifetime explicit
youknowdot Nov 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 103 additions & 55 deletions crates/vm/src/stdlib/_collections/ordered_dict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub(crate) mod ordered_dict {
common::{hash::PyHash, lock::PyMutex},
convert::ToPyObject,
dict_inner::DictKey,
function::{FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue},
function::{KwArgs, OptionalArg, PyArithmeticValue, PyComparisonValue},
object::{Traverse, TraverseFn},
protocol::{PyIter, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods},
recursion::ReprGuard,
Expand Down Expand Up @@ -403,17 +403,16 @@ pub(crate) mod ordered_dict {
}

#[pymethod]
fn update(zelf: PyRef<PyOrderedDict>, args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> {
if args.args.len() > 1 {
return Err(vm.new_type_error(format!(
"update expected at most 1 argument, got {}",
args.args.len()
)));
}
if let Some(arg) = args.args.first() {
odict_update_arg(zelf.as_object(), arg.clone(), vm)?;
fn update(
zelf: PyRef<PyOrderedDict>,
other: OptionalArg<PyObjectRef>,
kwargs: KwArgs,
vm: &VirtualMachine,
) -> PyResult<()> {
if let OptionalArg::Present(other) = other {
odict_update_arg(zelf.as_object(), other, vm)?;
}
for (key, value) in args.kwargs {
for (key, value) in kwargs {
zelf.as_object().set_item(&key, value, vm)?;
}
Ok(())
Expand Down Expand Up @@ -547,13 +546,11 @@ pub(crate) mod ordered_dict {

#[pymethod]
fn __reduce__(zelf: PyRef<PyOrderedDict>, vm: &VirtualMachine) -> PyResult {
// State hooks run before items(), and their result is preserved even
// when a subclass deliberately returns an empty dictionary.
let state = vm.call_method(zelf.as_object(), "__getstate__", ())?;
let items = zelf.as_object().get_attr("items", vm)?.call((), vm)?;
let items_iter = PyIter::try_from_object(vm, items)?;
let state = vm.call_method(zelf.as_object(), "__getstate__", ())?;
let state = match state.downcast_ref::<PyDict>() {
Some(d) if d.__len__() == 0 => vm.ctx.none(),
_ => state,
};
Ok(vm
.ctx
.new_tuple(vec![
Expand Down Expand Up @@ -597,16 +594,10 @@ pub(crate) mod ordered_dict {
impl DefaultConstructor for PyOrderedDict {}

impl Initializer for PyOrderedDict {
type Args = FuncArgs;

fn init(zelf: &Py<Self>, args: Self::Args, vm: &VirtualMachine) -> PyResult<()> {
if args.args.len() > 1 {
return Err(vm.new_type_error(format!(
"OrderedDict expected at most 1 argument, got {}",
args.args.len()
)));
}
Py::<Self>::update(zelf.to_owned(), args, vm)
type Args = (OptionalArg<PyObjectRef>, KwArgs);

fn init(zelf: &Py<Self>, (other, kwargs): Self::Args, vm: &VirtualMachine) -> PyResult<()> {
Py::<Self>::update(zelf.to_owned(), other, kwargs, vm)
}
}

Expand Down Expand Up @@ -759,32 +750,84 @@ pub(crate) mod ordered_dict {
}

fn odict_update_arg(zelf: &PyObject, arg: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
if let Some(dict) = arg.downcast_ref_if_exact::<PyDict>(vm) {
for (key, value) in dict {
if (arg.class().is(vm.ctx.types.dict_type) || arg.class().is(vm.ctx.types.frozendict_type))
&& let Some(dict) = PyAnyDict::from_object(&arg)
{
// A destination's __setitem__ can mutate the source. CPython takes
// an items snapshot before calling any of those overrides.
let items = dict.items_vec();
let result = items
.iter()
.try_for_each(|(key, value)| zelf.set_item(&**key, value.clone(), vm));
// The snapshot is a list of tuples in CPython: release entries
// backwards, and each value before its key, after all callbacks.
for (key, value) in items.into_iter().rev() {
drop(value);
drop(key);
}
return result;
}
if let Some(keys_fn) = vm.get_attribute_opt(&arg, "keys")? {
let keys = keys_fn.call((), vm)?;
let iter = PyIter::try_from_object(vm, keys)?;
for key in iter.iter::<PyObjectRef>(vm)? {
let key = key?;
let value = arg.get_item(&*key, vm)?;
zelf.set_item(&*key, value, vm)?;
}
return Ok(());
}
match arg.get_attr("keys", vm) {
Ok(keys_fn) => {
let keys = keys_fn.call((), vm)?;
let iter = PyIter::try_from_object(vm, keys)?;
for key in iter.iter::<PyObjectRef>(vm)? {
let key = key?;
let value = arg.get_item(&*key, vm)?;
zelf.set_item(&*key, value, vm)?;
}
return Ok(());
let pairs = match vm.get_attribute_opt(&arg, "items")? {
Some(items_fn) => items_fn.call((), vm)?,
None => arg,
};
odict_add_pairs(zelf, pairs, vm)
}

fn odict_add_pairs(zelf: &PyObject, pairs: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
// An iterator need not retain its iterable. Keep the container alive
// until all pair callbacks and the outer iterator have finished.
let result = (|| {
let iter = PyIter::try_from_object(vm, pairs.clone())?;
for pair in iter.iter::<PyObjectRef>(vm)? {
let pair = pair?;
let mut pair_iter = None;
let mut key = None;
let mut value = None;
let result = (|| {
let iterator = pair_iter.insert(PyIter::try_from_object(vm, pair.clone())?);
// Unlike dict.update(), OrderedDict unpacks at most three
// values and never consults a pair's length hint.
let key = key.insert(match iterator.next(vm)? {
PyIterReturn::Return(key) => key,
PyIterReturn::StopIteration(_) => {
return Err(vm.new_value_error("need more than 0 values to unpack"));
}
});
let value = value.insert(match iterator.next(vm)? {
PyIterReturn::Return(value) => value,
PyIterReturn::StopIteration(_) => {
return Err(vm.new_value_error("need more than 1 value to unpack"));
}
});
if let PyIterReturn::Return(unexpected) = iterator.next(vm)? {
drop(unexpected);
return Err(vm.new_value_error("too many values to unpack (expected 2)"));
}
zelf.set_item(&**key, value.clone(), vm)
})();
// Finalizers may call back into the destination. Match the pair,
// iterator, key, value release order on both success and failure.
drop(pair);
drop(pair_iter);
drop(key);
drop(value);
result?;
}
Err(e) if e.fast_isinstance(vm.ctx.exceptions.attribute_error) => {}
Err(e) => return Err(e),
}
let iter = PyIter::try_from_object(vm, arg)?;
for (index, element) in iter.iter::<PyObjectRef>(vm)?.enumerate() {
let (key, value) = PyDict::update_sequence_pair(element?, index, vm)?;
zelf.set_item(&*key, value, vm)?;
}
Ok(())
Ok(())
})();
drop(pairs);
result
}

#[pyattr]
Expand Down Expand Up @@ -881,15 +924,20 @@ pub(crate) mod ordered_dict {

impl PyODictIter {
fn collect_remaining(&self, vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
let Some(od) = self.od() else {
return Ok(Vec::new());
};
// Reduction consumes a copy through the normal iterator path so
// mutation checks and value lookup callbacks match next(). The
// original iterator must retain its position, including on error.
let copy = Self {
od: self.od.clone(),
kind: self.kind,
size: self.size,
state: self.state,
current: PyMutex::new(self.current.lock().clone()),
}
.into_ref(&vm.ctx);
let mut out = Vec::new();
let mut current = self.current.lock().clone();
let reversed = self.kind & ODICT_ITER_REVERSED != 0;
while let Some(key) = current {
out.push(self.project(key.clone(), vm)?);
current = od.links.lock().next_after(&key, reversed);
while let PyIterReturn::Return(value) = Self::next(&copy, vm)? {
out.push(value);
}
Ok(out)
}
Expand Down
Loading