Skip to content

refactor: add a repr helper function for the collections #3471

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
merged 2 commits into from
Mar 24, 2022
Merged

refactor: add a repr helper function for the collections #3471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9851643
refactor: add repr helper function for the collections
TeamTamoad Nov 29, 2021
0f4d6cd
fix: correct deque repr result and fix hanging problem
TeamTamoad Jan 21, 2022
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
13 changes: 5 additions & 8 deletions vm/src/builtins/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -229,14 +230,10 @@ impl PyList {

#[pymethod(magic)]
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
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()
};
Expand Down
36 changes: 6 additions & 30 deletions vm/src/builtins/set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -225,32 +226,7 @@ impl PySetInner {
}

fn repr(&self, class_name: Option<&str>, vm: &VirtualMachine) -> PyResult<String> {
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<()> {
Expand Down Expand Up @@ -549,7 +525,7 @@ impl PySet {
}

#[pymethod(magic)]
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
let class = zelf.class();
let borrowed_name = class.name();
let class_name = borrowed_name.deref();
Expand All @@ -565,7 +541,7 @@ impl PySet {
} else {
format!("{}(...)", class_name)
};
Ok(vm.ctx.new_str(s).into())
Ok(s)
}

#[pymethod]
Expand Down Expand Up @@ -878,7 +854,7 @@ impl PyFrozenSet {
}

#[pymethod(magic)]
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
let inner = &zelf.inner;
let class = zelf.class();
let class_name = class.name();
Expand All @@ -889,7 +865,7 @@ impl PyFrozenSet {
} else {
format!("{}(...)", class_name)
};
Ok(vm.ctx.new_str(s).into())
Ok(s)
}

#[pymethod(magic)]
Expand Down
17 changes: 7 additions & 10 deletions vm/src/builtins/tuple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -199,17 +200,13 @@ impl PyTuple {

#[pymethod(magic)]
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
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()
Expand Down
33 changes: 15 additions & 18 deletions vm/src/stdlib/collections.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -363,26 +363,23 @@ mod _collections {

#[pymethod(magic)]
fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
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::<Result<Vec<_>, _>>()?;
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)]
Expand Down
38 changes: 38 additions & 0 deletions vm/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>
where
I: std::iter::Iterator<Item = &'a PyObjectRef>,
{
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)
}