Skip to content

Commit c7a19f0

Browse files
authored
dict: match CPython errors for invalid update sequence elements (#8338)
* fix dict update type error * Use tuple for dictionary update sequence pairs * Propagate errors while adding dict update sequence notes * Match BaseException.add_note error message with CPython
1 parent 071d54d commit c7a19f0

3 files changed

Lines changed: 76 additions & 17 deletions

File tree

‎Lib/test/test_dict.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@ def __next__(self):
275275

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

278-
@unittest.expectedFailure # TODO: RUSTPYTHON
279278
def test_update_type_error(self):
280279
with self.assertRaises(TypeError) as cm:
281280
{}.update([object() for _ in range(3)])

‎crates/vm/src/builtins/dict.rs‎

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::object::{Traverse, TraverseFn};
77
use crate::{
88
AsObject, Context, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
99
TryFromObject, atomic_func,
10-
builtins::{PyTuple, iter::builtins_iter, type_::PyAttributes},
10+
builtins::{PyList, PyTuple, iter::builtins_iter, type_::PyAttributes},
1111
class::{PyClassDef, PyClassImpl},
1212
common::ascii,
1313
dict_inner::{self, DictKey},
@@ -183,6 +183,76 @@ impl PyDict {
183183
self.merge_object_with_override(other, false, vm)
184184
}
185185

186+
fn add_update_sequence_note(
187+
exc: PyBaseExceptionRef,
188+
index: usize,
189+
vm: &VirtualMachine,
190+
) -> PyBaseExceptionRef {
191+
if !exc.fast_isinstance(vm.ctx.exceptions.type_error) {
192+
return exc;
193+
}
194+
195+
let note =
196+
format!("Cannot convert dictionary update sequence element #{index} to a sequence");
197+
match vm.call_method(exc.as_object(), "add_note", (vm.ctx.new_str(note),)) {
198+
Ok(_) => exc,
199+
Err(note_err) => {
200+
note_err.set___context__(Some(exc));
201+
note_err
202+
}
203+
}
204+
}
205+
206+
fn update_sequence_pair_from_slice(
207+
elements: &[PyObjectRef],
208+
index: usize,
209+
vm: &VirtualMachine,
210+
) -> PyResult<(PyObjectRef, PyObjectRef)> {
211+
let [key, value] = elements else {
212+
return Err(vm.new_value_error(format!(
213+
"dictionary update sequence element #{index} has length {}; 2 is required",
214+
elements.len()
215+
)));
216+
};
217+
Ok((key.clone(), value.clone()))
218+
}
219+
220+
fn update_sequence_pair(
221+
element: PyObjectRef,
222+
index: usize,
223+
vm: &VirtualMachine,
224+
) -> PyResult<(PyObjectRef, PyObjectRef)> {
225+
let element = match element.downcast_exact::<PyList>(vm) {
226+
Ok(list) => {
227+
let elements = list.borrow_vec();
228+
return Self::update_sequence_pair_from_slice(&elements, index, vm);
229+
}
230+
Err(element) => element,
231+
};
232+
let element = match element.downcast_exact::<PyTuple>(vm) {
233+
Ok(tuple) => {
234+
return Self::update_sequence_pair_from_slice(tuple.as_slice(), index, vm);
235+
}
236+
Err(element) => element,
237+
};
238+
239+
let elements = (|| {
240+
let elem_iter = element.get_iter(vm).map_err(|exc| {
241+
if exc.fast_isinstance(vm.ctx.exceptions.type_error) {
242+
vm.new_type_error("object is not iterable")
243+
} else {
244+
exc
245+
}
246+
})?;
247+
elem_iter
248+
.into_iter::<PyObjectRef>(vm)?
249+
.collect::<PyResult<Vec<_>>>()
250+
})()
251+
.map_err(|exc| Self::add_update_sequence_note(exc, index, vm))?;
252+
253+
Self::update_sequence_pair_from_slice(&elements, index, vm)
254+
}
255+
186256
pub fn merge_from_seq2(
187257
&self,
188258
seq2: PyObjectRef,
@@ -191,20 +261,10 @@ impl PyDict {
191261
) -> PyResult<()> {
192262
let iter = seq2.get_iter(vm)?;
193263
let dict = &self.entries;
194-
loop {
195-
fn err(vm: &VirtualMachine) -> PyBaseExceptionRef {
196-
vm.new_value_error("Iterator must have exactly two elements")
197-
}
198-
let element = match iter.next(vm)? {
199-
PyIterReturn::Return(obj) => obj,
200-
PyIterReturn::StopIteration(_) => break,
201-
};
202-
let elem_iter = element.get_iter(vm)?;
203-
let key = elem_iter.next(vm)?.into_result().map_err(|_| err(vm))?;
204-
let value = elem_iter.next(vm)?.into_result().map_err(|_| err(vm))?;
205-
if matches!(elem_iter.next(vm)?, PyIterReturn::Return(_)) {
206-
return Err(err(vm));
207-
}
264+
265+
for (index, element) in iter.iter_without_hint::<PyObjectRef>(vm)?.enumerate() {
266+
let (key, value) = Self::update_sequence_pair(element?, index, vm)?;
267+
208268
if !override_existing && dict.contains(vm, &*key)? {
209269
continue;
210270
}

‎crates/vm/src/exceptions.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ impl PyRef<PyBaseException> {
704704

705705
let notes = notes
706706
.downcast::<PyList>()
707-
.map_err(|_| vm.new_type_error("__notes__ must be a list"))?;
707+
.map_err(|_| vm.new_type_error("Cannot add note: __notes__ is not a list"))?;
708708

709709
notes.borrow_vec_mut().push(note.into());
710710
Ok(())

0 commit comments

Comments
 (0)