Merge pull request #426 from skinny121/recursive_repr

Fixed #191, fixes recursive repr() for the standard collections
This commit is contained in:
Windel Bouwman
2019-02-10 08:35:02 +01:00
committed by GitHub
14 changed files with 117 additions and 106 deletions

1
Cargo.lock generated
View File

@@ -708,6 +708,7 @@ dependencies = [
"bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
"byteorder 1.2.6 (registry+https://github.com/rust-lang/crates.io-index)",
"caseless 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
"num-bigint 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
"num-complex 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",

View File

@@ -4,3 +4,7 @@ assert len({}) == 0
assert len({"a": "b"}) == 1
assert len({"a": "b", "b": 1}) == 2
assert len({"a": "b", "b": 1, "a" + "b": 2*2}) == 3
d = {}
d['a'] = d
assert repr(d) == "{'a': {...}}"

View File

@@ -38,3 +38,7 @@ except IndexError:
pass
else:
assert False, "IndexError was not raised"
recursive = []
recursive.append(recursive)
assert repr(recursive) == "[[...]]"

View File

@@ -24,3 +24,16 @@ assert not set([1,3]).issubset(set([1,2]))
assert set([1,2]) < set([1,2,3])
assert not set([1,2]) < set([1,2])
assert not set([1,3]) < set([1,2])
class Hashable(object):
def __init__(self, obj):
self.obj = obj
def __repr__(self):
return repr(self.obj)
recursive = set()
recursive.add(Hashable(recursive))
assert repr(recursive) == "{set(...)}"

View File

@@ -19,3 +19,8 @@ assert x > y, "tuple __gt__ failed"
b = (1,2,3)
assert b.index(2) == 1
recursive_list = []
recursive = (recursive_list,)
recursive_list.append(recursive)
assert repr(recursive) == "([(...)],)"

View File

@@ -20,3 +20,4 @@ regex = "1"
statrs = "0.10.0"
caseless = "0.2.1"
unicode-segmentation = "1.2.1"
lazy_static = "^1.0.1"

View File

@@ -1085,7 +1085,7 @@ impl fmt::Debug for Frame {
let stack_str = self
.stack
.iter()
.map(|elem| format!("\n > {}", elem.borrow().str()))
.map(|elem| format!("\n > {:?}", elem.borrow()))
.collect::<Vec<_>>()
.join("");
let block_str = self
@@ -1099,9 +1099,7 @@ impl fmt::Debug for Frame {
PyObjectPayload::Dict { ref elements } => {
objdict::get_key_value_pairs_from_content(elements)
.iter()
.map(|elem| {
format!("\n {} = {}", elem.0.borrow().str(), elem.1.borrow().str())
})
.map(|elem| format!("\n {:?} = {:?}", elem.0.borrow(), elem.1.borrow()))
.collect::<Vec<_>>()
.join("")
}

View File

@@ -8,6 +8,8 @@
#[macro_use]
extern crate bitflags;
#[macro_use]
extern crate lazy_static;
#[macro_use]
extern crate log;
// extern crate env_logger;
extern crate num_bigint;

View File

@@ -1,7 +1,7 @@
use super::super::pyobject::{
PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult, TypeProtocol,
};
use super::super::vm::VirtualMachine;
use super::super::vm::{ReprGuard, VirtualMachine};
use super::objiter;
use super::objstr;
use super::objtype;
@@ -158,16 +158,21 @@ fn dict_len(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
fn dict_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(dict_obj, Some(vm.ctx.dict_type()))]);
let elements = get_key_value_pairs(dict_obj);
let mut str_parts = vec![];
for (key, value) in elements {
let s = vm.to_repr(&value)?;
let key_str = objstr::get_value(&key);
let value_str = objstr::get_value(&s);
str_parts.push(format!("{}: {}", key_str, value_str));
}
let s = if let Some(_guard) = ReprGuard::enter(dict_obj) {
let elements = get_key_value_pairs(dict_obj);
let mut str_parts = vec![];
for (key, value) in elements {
let key_repr = vm.to_repr(&key)?;
let value_repr = vm.to_repr(&value)?;
let key_str = objstr::get_value(&key_repr);
let value_str = objstr::get_value(&value_repr);
str_parts.push(format!("{}: {}", key_str, value_str));
}
let s = format!("{{{}}}", str_parts.join(", "));
format!("{{{}}}", str_parts.join(", "))
} else {
"{...}".to_string()
};
Ok(vm.new_str(s))
}

View File

@@ -1,7 +1,7 @@
use super::super::pyobject::{
PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult, TypeProtocol,
};
use super::super::vm::VirtualMachine;
use super::super::vm::{ReprGuard, VirtualMachine};
use super::objbool;
use super::objint;
use super::objsequence::{
@@ -184,14 +184,18 @@ fn list_add(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
fn list_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(o, Some(vm.ctx.list_type()))]);
let elements = get_elements(o);
let mut str_parts = vec![];
for elem in elements.iter() {
let s = vm.to_repr(elem)?;
str_parts.push(objstr::get_value(&s));
}
let s = if let Some(_guard) = ReprGuard::enter(o) {
let elements = get_elements(o);
let mut str_parts = vec![];
for elem in elements.iter() {
let s = vm.to_repr(elem)?;
str_parts.push(objstr::get_value(&s));
}
format!("[{}]", str_parts.join(", "))
} else {
"[...]".to_string()
};
let s = format!("[{}]", str_parts.join(", "));
Ok(vm.new_str(s))
}

View File

@@ -6,7 +6,7 @@ use super::super::pyobject::{
IdProtocol, PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult,
TypeProtocol,
};
use super::super::vm::VirtualMachine;
use super::super::vm::{ReprGuard, VirtualMachine};
use super::objbool;
use super::objiter;
use super::objstr;
@@ -94,7 +94,7 @@ fn set_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
let elements = get_elements(o);
let s = if elements.is_empty() {
"set()".to_string()
} else {
} else if let Some(_guard) = ReprGuard::enter(o) {
let mut str_parts = vec![];
for elem in elements.values() {
let part = vm.to_repr(elem)?;
@@ -102,6 +102,8 @@ fn set_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
}
format!("{{{}}}", str_parts.join(", "))
} else {
"set(...)".to_string()
};
Ok(vm.new_str(s))
}

View File

@@ -1,7 +1,7 @@
use super::super::pyobject::{
PyContext, PyFuncArgs, PyObject, PyObjectPayload, PyObjectRef, PyResult, TypeProtocol,
};
use super::super::vm::VirtualMachine;
use super::super::vm::{ReprGuard, VirtualMachine};
use super::objbool;
use super::objint;
use super::objsequence::{
@@ -213,18 +213,22 @@ fn tuple_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
fn tuple_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(zelf, Some(vm.ctx.tuple_type()))]);
let elements = get_elements(zelf);
let s = if let Some(_guard) = ReprGuard::enter(zelf) {
let elements = get_elements(zelf);
let mut str_parts = vec![];
for elem in elements.iter() {
let s = vm.to_repr(elem)?;
str_parts.push(objstr::get_value(&s));
}
let mut str_parts = vec![];
for elem in elements.iter() {
let s = vm.to_repr(elem)?;
str_parts.push(objstr::get_value(&s));
}
let s = if str_parts.len() == 1 {
format!("({},)", str_parts[0])
if str_parts.len() == 1 {
format!("({},)", str_parts[0])
} else {
format!("({})", str_parts.join(", "))
}
} else {
format!("({})", str_parts.join(", "))
"(...)".to_string()
};
Ok(vm.new_str(s))
}

View File

@@ -1032,76 +1032,6 @@ impl PyObject {
.into_ref()
}
/// Deprecated method, please call `vm.to_pystr`
pub fn str(&self) -> String {
match self.payload {
PyObjectPayload::String { ref value } => value.clone(),
PyObjectPayload::Integer { ref value } => format!("{:?}", value),
PyObjectPayload::Float { ref value } => format!("{:?}", value),
PyObjectPayload::Complex { ref value } => format!("{:?}", value),
PyObjectPayload::Bytes { ref value } => format!("b'{:?}'", value),
PyObjectPayload::MemoryView { ref obj } => format!("b'{:?}'", obj),
PyObjectPayload::Sequence { ref elements } => format!(
"(/[{}]/)",
elements
.iter()
.map(|elem| elem.borrow().str())
.collect::<Vec<_>>()
.join(", ")
),
PyObjectPayload::Dict { ref elements } => format!(
"{{ {} }}",
elements
.iter()
.map(|elem| format!("{}: ...", elem.0))
.collect::<Vec<_>>()
.join(", ")
),
PyObjectPayload::Set { ref elements } => format!(
"{{ {} }}",
elements
.iter()
.map(|elem| elem.1.borrow().str())
.collect::<Vec<_>>()
.join(", ")
),
PyObjectPayload::WeakRef { .. } => String::from("weakref"),
PyObjectPayload::None => String::from("None"),
PyObjectPayload::Class {
ref name,
dict: ref _dict,
..
} => format!("<class '{}'>", name),
PyObjectPayload::Instance { .. } => "<instance>".to_string(),
PyObjectPayload::Code { .. } => "<code>".to_string(),
PyObjectPayload::Function { .. } => "<func>".to_string(),
PyObjectPayload::Generator { .. } => "<generator>".to_string(),
PyObjectPayload::Frame { .. } => "<frame>".to_string(),
PyObjectPayload::BoundMethod { .. } => "<bound-method>".to_string(),
PyObjectPayload::RustFunction { .. } => "<rustfunc>".to_string(),
PyObjectPayload::Module { ref name, .. } => format!("<module '{}'>", name),
PyObjectPayload::Scope { ref scope } => format!("<scope '{:?}'>", scope),
PyObjectPayload::Slice {
ref start,
ref stop,
ref step,
} => format!("<slice '{:?}:{:?}:{:?}'>", start, stop, step),
PyObjectPayload::Range { ref range } => format!("<range '{:?}'>", range),
PyObjectPayload::Iterator {
ref position,
ref iterated_obj,
} => format!(
"<iter pos {} in {}>",
position,
iterated_obj.borrow_mut().str()
),
PyObjectPayload::EnumerateIterator { .. } => format!("<enumerate>"),
PyObjectPayload::FilterIterator { .. } => format!("<filter>"),
PyObjectPayload::MapIterator { .. } => format!("<map>"),
PyObjectPayload::ZipIterator { .. } => format!("<zip>"),
}
}
// Move this object into a reference object, transferring ownership.
pub fn into_ref(self) -> PyObjectRef {
Rc::new(RefCell::new(self))

View File

@@ -7,6 +7,8 @@
extern crate rustpython_parser;
use std::collections::hash_map::HashMap;
use std::collections::hash_set::HashSet;
use std::sync::{Mutex, MutexGuard};
use super::builtins;
use super::bytecode;
@@ -18,8 +20,8 @@ use super::obj::objsequence;
use super::obj::objstr;
use super::obj::objtype;
use super::pyobject::{
AttributeProtocol, DictProtocol, PyContext, PyFuncArgs, PyObjectPayload, PyObjectRef, PyResult,
TypeProtocol,
AttributeProtocol, DictProtocol, IdProtocol, PyContext, PyFuncArgs, PyObjectPayload,
PyObjectRef, PyResult, TypeProtocol,
};
use super::stdlib;
use super::sysmodule;
@@ -615,6 +617,42 @@ impl VirtualMachine {
}
}
lazy_static! {
static ref REPR_GUARDS: Mutex<HashSet<usize>> = { Mutex::new(HashSet::new()) };
}
pub struct ReprGuard {
id: usize,
}
/// A guard to protect repr methods from recursion into itself,
impl ReprGuard {
fn get_guards<'a>() -> MutexGuard<'a, HashSet<usize>> {
REPR_GUARDS.lock().expect("ReprGuard lock poisoned")
}
/// Returns None if the guard against 'obj' is still held otherwise returns the guard. The guard
/// which is released if dropped.
pub fn enter(obj: &PyObjectRef) -> Option<ReprGuard> {
let mut guards = ReprGuard::get_guards();
// Should this be a flag on the obj itself? putting it in a global variable for now until it
// decided the form of the PyObject. https://github.com/RustPython/RustPython/issues/371
let id = obj.get_id();
if guards.contains(&id) {
return None;
}
guards.insert(id);
Some(ReprGuard { id })
}
}
impl Drop for ReprGuard {
fn drop(&mut self) {
ReprGuard::get_guards().remove(&self.id);
}
}
#[cfg(test)]
mod tests {
use super::super::obj::{objint, objstr};