Skip to content

Commit b725188

Browse files
authored
Represent OrderedDict views with their contents (#9038)
Extract the Python 3.14-compatible view representation from #8954. Snapshot view elements before invoking their repr methods, preserve recursion handling, and cover each view in the existing collections snippet. Leave migration-specific mapping changes out. Assisted-by: OpenAI Codex:model identifier unavailable
1 parent 3af5f8a commit b725188

2 files changed

Lines changed: 130 additions & 4 deletions

File tree

‎crates/vm/src/stdlib/_collections/ordered_dict.rs‎

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub(crate) mod ordered_dict {
77
AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject,
88
VirtualMachine, atomic_func,
99
builtins::{
10-
PyDict, PyGenericAlias, PyMappingProxy, PyTuple, PyTypeRef,
10+
PyDict, PyGenericAlias, PyMappingProxy, PyStrRef, PyTuple, PyTypeRef,
1111
dict::{
1212
PyDictItems, set_inner_number_or, set_inner_number_subtract, set_inner_number_xor,
1313
set_item_view_number_xor, set_view_number_and,
@@ -27,6 +27,7 @@ pub(crate) mod ordered_dict {
2727
Initializer, IterNext, Iterable, PyComparisonOp, Representable, SelfIter,
2828
},
2929
};
30+
use rustpython_common::wtf8::Wtf8Buf;
3031
use std::collections::HashMap;
3132

3233
const ODICT_ITER_REVERSED: u8 = 1;
@@ -977,14 +978,37 @@ pub(crate) mod ordered_dict {
977978
set_item_view_number_xor(a, b, is_item_view(a) && is_item_view(b), vm)
978979
}
979980

981+
fn ordered_view_repr(
982+
view: &PyObject,
983+
od: &PyRef<PyOrderedDict>,
984+
kind: u8,
985+
name: &str,
986+
vm: &VirtualMachine,
987+
) -> PyResult<PyStrRef> {
988+
let Some(_guard) = ReprGuard::enter(vm, view) else {
989+
return Ok(vm.ctx.intern_str("...").to_owned());
990+
};
991+
// Snapshot the view before calling user-defined repr methods, which
992+
// can mutate the underlying ordered dictionary.
993+
let iterator = PyODictIter::new(od.clone(), kind).to_pyobject(vm);
994+
let items = PyIter::try_from_object(vm, iterator)?
995+
.iter::<PyObjectRef>(vm)?
996+
.collect::<PyResult<Vec<_>>>()?;
997+
let list_repr = vm.ctx.new_list(items).as_object().repr(vm)?;
998+
let mut result = Wtf8Buf::from(format!("{name}("));
999+
result.push_wtf8(list_repr.as_wtf8());
1000+
result.push_str(")");
1001+
Ok(vm.ctx.new_str(result))
1002+
}
1003+
9801004
#[pyattr]
9811005
#[pyclass(name = "odict_keys", module = "builtins", unhashable = true, traverse)]
9821006
#[derive(Debug, PyPayload)]
9831007
struct PyODictKeys {
9841008
od: PyRef<PyOrderedDict>,
9851009
}
9861010

987-
#[pyclass(with(Iterable, Comparable, AsMapping, AsSequence, AsNumber))]
1011+
#[pyclass(with(Iterable, Comparable, AsMapping, AsSequence, AsNumber, Representable))]
9881012
impl Py<PyODictKeys> {
9891013
#[pymethod]
9901014
fn __reversed__(&self, vm: &VirtualMachine) -> PyObjectRef {
@@ -1007,6 +1031,22 @@ pub(crate) mod ordered_dict {
10071031
}
10081032
}
10091033

1034+
impl Representable for PyODictKeys {
1035+
fn repr(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyStrRef> {
1036+
ordered_view_repr(
1037+
zelf.as_object(),
1038+
&zelf.od,
1039+
ODICT_ITER_KEYS,
1040+
"odict_keys",
1041+
vm,
1042+
)
1043+
}
1044+
1045+
fn repr_str(_zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
1046+
unreachable!("use repr instead")
1047+
}
1048+
}
1049+
10101050
impl AsMapping for PyODictKeys {
10111051
fn as_mapping() -> &'static PyMappingMethods {
10121052
static AS_MAPPING: PyMappingMethods = PyMappingMethods {
@@ -1077,7 +1117,7 @@ pub(crate) mod ordered_dict {
10771117
od: PyRef<PyOrderedDict>,
10781118
}
10791119

1080-
#[pyclass(with(Iterable, Comparable, AsMapping))]
1120+
#[pyclass(with(Iterable, Comparable, AsMapping, Representable))]
10811121
impl Py<PyODictValues> {
10821122
#[pymethod]
10831123
fn __reversed__(&self, vm: &VirtualMachine) -> PyObjectRef {
@@ -1091,6 +1131,22 @@ pub(crate) mod ordered_dict {
10911131
}
10921132
}
10931133

1134+
impl Representable for PyODictValues {
1135+
fn repr(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyStrRef> {
1136+
ordered_view_repr(
1137+
zelf.as_object(),
1138+
&zelf.od,
1139+
ODICT_ITER_VALUES,
1140+
"odict_values",
1141+
vm,
1142+
)
1143+
}
1144+
1145+
fn repr_str(_zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
1146+
unreachable!("use repr instead")
1147+
}
1148+
}
1149+
10941150
impl AsMapping for PyODictValues {
10951151
fn as_mapping() -> &'static PyMappingMethods {
10961152
static AS_MAPPING: PyMappingMethods = PyMappingMethods {
@@ -1128,7 +1184,7 @@ pub(crate) mod ordered_dict {
11281184
od: PyRef<PyOrderedDict>,
11291185
}
11301186

1131-
#[pyclass(with(Iterable, Comparable, AsMapping, AsSequence, AsNumber))]
1187+
#[pyclass(with(Iterable, Comparable, AsMapping, AsSequence, AsNumber, Representable))]
11321188
impl Py<PyOrderedDictItems> {
11331189
#[pymethod]
11341190
fn __reversed__(&self, vm: &VirtualMachine) -> PyObjectRef {
@@ -1163,6 +1219,22 @@ pub(crate) mod ordered_dict {
11631219
}
11641220
}
11651221

1222+
impl Representable for PyOrderedDictItems {
1223+
fn repr(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyStrRef> {
1224+
ordered_view_repr(
1225+
zelf.as_object(),
1226+
&zelf.od,
1227+
ODICT_ITER_ITEMS,
1228+
"odict_items",
1229+
vm,
1230+
)
1231+
}
1232+
1233+
fn repr_str(_zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
1234+
unreachable!("use repr instead")
1235+
}
1236+
}
1237+
11661238
impl AsMapping for PyOrderedDictItems {
11671239
fn as_mapping() -> &'static PyMappingMethods {
11681240
static AS_MAPPING: PyMappingMethods = PyMappingMethods {

‎extra_tests/snippets/stdlib_collections.py‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,60 @@ def __repr__(self):
9191
ordered = OrderedDict()
9292
_count_elements(ordered, "bab")
9393
assert list(ordered.items()) == [("b", 2), ("a", 1)]
94+
95+
96+
# OrderedDict views show their contents in insertion order, including recursion.
97+
def check_ordered_dict_view_repr():
98+
ordered = OrderedDict([("b", 2), ("a", 1)])
99+
for method, contents in (
100+
("keys", "['b', 'a']"),
101+
("values", "[2, 1]"),
102+
("items", "[('b', 2), ('a', 1)]"),
103+
):
104+
assert repr(getattr(OrderedDict(), method)()) == "odict_" + method + "([])"
105+
assert (
106+
repr(getattr(ordered, method)()) == "odict_" + method + "(" + contents + ")"
107+
)
108+
109+
recursive = OrderedDict()
110+
values = recursive.values()
111+
items = recursive.items()
112+
recursive["self"] = values
113+
assert repr(values) == "odict_values([...])"
114+
recursive["self"] = items
115+
assert repr(items) == "odict_items([('self', ...)])"
116+
117+
# Repr must snapshot the contents before arbitrary repr code mutates them.
118+
for method in ("keys", "values", "items"):
119+
120+
class MutatingRepr:
121+
def __repr__(self):
122+
ordered.clear()
123+
return "mutated"
124+
125+
item = MutatingRepr()
126+
ordered = OrderedDict([(item, item), ("tail", "last")])
127+
expected = {
128+
"keys": "odict_keys([mutated, 'tail'])",
129+
"values": "odict_values([mutated, 'last'])",
130+
"items": "odict_items([(mutated, mutated), ('tail', 'last')])",
131+
}
132+
assert repr(getattr(ordered, method)()) == expected[method]
133+
assert not ordered
134+
135+
class BadRepr:
136+
def __repr__(self):
137+
raise ValueError("view repr")
138+
139+
ordered = OrderedDict([(BadRepr(), BadRepr())])
140+
for method in ("keys", "values", "items"):
141+
with assert_raises(ValueError) as caught:
142+
repr(getattr(ordered, method)())
143+
assert str(caught.exception) == "view repr"
144+
145+
146+
check_ordered_dict_view_repr()
147+
94148
factory = defaultdict(lambda: 100)
95149
_count_elements(factory, "a")
96150
assert factory == {"a": 1}

0 commit comments

Comments
 (0)