diff --git a/vm/src/builtins/list.rs b/vm/src/builtins/list.rs index 1089648dca..94b0548da4 100644 --- a/vm/src/builtins/list.rs +++ b/vm/src/builtins/list.rs @@ -12,6 +12,7 @@ use crate::{ AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable, }, + utils::collection_repr, vm::{ReprGuard, VirtualMachine}, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyObjectView, PyObjectWrap, PyRef, PyResult, PyValue, TypeProtocol, @@ -229,14 +230,10 @@ impl PyList { #[pymethod(magic)] fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { - let s = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { - let elements = zelf.borrow_vec().to_vec(); - let mut str_parts = Vec::with_capacity(elements.len()); - for elem in elements.iter() { - let s = elem.repr(vm)?; - str_parts.push(s.as_str().to_owned()); - } - format!("[{}]", str_parts.join(", ")) + let s = if zelf.len() == 0 { + "[]".to_owned() + } else if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { + collection_repr(None, "[", "]", zelf.borrow_vec().iter(), vm)? } else { "[...]".to_owned() }; diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index 2183082368..e77dd45dff 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -14,6 +14,7 @@ use crate::{ AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable, }, + utils::collection_repr, vm::{ReprGuard, VirtualMachine}, IdProtocol, PyArithmeticValue, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, @@ -225,32 +226,7 @@ impl PySetInner { } fn repr(&self, class_name: Option<&str>, vm: &VirtualMachine) -> PyResult { - let mut repr = String::new(); - if let Some(name) = class_name { - repr.push_str(name); - repr.push('('); - } - repr.push('{'); - { - let mut parts_iter = self.elements().into_iter().map(|o| o.repr(vm)); - repr.push_str( - parts_iter - .next() - .transpose()? - .expect("this is not called for empty set") - .as_str(), - ); - for part in parts_iter { - repr.push_str(", "); - repr.push_str(part?.as_str()); - } - } - repr.push('}'); - if class_name.is_some() { - repr.push(')'); - } - - Ok(repr) + collection_repr(class_name, "{", "}", self.elements().iter(), vm) } fn add(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { @@ -549,7 +525,7 @@ impl PySet { } #[pymethod(magic)] - fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { + fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { let class = zelf.class(); let borrowed_name = class.name(); let class_name = borrowed_name.deref(); @@ -565,7 +541,7 @@ impl PySet { } else { format!("{}(...)", class_name) }; - Ok(vm.ctx.new_str(s).into()) + Ok(s) } #[pymethod] @@ -878,7 +854,7 @@ impl PyFrozenSet { } #[pymethod(magic)] - fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { + fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { let inner = &zelf.inner; let class = zelf.class(); let class_name = class.name(); @@ -889,7 +865,7 @@ impl PyFrozenSet { } else { format!("{}(...)", class_name) }; - Ok(vm.ctx.new_str(s).into()) + Ok(s) } #[pymethod(magic)] diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index 67c6fc3b1e..c6bc0fca1d 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -12,6 +12,7 @@ use crate::{ AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, }, + utils::collection_repr, vm::{ReprGuard, VirtualMachine}, IdProtocol, PyArithmeticValue, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TransmuteFromObject, TryFromObject, TypeProtocol, @@ -199,17 +200,13 @@ impl PyTuple { #[pymethod(magic)] fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { - let s = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { - let mut str_parts = Vec::with_capacity(zelf.elements.len()); - for elem in zelf.elements.iter() { - let s = elem.repr(vm)?; - str_parts.push(s.as_str().to_owned()); - } - - if str_parts.len() == 1 { - format!("({},)", str_parts[0]) + let s = if zelf.len() == 0 { + "()".to_owned() + } else if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { + if zelf.len() == 1 { + format!("({},)", zelf.elements[0].repr(vm)?) } else { - format!("({})", str_parts.join(", ")) + collection_repr(None, "(", ")", zelf.elements.iter(), vm)? } } else { "(...)".to_owned() diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 9c952211dd..2f60533620 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -17,12 +17,12 @@ mod _collections { AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unhashable, }, + utils::collection_repr, vm::ReprGuard, PyComparisonValue, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, }; use crossbeam_utils::atomic::AtomicCell; - use itertools::Itertools; use std::cmp::max; use std::collections::VecDeque; @@ -363,26 +363,23 @@ mod _collections { #[pymethod(magic)] fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { - let repr = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { - let deque = zelf.borrow_deque().clone(); - let elements = deque - .iter() - .map(|obj| obj.repr(vm)) - .collect::, _>>()?; - let maxlen = zelf - .maxlen - .map(|maxlen| format!(", maxlen={}", maxlen)) - .unwrap_or_default(); - format!( - "{}([{}]{})", - zelf.class().name(), - elements.into_iter().format(", "), - maxlen - ) + let deque = zelf.borrow_deque().clone(); + let class = zelf.class(); + let class_name = class.name(); + let closing_part = zelf + .maxlen + .map(|maxlen| format!("], maxlen={}", maxlen)) + .unwrap_or_else(|| "]".to_owned()); + + let s = if zelf.len() == 0 { + format!("{}([{})", class_name, closing_part) + } else if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { + collection_repr(Some(&class_name), "[", &closing_part, deque.iter(), vm)? } else { "[...]".to_owned() }; - Ok(repr) + + Ok(s) } #[pymethod(magic)] diff --git a/vm/src/utils.rs b/vm/src/utils.rs index d7aa50f3de..567e517fce 100644 --- a/vm/src/utils.rs +++ b/vm/src/utils.rs @@ -127,3 +127,41 @@ impl ToCString for PyStr { std::ffi::CString::new(self.as_ref()).map_err(|err| err.into_pyexception(vm)) } } + +pub(crate) fn collection_repr<'a, I>( + class_name: Option<&str>, + prefix: &str, + suffix: &str, + iter: I, + vm: &VirtualMachine, +) -> PyResult +where + I: std::iter::Iterator, +{ + let mut repr = String::new(); + if let Some(name) = class_name { + repr.push_str(name); + repr.push('('); + } + repr.push_str(prefix); + { + let mut parts_iter = iter.map(|o| o.repr(vm)); + repr.push_str( + parts_iter + .next() + .transpose()? + .expect("this is not called for empty collection") + .as_str(), + ); + for part in parts_iter { + repr.push_str(", "); + repr.push_str(part?.as_str()); + } + } + repr.push_str(suffix); + if class_name.is_some() { + repr.push(')'); + } + + Ok(repr) +}