diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index 9e245fe6d..6f99d7358 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -16,11 +16,12 @@ use crate::{ }, protocol::{PyIterIter, PyIterReturn, PyMappingMethods, PySequenceMethods}, pyclass::{PyClassDef, PyClassImpl}, + recursion::ReprGuard, types::{ AsMapping, AsSequence, Callable, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable, }, - vm::{ReprGuard, VirtualMachine}, + vm::VirtualMachine, AsObject, Py, PyContext, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, }; use rustpython_common::lock::PyMutex; diff --git a/vm/src/builtins/list.rs b/vm/src/builtins/list.rs index dbb1c5104..e4fcb4138 100644 --- a/vm/src/builtins/list.rs +++ b/vm/src/builtins/list.rs @@ -8,6 +8,7 @@ use crate::{ function::{FuncArgs, OptionalArg, PyComparisonValue}, protocol::{PyIterReturn, PyMappingMethods, PySequence, PySequenceMethods}, pyclass::PyClassImpl, + recursion::ReprGuard, sequence::{MutObjectSequenceOp, ObjectSequenceOp, SequenceMutOp, SequenceOp}, sliceable::{saturate_index, SequenceIndex, SliceableSequenceMutOp, SliceableSequenceOp}, types::{ @@ -15,7 +16,7 @@ use crate::{ Iterable, PyComparisonOp, Unconstructible, Unhashable, }, utils::collection_repr, - vm::{ReprGuard, VirtualMachine}, + vm::VirtualMachine, AsObject, Py, PyContext, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, }; use std::{borrow::Cow, fmt, ops::DerefMut}; diff --git a/vm/src/builtins/namespace.rs b/vm/src/builtins/namespace.rs index f50120dcf..f7e8a9bba 100644 --- a/vm/src/builtins/namespace.rs +++ b/vm/src/builtins/namespace.rs @@ -3,8 +3,8 @@ use crate::{ builtins::PyDict, function::{FuncArgs, PyComparisonValue}, pyclass::PyClassImpl, + recursion::ReprGuard, types::{Comparable, Constructor, PyComparisonOp}, - vm::ReprGuard, AsObject, PyContext, PyObject, PyPayload, PyRef, PyResult, VirtualMachine, }; diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index 002c20f06..a29d46f77 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -11,12 +11,13 @@ use crate::{ function::{ArgIterable, FuncArgs, OptionalArg, PosArgs, PyArithmeticValue, PyComparisonValue}, protocol::{PyIterReturn, PySequenceMethods}, pyclass::PyClassImpl, + recursion::ReprGuard, types::{ AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable, }, utils::collection_repr, - vm::{ReprGuard, VirtualMachine}, + vm::VirtualMachine, AsObject, PyContext, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, }; use std::borrow::Cow; diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index 09eee110a..560e25406 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -5,6 +5,7 @@ use crate::{ function::{OptionalArg, PyArithmeticValue, PyComparisonValue}, protocol::{PyIterReturn, PyMappingMethods, PySequenceMethods}, pyclass::PyClassImpl, + recursion::ReprGuard, sequence::{ObjectSequenceOp, SequenceOp}, sliceable::{SequenceIndex, SliceableSequenceOp}, stdlib::sys, @@ -13,7 +14,7 @@ use crate::{ Iterable, PyComparisonOp, Unconstructible, }, utils::collection_repr, - vm::{ReprGuard, VirtualMachine}, + vm::VirtualMachine, AsObject, PyContext, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, }; use std::{borrow::Cow, fmt, marker::PhantomData}; diff --git a/vm/src/lib.rs b/vm/src/lib.rs index d0ba7965e..1a65bacbd 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -67,6 +67,7 @@ pub mod py_io; pub mod py_serde; pub mod pyclass; pub mod readline; +pub mod recursion; pub mod scope; pub mod sequence; pub mod signal; diff --git a/vm/src/recursion.rs b/vm/src/recursion.rs new file mode 100644 index 000000000..c7b75e588 --- /dev/null +++ b/vm/src/recursion.rs @@ -0,0 +1,30 @@ +use crate::{AsObject, PyObject, VirtualMachine}; + +pub struct ReprGuard<'vm> { + vm: &'vm VirtualMachine, + id: usize, +} + +/// A guard to protect repr methods from recursion into itself, +impl<'vm> ReprGuard<'vm> { + /// Returns None if the guard against 'obj' is still held otherwise returns the guard. The guard + /// which is released if dropped. + pub fn enter(vm: &'vm VirtualMachine, obj: &PyObject) -> Option { + let mut guards = vm.repr_guards.borrow_mut(); + + // Should this be a flag on the obj itself? putting it in a global variable for now until it + // decided the form of PyObject. https://github.com/RustPython/RustPython/issues/371 + let id = obj.get_id(); + if guards.contains(&id) { + return None; + } + guards.insert(id); + Some(ReprGuard { vm, id }) + } +} + +impl<'vm> Drop for ReprGuard<'vm> { + fn drop(&mut self) { + self.vm.repr_guards.borrow_mut().remove(&self.id); + } +} diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 7400628ff..4d94fbea5 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -10,6 +10,7 @@ mod _collections { common::lock::{PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard}, function::{FuncArgs, KwArgs, OptionalArg, PyComparisonValue}, protocol::{PyIterReturn, PySequenceMethods}, + recursion::ReprGuard, sequence::{MutObjectSequenceOp, ObjectSequenceOp}, sliceable, sliceable::saturate_index, @@ -18,7 +19,6 @@ mod _collections { PyComparisonOp, Unhashable, }, utils::collection_repr, - vm::ReprGuard, AsObject, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, }; use crossbeam_utils::atomic::AtomicCell; diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index ffc7b6c3d..2d337de71 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -111,9 +111,10 @@ mod _io { BufferDescriptor, BufferMethods, BufferResizeGuard, PyBuffer, PyIterReturn, VecBuffer, }, pyclass::StaticType, + recursion::ReprGuard, types::{Constructor, Destructor, IterNext, Iterable}, utils::Either, - vm::{ReprGuard, VirtualMachine}, + vm::VirtualMachine, AsObject, PyContext, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromBorrowedObject, TryFromObject, }; diff --git a/vm/src/stdlib/operator.rs b/vm/src/stdlib/operator.rs index 9fe571cce..2aedb6cc2 100644 --- a/vm/src/stdlib/operator.rs +++ b/vm/src/stdlib/operator.rs @@ -14,12 +14,12 @@ mod _operator { builtins::{PyInt, PyIntRef, PyStrRef, PyTupleRef, PyTypeRef}, function::{ArgBytesLike, FuncArgs, KwArgs, OptionalArg}, protocol::PyIter, + recursion::ReprGuard, types::{ Callable, Constructor, PyComparisonOp::{Eq, Ge, Gt, Le, Lt, Ne}, }, utils::Either, - vm::ReprGuard, AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, }; diff --git a/vm/src/stdlib/os.rs b/vm/src/stdlib/os.rs index 9569db0f1..1b6641f94 100644 --- a/vm/src/stdlib/os.rs +++ b/vm/src/stdlib/os.rs @@ -412,10 +412,11 @@ pub(super) mod _os { crt_fd::{Fd, Offset}, function::{ArgBytesLike, FuncArgs, OptionalArg}, protocol::PyIterReturn, + recursion::ReprGuard, suppress_iph, types::{IterNext, IterNextIterable, PyStructSequence}, utils::Either, - vm::{ReprGuard, VirtualMachine}, + vm::VirtualMachine, AsObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, }; use crossbeam_utils::atomic::AtomicCell; diff --git a/vm/src/types/structseq.rs b/vm/src/types/structseq.rs index 2bb47599f..375eb27d3 100644 --- a/vm/src/types/structseq.rs +++ b/vm/src/types/structseq.rs @@ -46,24 +46,25 @@ pub trait PyStructSequence: StaticType + PyClassImpl + Sized + 'static { let s = value.repr(vm)?; Ok(format!("{}={}", name, s)) }; - let (body, suffix) = - if let Some(_guard) = rustpython_vm::vm::ReprGuard::enter(vm, zelf.as_object()) { - if Self::FIELD_NAMES.len() == 1 { - let value = zelf.as_slice().first().unwrap(); - let formatted = format_field((value, Self::FIELD_NAMES[0]))?; - (formatted, ",") - } else { - let fields: PyResult> = zelf - .as_slice() - .iter() - .zip(Self::FIELD_NAMES.iter().copied()) - .map(format_field) - .collect(); - (fields?.join(", "), "") - } + let (body, suffix) = if let Some(_guard) = + rustpython_vm::recursion::ReprGuard::enter(vm, zelf.as_object()) + { + if Self::FIELD_NAMES.len() == 1 { + let value = zelf.as_slice().first().unwrap(); + let formatted = format_field((value, Self::FIELD_NAMES[0]))?; + (formatted, ",") } else { - (String::new(), "...") - }; + let fields: PyResult> = zelf + .as_slice() + .iter() + .zip(Self::FIELD_NAMES.iter().copied()) + .map(format_field) + .collect(); + (fields?.join(", "), "") + } + } else { + (String::new(), "...") + }; Ok(format!("{}({}{})", Self::TP_NAME, body, suffix)) } diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index 8edcfd336..0c9680030 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -750,32 +750,3 @@ impl VirtualMachine { .map(|code| PyCode::new(self.map_codeobj(code)).into_ref(self)) } } - -pub struct ReprGuard<'vm> { - vm: &'vm VirtualMachine, - id: usize, -} - -/// A guard to protect repr methods from recursion into itself, -impl<'vm> ReprGuard<'vm> { - /// Returns None if the guard against 'obj' is still held otherwise returns the guard. The guard - /// which is released if dropped. - pub fn enter(vm: &'vm VirtualMachine, obj: &PyObject) -> Option { - let mut guards = vm.repr_guards.borrow_mut(); - - // Should this be a flag on the obj itself? putting it in a global variable for now until it - // decided the form of PyObject. https://github.com/RustPython/RustPython/issues/371 - let id = obj.get_id(); - if guards.contains(&id) { - return None; - } - guards.insert(id); - Some(ReprGuard { vm, id }) - } -} - -impl<'vm> Drop for ReprGuard<'vm> { - fn drop(&mut self) { - self.vm.repr_guards.borrow_mut().remove(&self.id); - } -}