Skip to content

Commit 7466d17

Browse files
youknowoneyouknowone
authored
Merge pull request RustPython#3471 from TeamTamoad/collection-repr
refactor: add a repr helper function for the collections
2 parents 71f6db3 + 0f4d6cd commit 7466d17

File tree

5 files changed

+71
-66
lines changed

5 files changed

+71
-66
lines changed

‎vm/src/builtins/list.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::{
1212
AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable,
1313
Iterable, PyComparisonOp, Unconstructible, Unhashable,
1414
},
15+
utils::collection_repr,
1516
vm::{ReprGuard, VirtualMachine},
1617
PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyObjectView, PyObjectWrap,
1718
PyRef, PyResult, PyValue, TypeProtocol,
@@ -229,14 +230,10 @@ impl PyList {
229230

230231
#[pymethod(magic)]
231232
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
232-
let s = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
233-
let elements = zelf.borrow_vec().to_vec();
234-
let mut str_parts = Vec::with_capacity(elements.len());
235-
for elem in elements.iter() {
236-
let s = elem.repr(vm)?;
237-
str_parts.push(s.as_str().to_owned());
238-
}
239-
format!("[{}]", str_parts.join(", "))
233+
let s = if zelf.len() == 0 {
234+
"[]".to_owned()
235+
} else if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
236+
collection_repr(None, "[", "]", zelf.borrow_vec().iter(), vm)?
240237
} else {
241238
"[...]".to_owned()
242239
};

‎vm/src/builtins/set.rs

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use crate::{
1414
AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable,
1515
PyComparisonOp, Unconstructible, Unhashable,
1616
},
17+
utils::collection_repr,
1718
vm::{ReprGuard, VirtualMachine},
1819
IdProtocol, PyArithmeticValue, PyClassImpl, PyComparisonValue, PyContext, PyObject,
1920
PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
@@ -225,32 +226,7 @@ impl PySetInner {
225226
}
226227

227228
fn repr(&self, class_name: Option<&str>, vm: &VirtualMachine) -> PyResult<String> {
228-
let mut repr = String::new();
229-
if let Some(name) = class_name {
230-
repr.push_str(name);
231-
repr.push('(');
232-
}
233-
repr.push('{');
234-
{
235-
let mut parts_iter = self.elements().into_iter().map(|o| o.repr(vm));
236-
repr.push_str(
237-
parts_iter
238-
.next()
239-
.transpose()?
240-
.expect("this is not called for empty set")
241-
.as_str(),
242-
);
243-
for part in parts_iter {
244-
repr.push_str(", ");
245-
repr.push_str(part?.as_str());
246-
}
247-
}
248-
repr.push('}');
249-
if class_name.is_some() {
250-
repr.push(')');
251-
}
252-
253-
Ok(repr)
229+
collection_repr(class_name, "{", "}", self.elements().iter(), vm)
254230
}
255231

256232
fn add(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
@@ -549,7 +525,7 @@ impl PySet {
549525
}
550526

551527
#[pymethod(magic)]
552-
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
528+
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
553529
let class = zelf.class();
554530
let borrowed_name = class.name();
555531
let class_name = borrowed_name.deref();
@@ -565,7 +541,7 @@ impl PySet {
565541
} else {
566542
format!("{}(...)", class_name)
567543
};
568-
Ok(vm.ctx.new_str(s).into())
544+
Ok(s)
569545
}
570546

571547
#[pymethod]
@@ -878,7 +854,7 @@ impl PyFrozenSet {
878854
}
879855

880856
#[pymethod(magic)]
881-
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
857+
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
882858
let inner = &zelf.inner;
883859
let class = zelf.class();
884860
let class_name = class.name();
@@ -889,7 +865,7 @@ impl PyFrozenSet {
889865
} else {
890866
format!("{}(...)", class_name)
891867
};
892-
Ok(vm.ctx.new_str(s).into())
868+
Ok(s)
893869
}
894870

895871
#[pymethod(magic)]

‎vm/src/builtins/tuple.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::{
1212
AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable,
1313
Iterable, PyComparisonOp, Unconstructible,
1414
},
15+
utils::collection_repr,
1516
vm::{ReprGuard, VirtualMachine},
1617
IdProtocol, PyArithmeticValue, PyClassImpl, PyComparisonValue, PyContext, PyObject,
1718
PyObjectRef, PyRef, PyResult, PyValue, TransmuteFromObject, TryFromObject, TypeProtocol,
@@ -199,17 +200,13 @@ impl PyTuple {
199200

200201
#[pymethod(magic)]
201202
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
202-
let s = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
203-
let mut str_parts = Vec::with_capacity(zelf.elements.len());
204-
for elem in zelf.elements.iter() {
205-
let s = elem.repr(vm)?;
206-
str_parts.push(s.as_str().to_owned());
207-
}
208-
209-
if str_parts.len() == 1 {
210-
format!("({},)", str_parts[0])
203+
let s = if zelf.len() == 0 {
204+
"()".to_owned()
205+
} else if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
206+
if zelf.len() == 1 {
207+
format!("({},)", zelf.elements[0].repr(vm)?)
211208
} else {
212-
format!("({})", str_parts.join(", "))
209+
collection_repr(None, "(", ")", zelf.elements.iter(), vm)?
213210
}
214211
} else {
215212
"(...)".to_owned()

‎vm/src/stdlib/collections.rs

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ mod _collections {
1717
AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable,
1818
PyComparisonOp, Unhashable,
1919
},
20+
utils::collection_repr,
2021
vm::ReprGuard,
2122
PyComparisonValue, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
2223
VirtualMachine,
2324
};
2425
use crossbeam_utils::atomic::AtomicCell;
25-
use itertools::Itertools;
2626
use std::cmp::max;
2727
use std::collections::VecDeque;
2828

@@ -363,26 +363,23 @@ mod _collections {
363363

364364
#[pymethod(magic)]
365365
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
366-
let repr = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
367-
let deque = zelf.borrow_deque().clone();
368-
let elements = deque
369-
.iter()
370-
.map(|obj| obj.repr(vm))
371-
.collect::<Result<Vec<_>, _>>()?;
372-
let maxlen = zelf
373-
.maxlen
374-
.map(|maxlen| format!(", maxlen={}", maxlen))
375-
.unwrap_or_default();
376-
format!(
377-
"{}([{}]{})",
378-
zelf.class().name(),
379-
elements.into_iter().format(", "),
380-
maxlen
381-
)
366+
let deque = zelf.borrow_deque().clone();
367+
let class = zelf.class();
368+
let class_name = class.name();
369+
let closing_part = zelf
370+
.maxlen
371+
.map(|maxlen| format!("], maxlen={}", maxlen))
372+
.unwrap_or_else(|| "]".to_owned());
373+
374+
let s = if zelf.len() == 0 {
375+
format!("{}([{})", class_name, closing_part)
376+
} else if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
377+
collection_repr(Some(&class_name), "[", &closing_part, deque.iter(), vm)?
382378
} else {
383379
"[...]".to_owned()
384380
};
385-
Ok(repr)
381+
382+
Ok(s)
386383
}
387384

388385
#[pymethod(magic)]

‎vm/src/utils.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,41 @@ impl ToCString for PyStr {
127127
std::ffi::CString::new(self.as_ref()).map_err(|err| err.into_pyexception(vm))
128128
}
129129
}
130+
131+
pub(crate) fn collection_repr<'a, I>(
132+
class_name: Option<&str>,
133+
prefix: &str,
134+
suffix: &str,
135+
iter: I,
136+
vm: &VirtualMachine,
137+
) -> PyResult<String>
138+
where
139+
I: std::iter::Iterator<Item = &'a PyObjectRef>,
140+
{
141+
let mut repr = String::new();
142+
if let Some(name) = class_name {
143+
repr.push_str(name);
144+
repr.push('(');
145+
}
146+
repr.push_str(prefix);
147+
{
148+
let mut parts_iter = iter.map(|o| o.repr(vm));
149+
repr.push_str(
150+
parts_iter
151+
.next()
152+
.transpose()?
153+
.expect("this is not called for empty collection")
154+
.as_str(),
155+
);
156+
for part in parts_iter {
157+
repr.push_str(", ");
158+
repr.push_str(part?.as_str());
159+
}
160+
}
161+
repr.push_str(suffix);
162+
if class_name.is_some() {
163+
repr.push(')');
164+
}
165+
166+
Ok(repr)
167+
}

0 commit comments

Comments
 (0)