Skip to content

dict: match CPython errors for invalid update sequence elements #8338

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

Merged
youknowone merged 5 commits into from
Jul 29, 2026
Merged

dict: match CPython errors for invalid update sequence elements #8338

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0aed904
fix dict update type error
2jiyong Jul 18, 2026
73891b1
Use tuple for dictionary update sequence pairs
2jiyong Jul 25, 2026
a0adb66
Propagate errors while adding dict update sequence notes
2jiyong Jul 29, 2026
a0817f9
Match BaseException.add_note error message with CPython
2jiyong Jul 29, 2026
f012d4f
Merge branch 'main' into fix-dict-update-type-error
2jiyong Jul 29, 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
Next Next commit
fix dict update type error
  • Loading branch information
@2jiyong
2jiyong committed Jul 20, 2026
commit 0aed904dd70efc1fb2ce63508812d44082261cca
1 change: 0 additions & 1 deletion Lib/test/test_dict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ def __next__(self):

self.assertRaises(ValueError, {}.update, [(1, 2, 3)])

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_update_type_error(self):
with self.assertRaises(TypeError) as cm:
{}.update([object() for _ in range(3)])
Expand Down
78 changes: 63 additions & 15 deletions crates/vm/src/builtins/dict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
AsObject, Context, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
TryFromObject, atomic_func,
builtins::{
PyTuple,
PyList, PyTuple,
iter::{builtins_iter, builtins_reversed},
type_::PyAttributes,
},
Expand Down Expand Up @@ -187,6 +187,64 @@ impl PyDict {
self.merge_object_with_override(other, false, vm)
}

fn add_update_sequence_note(exc: &PyBaseExceptionRef, index: usize, vm: &VirtualMachine) {
if exc.fast_isinstance(vm.ctx.exceptions.type_error) {
let note =
format!("Cannot convert dictionary update sequence element #{index} to a sequence");
let _ = vm.call_method(exc.as_object(), "add_note", (vm.ctx.new_str(note),));

@youknowone youknowone Jul 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, one more thing to check. is it correct to ignore the result of vm.call_method? don't we need to propagate this error?
If we don't need to propagate this error, please add a comment why let _ = is justified here.

@2jiyong 2jiyong Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used let _ = to ignore the error, following the existing add_note handling in type.rs and codecs.rs.
However, CPython propagates an error raised while adding the note and chains the original exception as its __context__.
I’ll update this to match CPython’s behavior.

}
}

fn update_sequence_pair_from_slice(
elements: &[PyObjectRef],
index: usize,
vm: &VirtualMachine,
) -> PyResult<[PyObjectRef; 2]> {
let [key, value] = elements else {
return Err(vm.new_value_error(format!(
"dictionary update sequence element #{index} has length {}; 2 is required",
elements.len()
)));
};
Ok([key.clone(), value.clone()])
}

fn update_sequence_pair(
element: PyObjectRef,
index: usize,
vm: &VirtualMachine,
) -> PyResult<[PyObjectRef; 2]> {
Comment thread
2jiyong marked this conversation as resolved.
Outdated
let element = match element.downcast_exact::<PyList>(vm) {
Ok(list) => {
let elements = list.borrow_vec();
return Self::update_sequence_pair_from_slice(&elements, index, vm);
}
Err(element) => element,
};
let element = match element.downcast_exact::<PyTuple>(vm) {
Ok(tuple) => {
return Self::update_sequence_pair_from_slice(tuple.as_slice(), index, vm);
}
Err(element) => element,
};

let elements = (|| {
let elem_iter = element.get_iter(vm).map_err(|exc| {
if exc.fast_isinstance(vm.ctx.exceptions.type_error) {
vm.new_type_error("object is not iterable")
} else {
exc
}
})?;
elem_iter
.into_iter::<PyObjectRef>(vm)?
.collect::<PyResult<Vec<_>>>()
})()
.inspect_err(|exc| Self::add_update_sequence_note(exc, index, vm))?;

Self::update_sequence_pair_from_slice(&elements, index, vm)
}

pub fn merge_from_seq2(
&self,
seq2: PyObjectRef,
Expand All @@ -195,20 +253,10 @@ impl PyDict {
) -> PyResult<()> {
let iter = seq2.get_iter(vm)?;
let dict = &self.entries;
loop {
fn err(vm: &VirtualMachine) -> PyBaseExceptionRef {
vm.new_value_error("Iterator must have exactly two elements")
}
let element = match iter.next(vm)? {
PyIterReturn::Return(obj) => obj,
PyIterReturn::StopIteration(_) => break,
};
let elem_iter = element.get_iter(vm)?;
let key = elem_iter.next(vm)?.into_result().map_err(|_| err(vm))?;
let value = elem_iter.next(vm)?.into_result().map_err(|_| err(vm))?;
if matches!(elem_iter.next(vm)?, PyIterReturn::Return(_)) {
return Err(err(vm));
}

for (index, element) in iter.iter_without_hint::<PyObjectRef>(vm)?.enumerate() {
let [key, value] = Self::update_sequence_pair(element?, index, vm)?;

if !override_existing && dict.contains(vm, &*key)? {
continue;
}
Expand Down
Loading