From dcbc4668a865faa41bfc08b93a2e3631e51ac1cb Mon Sep 17 00:00:00 2001 From: Jeong Yunwon Date: Wed, 27 Apr 2022 02:29:14 +0900 Subject: [PATCH] Constructor and Initializer for PyDeque --- vm/src/stdlib/collections.rs | 149 +++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 70 deletions(-) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 471481359..74a7155f9 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -15,8 +15,8 @@ mod _collections { sliceable, sliceable::saturate_index, types::{ - AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, - PyComparisonOp, Unhashable, + AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext, IterNextIterable, + Iterable, PyComparisonOp, Unhashable, }, utils::collection_repr, AsObject, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, @@ -54,75 +54,11 @@ mod _collections { } } - #[pyimpl(flags(BASETYPE), with(AsSequence, Comparable, Hashable, Iterable))] + #[pyimpl( + flags(BASETYPE), + with(Constructor, Initializer, AsSequence, Comparable, Hashable, Iterable) + )] impl PyDeque { - #[pyslot] - fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult { - PyDeque::default() - .into_ref_with_type(vm, cls) - .map(Into::into) - } - - #[pyslot] - #[pymethod(magic)] - fn init(zelf: PyObjectRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { - let zelf: PyRef = zelf.try_into_value(vm)?; - let PyDequeOptions { iterable, maxlen } = args.bind(vm)?; - - // TODO: This is _basically_ pyobject_to_opt_usize in itertools.rs - // need to move that function elsewhere and refactor usages. - let maxlen = if let Some(obj) = maxlen.into_option() { - if !vm.is_none(&obj) { - let maxlen: isize = obj - .payload::() - .ok_or_else(|| vm.new_type_error("an integer is required.".to_owned()))? - .try_to_primitive(vm)?; - - if maxlen.is_negative() { - return Err(vm.new_value_error("maxlen must be non-negative.".to_owned())); - } - Some(maxlen as usize) - } else { - None - } - } else { - None - }; - - // retrieve elements first to not to make too huge lock - let elements = iterable - .into_option() - .map(|iter| { - let mut elements: Vec = iter.try_to_value(vm)?; - if let Some(maxlen) = maxlen { - elements.drain(..elements.len().saturating_sub(maxlen)); - } - Ok(elements) - }) - .transpose()?; - - // SAFETY: This is hacky part for read-only field - // Because `maxlen` is only mutated from __init__. We can abuse the lock of deque to ensure this is locked enough. - // If we make a single lock of deque not only for extend but also for setting maxlen, it will be safe. - { - let mut deque = zelf.borrow_deque_mut(); - // Clear any previous data present. - deque.clear(); - unsafe { - // `maxlen` is better to be defined as UnsafeCell in common practice, - // but then more type works without any safety benefits - let unsafe_maxlen = - &zelf.maxlen as *const _ as *const std::cell::UnsafeCell>; - *(*unsafe_maxlen).get() = maxlen; - } - if let Some(elements) = elements { - deque.extend(elements); - } - } - - Ok(()) - } - #[pymethod] fn append(&self, obj: PyObjectRef) { self.state.fetch_add(1); @@ -508,6 +444,79 @@ mod _collections { } } + impl Constructor for PyDeque { + type Args = FuncArgs; + + fn py_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult { + PyDeque::default() + .into_ref_with_type(vm, cls) + .map(Into::into) + } + } + + impl Initializer for PyDeque { + type Args = PyDequeOptions; + + fn init( + zelf: PyRef, + PyDequeOptions { iterable, maxlen }: Self::Args, + vm: &VirtualMachine, + ) -> PyResult<()> { + // TODO: This is _basically_ pyobject_to_opt_usize in itertools.rs + // need to move that function elsewhere and refactor usages. + let maxlen = if let Some(obj) = maxlen.into_option() { + if !vm.is_none(&obj) { + let maxlen: isize = obj + .payload::() + .ok_or_else(|| vm.new_type_error("an integer is required.".to_owned()))? + .try_to_primitive(vm)?; + + if maxlen.is_negative() { + return Err(vm.new_value_error("maxlen must be non-negative.".to_owned())); + } + Some(maxlen as usize) + } else { + None + } + } else { + None + }; + + // retrieve elements first to not to make too huge lock + let elements = iterable + .into_option() + .map(|iter| { + let mut elements: Vec = iter.try_to_value(vm)?; + if let Some(maxlen) = maxlen { + elements.drain(..elements.len().saturating_sub(maxlen)); + } + Ok(elements) + }) + .transpose()?; + + // SAFETY: This is hacky part for read-only field + // Because `maxlen` is only mutated from __init__. We can abuse the lock of deque to ensure this is locked enough. + // If we make a single lock of deque not only for extend but also for setting maxlen, it will be safe. + { + let mut deque = zelf.borrow_deque_mut(); + // Clear any previous data present. + deque.clear(); + unsafe { + // `maxlen` is better to be defined as UnsafeCell in common practice, + // but then more type works without any safety benefits + let unsafe_maxlen = + &zelf.maxlen as *const _ as *const std::cell::UnsafeCell>; + *(*unsafe_maxlen).get() = maxlen; + } + if let Some(elements) = elements { + deque.extend(elements); + } + } + + Ok(()) + } + } + impl AsSequence for PyDeque { fn as_sequence( _zelf: &crate::Py,