diff --git a/Cargo.lock b/Cargo.lock index aa31678ad..ec5a35a00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -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)", diff --git a/tests/snippets/builtin_dict.py b/tests/snippets/builtin_dict.py index d401aabca..a57541fa5 100644 --- a/tests/snippets/builtin_dict.py +++ b/tests/snippets/builtin_dict.py @@ -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': {...}}" diff --git a/tests/snippets/list.py b/tests/snippets/list.py index dba809868..97e689be2 100644 --- a/tests/snippets/list.py +++ b/tests/snippets/list.py @@ -38,3 +38,7 @@ except IndexError: pass else: assert False, "IndexError was not raised" + +recursive = [] +recursive.append(recursive) +assert repr(recursive) == "[[...]]" diff --git a/tests/snippets/set.py b/tests/snippets/set.py index 8b31c7c23..706abaeb1 100644 --- a/tests/snippets/set.py +++ b/tests/snippets/set.py @@ -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(...)}" diff --git a/tests/snippets/tuple.py b/tests/snippets/tuple.py index eb5102fa3..f45aed3c6 100644 --- a/tests/snippets/tuple.py +++ b/tests/snippets/tuple.py @@ -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) == "([(...)],)" diff --git a/vm/Cargo.toml b/vm/Cargo.toml index fe278edb7..a4fecade7 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -20,3 +20,4 @@ regex = "1" statrs = "0.10.0" caseless = "0.2.1" unicode-segmentation = "1.2.1" +lazy_static = "^1.0.1" diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 474b6d890..d3f63e761 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -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::>() .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::>() .join("") } diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 107ac3896..53f2bf756 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -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; diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs index 8a3e77ff8..0839bbdaf 100644 --- a/vm/src/obj/objdict.rs +++ b/vm/src/obj/objdict.rs @@ -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)) } diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 369ed9fc5..be30b672e 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -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)) } diff --git a/vm/src/obj/objset.rs b/vm/src/obj/objset.rs index 2648fb4f3..8967cf22a 100644 --- a/vm/src/obj/objset.rs +++ b/vm/src/obj/objset.rs @@ -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)) } diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 2d99db2d4..3b7646674 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -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)) } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 212757671..c5968f1dc 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -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::>() - .join(", ") - ), - PyObjectPayload::Dict { ref elements } => format!( - "{{ {} }}", - elements - .iter() - .map(|elem| format!("{}: ...", elem.0)) - .collect::>() - .join(", ") - ), - PyObjectPayload::Set { ref elements } => format!( - "{{ {} }}", - elements - .iter() - .map(|elem| elem.1.borrow().str()) - .collect::>() - .join(", ") - ), - PyObjectPayload::WeakRef { .. } => String::from("weakref"), - PyObjectPayload::None => String::from("None"), - PyObjectPayload::Class { - ref name, - dict: ref _dict, - .. - } => format!("", name), - PyObjectPayload::Instance { .. } => "".to_string(), - PyObjectPayload::Code { .. } => "".to_string(), - PyObjectPayload::Function { .. } => "".to_string(), - PyObjectPayload::Generator { .. } => "".to_string(), - PyObjectPayload::Frame { .. } => "".to_string(), - PyObjectPayload::BoundMethod { .. } => "".to_string(), - PyObjectPayload::RustFunction { .. } => "".to_string(), - PyObjectPayload::Module { ref name, .. } => format!("", name), - PyObjectPayload::Scope { ref scope } => format!("", scope), - PyObjectPayload::Slice { - ref start, - ref stop, - ref step, - } => format!("", start, stop, step), - PyObjectPayload::Range { ref range } => format!("", range), - PyObjectPayload::Iterator { - ref position, - ref iterated_obj, - } => format!( - "", - position, - iterated_obj.borrow_mut().str() - ), - PyObjectPayload::EnumerateIterator { .. } => format!(""), - PyObjectPayload::FilterIterator { .. } => format!(""), - PyObjectPayload::MapIterator { .. } => format!(""), - PyObjectPayload::ZipIterator { .. } => format!(""), - } - } - // Move this object into a reference object, transferring ownership. pub fn into_ref(self) -> PyObjectRef { Rc::new(RefCell::new(self)) diff --git a/vm/src/vm.rs b/vm/src/vm.rs index cd7567c8e..5c432ecc0 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -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> = { 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> { + 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 { + 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};