From 9f4f8ad0ec0fc1a782329a5ed0ea526fdd6a6da3 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 2 Jan 2020 23:28:28 +0900 Subject: [PATCH 01/11] PyTuple::elements into private --- src/main.rs | 4 +-- vm/src/exceptions.rs | 14 ++++----- vm/src/function.rs | 2 +- vm/src/obj/objbyteinner.rs | 6 ++-- vm/src/obj/objiter.rs | 2 +- vm/src/obj/objsequence.rs | 7 ----- vm/src/obj/objstr.rs | 12 ++++---- vm/src/obj/objtuple.rs | 60 +++++++++++++++++++------------------- vm/src/py_serde.rs | 11 +++---- vm/src/stdlib/socket.rs | 6 ++-- vm/src/vm.rs | 11 ++++--- 11 files changed, 65 insertions(+), 70 deletions(-) diff --git a/src/main.rs b/src/main.rs index dab0d2f9b..3833ee428 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,9 +52,9 @@ fn main() { if let Err(err) = res { if objtype::isinstance(&err, &vm.ctx.exceptions.system_exit) { let args = err.args(); - match args.elements.len() { + match args.as_slice().len() { 0 => return, - 1 => match_class!(match args.elements[0].clone() { + 1 => match_class!(match args.as_slice()[0].clone() { i @ PyInt => { use num_traits::cast::ToPrimitive; process::exit(i.as_bigint().to_i32().unwrap()); diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 5552eb154..5a7cc1d8d 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -275,20 +275,20 @@ fn exception_args_as_string( varargs: PyTupleRef, str_single: bool, ) -> Vec { - match varargs.elements.len() { + let varargs = varargs.as_slice(); + match varargs.len() { 0 => vec![], 1 => { let args0_repr = if str_single { - vm.to_str(&varargs.elements[0]) + vm.to_str(&varargs[0]) .unwrap_or_else(|_| PyString::from("").into_ref(vm)) } else { - vm.to_repr(&varargs.elements[0]) + vm.to_repr(&varargs[0]) .unwrap_or_else(|_| PyString::from("").into_ref(vm)) }; vec![args0_repr] } _ => varargs - .elements .iter() .map(|vararg| { vm.to_repr(vararg) @@ -362,8 +362,8 @@ impl ExceptionCtor { (Self::Class(cls), _) => { let args = match_class!(match value { PyNone => vec![], - tup @ PyTuple => tup.elements.clone(), - exc @ PyBaseException => exc.args().elements.clone(), + tup @ PyTuple => tup.as_slice().to_vec(), + exc @ PyBaseException => exc.args().as_slice().to_vec(), obj => vec![obj], }); invoke(cls, args, vm) @@ -609,7 +609,7 @@ fn make_arg_getter(idx: usize) -> impl Fn(PyBaseExceptionRef, &VirtualMachine) - move |exc, vm| { exc.args .borrow() - .elements + .as_slice() .get(idx) .cloned() .unwrap_or_else(|| vm.get_none()) diff --git a/vm/src/function.rs b/vm/src/function.rs index 41552de72..f2232ff93 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -583,7 +583,7 @@ pub fn single_or_tuple_any) -> PyResult>( match_class!(match obj { obj @ T => (self.predicate)(obj), tuple @ PyTuple => { - for obj in tuple.elements.iter() { + for obj in tuple.as_slice().iter() { if self.check(obj.clone())? { return Ok(true); } diff --git a/vm/src/obj/objbyteinner.rs b/vm/src/obj/objbyteinner.rs index ac6e5c841..666ed82d0 100644 --- a/vm/src/obj/objbyteinner.rs +++ b/vm/src/obj/objbyteinner.rs @@ -11,7 +11,7 @@ use super::objint::{self, PyInt, PyIntRef}; use super::objlist::PyList; use super::objmemory::PyMemoryView; use super::objnone::PyNoneRef; -use super::objsequence::{self, is_valid_slice_arg, PySliceableSequence}; +use super::objsequence::{is_valid_slice_arg, PySliceableSequence}; use super::objslice::PySliceRef; use super::objstr::{self, PyString, PyStringRef}; use super::objtuple::PyTupleRef; @@ -859,8 +859,8 @@ impl PyByteInner { Either::A(byte) => byte.elements, Either::B(tuple) => { let mut flatten = vec![]; - for v in objsequence::get_elements_tuple(tuple.as_object()).to_vec() { - flatten.extend(PyByteInner::try_from_object(vm, v)?.elements) + for v in tuple.as_slice() { + flatten.extend(PyByteInner::try_from_object(vm, v.clone())?.elements) } flatten } diff --git a/vm/src/obj/objiter.rs b/vm/src/obj/objiter.rs index b770e1e33..4fdd616e7 100644 --- a/vm/src/obj/objiter.rs +++ b/vm/src/obj/objiter.rs @@ -76,7 +76,7 @@ pub fn new_stop_iteration(vm: &VirtualMachine) -> PyBaseExceptionRef { pub fn stop_iter_value(vm: &VirtualMachine, exc: &PyBaseExceptionRef) -> PyResult { let args = exc.args(); let val = args - .elements + .as_slice() .first() .cloned() .unwrap_or_else(|| vm.get_none()); diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index 6dca84e25..fa63e52db 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -387,13 +387,6 @@ pub fn get_elements_list<'a>(obj: &'a PyObjectRef) -> impl Deref(obj: &'a PyObjectRef) -> impl Deref> + 'a { - if let Some(tuple) = obj.payload::() { - return &tuple.elements; - } - panic!("Cannot extract elements from non-sequence"); -} - pub fn get_mut_elements<'a>(obj: &'a PyObjectRef) -> impl DerefMut> + 'a { if let Some(list) = obj.payload::() { return list.elements.borrow_mut(); diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 063e18152..b48983b60 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -1472,8 +1472,10 @@ pub fn do_cformat_string( call_getitem(vm, &values, &vm.ctx.new_str(key.to_string()))? } None => { - let mut elements = - objtuple::get_value(&values).into_iter().skip(tuple_index); + let mut elements = objtuple::get_value(&values) + .to_vec() + .into_iter() + .skip(tuple_index); tuple_index = try_update_quantity_from_tuple( vm, @@ -1507,11 +1509,7 @@ pub fn do_cformat_string( } // check that all arguments were converted - if (!mapping_required - && objtuple::get_value(&values) - .into_iter() - .nth(tuple_index) - .is_some()) + if (!mapping_required && objtuple::get_value(&values).get(tuple_index).is_some()) && !objtype::isinstance(&values_obj, &vm.ctx.types.dict_type) { return Err( diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index d45955676..f34184c7b 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -2,10 +2,8 @@ use std::cell::Cell; use std::fmt; use super::objiter; -use super::objsequence::{ - get_elements_tuple, get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, -}; -use super::objtype::{self, PyClassRef}; +use super::objsequence::{get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul}; +use super::objtype::PyClassRef; use crate::function::OptionalArg; use crate::pyhash; use crate::pyobject::{ @@ -19,8 +17,7 @@ use crate::vm::{ReprGuard, VirtualMachine}; /// If the argument is a tuple, the return value is the same object. #[pyclass] pub struct PyTuple { - // TODO: shouldn't be public - pub elements: Vec, + elements: Vec, } impl fmt::Debug for PyTuple { @@ -64,21 +61,24 @@ impl PyTuple { pub fn fast_getitem(&self, idx: usize) -> PyObjectRef { self.elements[idx].clone() } + + pub fn as_slice(&self) -> &[PyObjectRef] { + &self.elements + } } pub type PyTupleRef = PyRef; -pub fn get_value(obj: &PyObjectRef) -> Vec { - obj.payload::().unwrap().elements.clone() +pub fn get_value(obj: &PyObjectRef) -> &[PyObjectRef] { + obj.payload::().unwrap().as_slice() } #[pyimpl] impl PyTuple { #[pymethod(name = "__lt__")] fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { - let other = get_elements_tuple(&other); - let res = seq_lt(vm, &self.elements.as_slice(), &other.as_slice())?; + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_lt(vm, &self.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -87,9 +87,8 @@ impl PyTuple { #[pymethod(name = "__gt__")] fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { - let other = get_elements_tuple(&other); - let res = seq_gt(vm, &self.elements.as_slice(), &other.as_slice())?; + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_gt(vm, &self.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -98,9 +97,8 @@ impl PyTuple { #[pymethod(name = "__ge__")] fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { - let other = get_elements_tuple(&other); - let res = seq_ge(vm, &self.elements.as_slice(), &other.as_slice())?; + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_ge(vm, &self.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -109,9 +107,8 @@ impl PyTuple { #[pymethod(name = "__le__")] fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { - let other = get_elements_tuple(&other); - let res = seq_le(vm, &self.elements.as_slice(), &other.as_slice())?; + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_le(vm, &self.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -120,9 +117,13 @@ impl PyTuple { #[pymethod(name = "__add__")] fn add(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { - let e2 = get_elements_tuple(&other); - let elements = self.elements.iter().chain(e2.iter()).cloned().collect(); + if let Some(other) = other.payload_if_subclass::(vm) { + let elements = self + .elements + .iter() + .chain(other.as_slice().iter()) + .cloned() + .collect(); Ok(vm.ctx.new_tuple(elements)) } else { Ok(vm.ctx.not_implemented()) @@ -147,7 +148,7 @@ impl PyTuple { #[pymethod(name = "__eq__")] fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { + if let Some(other) = other.payload_if_subclass::(vm) { Ok(vm.new_bool(self.inner_eq(&other, vm)?)) } else { Ok(vm.ctx.not_implemented()) @@ -156,16 +157,15 @@ impl PyTuple { #[pymethod(name = "__ne__")] fn ne(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.tuple_type()) { + if let Some(other) = other.payload_if_subclass::(vm) { Ok(vm.new_bool(!self.inner_eq(&other, vm)?)) } else { Ok(vm.ctx.not_implemented()) } } - fn inner_eq(&self, other: &PyObjectRef, vm: &VirtualMachine) -> PyResult { - let other = get_elements_tuple(other); - seq_equal(vm, &self.elements.as_slice(), &other.as_slice()) + fn inner_eq(&self, other: &PyTuple, vm: &VirtualMachine) -> PyResult { + seq_equal(vm, &self.as_slice(), &other.as_slice()) } #[pymethod(name = "__hash__")] @@ -277,8 +277,8 @@ impl PyValue for PyTupleIterator { impl PyTupleIterator { #[pymethod(name = "__next__")] fn next(&self, vm: &VirtualMachine) -> PyResult { - if self.position.get() < self.tuple.elements.len() { - let ret = self.tuple.elements[self.position.get()].clone(); + if self.position.get() < self.tuple.as_slice().len() { + let ret = self.tuple.as_slice()[self.position.get()].clone(); self.position.set(self.position.get() + 1); Ok(ret) } else { diff --git a/vm/src/py_serde.rs b/vm/src/py_serde.rs index 92200da5b..dbcba3060 100644 --- a/vm/src/py_serde.rs +++ b/vm/src/py_serde.rs @@ -4,7 +4,9 @@ use serde; use serde::de::{DeserializeSeed, Visitor}; use serde::ser::{Serialize, SerializeMap, SerializeSeq}; -use crate::obj::{objbool, objdict::PyDictRef, objfloat, objint, objsequence, objstr, objtype}; +use crate::obj::{ + objbool, objdict::PyDictRef, objfloat, objint, objsequence, objstr, objtuple::PyTuple, objtype, +}; use crate::pyobject::{IdProtocol, ItemProtocol, PyObjectRef, TypeProtocol}; use crate::VirtualMachine; use num_traits::cast::ToPrimitive; @@ -59,7 +61,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { S: serde::Serializer, { let serialize_seq_elements = - |serializer: S, elements: &Vec| -> Result { + |serializer: S, elements: &[PyObjectRef]| -> Result { let mut seq = serializer.serialize_seq(Some(elements.len()))?; for e in elements.iter() { seq.serialize_element(&self.clone_with_object(e))?; @@ -87,9 +89,8 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { } else if objtype::isinstance(self.pyobject, &self.vm.ctx.list_type()) { let elements = objsequence::get_elements_list(self.pyobject); serialize_seq_elements(serializer, &elements) - } else if objtype::isinstance(self.pyobject, &self.vm.ctx.tuple_type()) { - let elements = objsequence::get_elements_tuple(self.pyobject); - serialize_seq_elements(serializer, &elements) + } else if let Some(tuple) = self.pyobject.payload_if_subclass::(self.vm) { + serialize_seq_elements(serializer, tuple.as_slice()) } else if objtype::isinstance(self.pyobject, &self.vm.ctx.dict_type()) { let dict: PyDictRef = self.pyobject.clone().downcast().unwrap(); let pairs: Vec<_> = dict.into_iter().collect(); diff --git a/vm/src/stdlib/socket.rs b/vm/src/stdlib/socket.rs index c6cbfc8a8..b652509fd 100644 --- a/vm/src/stdlib/socket.rs +++ b/vm/src/stdlib/socket.rs @@ -389,16 +389,16 @@ impl ToSocketAddrs for Address { impl TryFromObject for Address { fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { let tuple = PyTupleRef::try_from_object(vm, obj)?; - if tuple.elements.len() != 2 { + if tuple.as_slice().len() != 2 { Err(vm.new_type_error("Address tuple should have only 2 values".to_string())) } else { - let host = PyStringRef::try_from_object(vm, tuple.elements[0].clone())?; + let host = PyStringRef::try_from_object(vm, tuple.as_slice()[0].clone())?; let host = if host.as_str().is_empty() { PyString::from("0.0.0.0").into_ref(vm) } else { host }; - let port = u16::try_from_object(vm, tuple.elements[1].clone())?; + let port = u16::try_from_object(vm, tuple.as_slice()[1].clone())?; Ok(Address { host, port }) } } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 488f168fb..8abdf94d8 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -36,7 +36,7 @@ use crate::obj::objiter; use crate::obj::objmodule::{self, PyModule}; use crate::obj::objsequence; use crate::obj::objstr::{PyString, PyStringRef}; -use crate::obj::objtuple::PyTupleRef; +use crate::obj::objtuple::{PyTuple, PyTupleRef}; use crate::obj::objtype::{self, PyClassRef}; use crate::pyhash; use crate::pyobject::{ @@ -850,7 +850,7 @@ impl VirtualMachine { // Add missing positional arguments, if we have fewer positional arguments than the // function definition calls for if nargs < nexpected_args { - let num_defaults_available = defaults.as_ref().map_or(0, |d| d.elements.len()); + let num_defaults_available = defaults.as_ref().map_or(0, |d| d.as_slice().len()); // Given the number of defaults available, check all the arguments for which we // _don't_ have defaults; if any are missing, raise an exception @@ -870,7 +870,7 @@ impl VirtualMachine { ))); } if let Some(defaults) = defaults { - let defaults = &defaults.elements; + let defaults = defaults.as_slice(); // We have sufficient defaults, so iterate over the corresponding names and use // the default if we don't already have a value for (default_index, i) in (required_args..nexpected_args).enumerate() { @@ -905,7 +905,10 @@ impl VirtualMachine { // Extract elements from item, if possible: let cls = value.class(); if cls.is(&self.ctx.tuple_type()) { - objsequence::get_elements_tuple(value) + value + .payload::() + .unwrap() + .as_slice() .iter() .map(|obj| T::try_from_object(self, obj.clone())) .collect() From 6f313c191ea559d0c349b64de0b2f5ae0f4489f8 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 2 Jan 2020 23:52:34 +0900 Subject: [PATCH 02/11] Refactor tuple comparison operators --- vm/src/obj/objtuple.rs | 59 ++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 39 deletions(-) diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index f34184c7b..73ff95cb8 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -75,44 +75,37 @@ pub fn get_value(obj: &PyObjectRef) -> &[PyObjectRef] { #[pyimpl] impl PyTuple { + #[inline] + fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult + where + F: Fn(&[PyObjectRef], &[PyObjectRef]) -> PyResult, + { + let r = if let Some(other) = other.payload_if_subclass::(vm) { + vm.new_bool(op(self.as_slice(), other.as_slice())?) + } else { + vm.ctx.not_implemented() + }; + Ok(r) + } + #[pymethod(name = "__lt__")] fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_lt(vm, &self.as_slice(), &other.as_slice())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + self.cmp(other, |a, b| seq_lt(vm, &a, &b), vm) } #[pymethod(name = "__gt__")] fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_gt(vm, &self.as_slice(), &other.as_slice())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + self.cmp(other, |a, b| seq_gt(vm, &a, &b), vm) } #[pymethod(name = "__ge__")] fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_ge(vm, &self.as_slice(), &other.as_slice())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + self.cmp(other, |a, b| seq_ge(vm, &a, &b), vm) } #[pymethod(name = "__le__")] fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_le(vm, &self.as_slice(), &other.as_slice())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + self.cmp(other, |a, b| seq_le(vm, &a, &b), vm) } #[pymethod(name = "__add__")] @@ -148,24 +141,12 @@ impl PyTuple { #[pymethod(name = "__eq__")] fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - Ok(vm.new_bool(self.inner_eq(&other, vm)?)) - } else { - Ok(vm.ctx.not_implemented()) - } + self.cmp(other, |a, b| seq_equal(vm, &a, &b), vm) } #[pymethod(name = "__ne__")] fn ne(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - Ok(vm.new_bool(!self.inner_eq(&other, vm)?)) - } else { - Ok(vm.ctx.not_implemented()) - } - } - - fn inner_eq(&self, other: &PyTuple, vm: &VirtualMachine) -> PyResult { - seq_equal(vm, &self.as_slice(), &other.as_slice()) + self.cmp(other, |a, b| Ok(!seq_equal(vm, &a, &b)?), vm) } #[pymethod(name = "__hash__")] @@ -208,7 +189,7 @@ impl PyTuple { #[pymethod(name = "__mul__")] fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements.as_slice(), counter) + let new_elements = seq_mul(&self.as_slice(), counter) .cloned() .collect(); vm.ctx.new_tuple(new_elements) From 50ff5e89f9c31da6c82baadbc054eabfa111397d Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 2 Jan 2020 23:52:54 +0900 Subject: [PATCH 03/11] sequence comparison uses `impl SimpleSeq` rather than `dyn SimpleSeq` --- vm/src/obj/objsequence.rs | 30 +++++++++++++++++++++++------- vm/src/obj/objtuple.rs | 4 +--- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index fa63e52db..44100cb55 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -277,8 +277,8 @@ impl SimpleSeq for std::collections::VecDeque { pub fn seq_equal( vm: &VirtualMachine, - zelf: &dyn SimpleSeq, - other: &dyn SimpleSeq, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, ) -> PyResult { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -295,7 +295,11 @@ pub fn seq_equal( } } -pub fn seq_lt(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) -> PyResult { +pub fn seq_lt( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { return Ok(v); @@ -304,7 +308,11 @@ pub fn seq_lt(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) Ok(zelf.len() < other.len()) } -pub fn seq_gt(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) -> PyResult { +pub fn seq_gt( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { return Ok(v); @@ -313,7 +321,11 @@ pub fn seq_gt(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) Ok(zelf.len() > other.len()) } -pub fn seq_ge(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) -> PyResult { +pub fn seq_ge( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { return Ok(v); @@ -323,7 +335,11 @@ pub fn seq_ge(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) Ok(zelf.len() >= other.len()) } -pub fn seq_le(vm: &VirtualMachine, zelf: &dyn SimpleSeq, other: &dyn SimpleSeq) -> PyResult { +pub fn seq_le( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { return Ok(v); @@ -365,7 +381,7 @@ impl<'a> Iterator for SeqMul<'a> { } impl ExactSizeIterator for SeqMul<'_> {} -pub fn seq_mul(seq: &dyn SimpleSeq, repetitions: isize) -> SeqMul { +pub fn seq_mul(seq: &impl SimpleSeq, repetitions: isize) -> SeqMul { SeqMul { seq, repetitions: repetitions.max(0) as usize, diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 73ff95cb8..00a011504 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -189,9 +189,7 @@ impl PyTuple { #[pymethod(name = "__mul__")] fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.as_slice(), counter) - .cloned() - .collect(); + let new_elements = seq_mul(&self.as_slice(), counter).cloned().collect(); vm.ctx.new_tuple(new_elements) } From 0aca07a8cb5a992384579a290a0da02bf8994e4a Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 6 Jan 2020 02:26:17 +0900 Subject: [PATCH 04/11] rustpython_vm::sequence & PySequenceContainer --- vm/src/lib.rs | 1 + vm/src/obj/objtuple.rs | 9 ++++++++- vm/src/sequence.rs | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 vm/src/sequence.rs diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 1f8fe9be4..c9b8a7686 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -64,6 +64,7 @@ pub mod py_serde; mod pyhash; pub mod pyobject; pub mod scope; +mod sequence; pub mod stdlib; mod sysmodule; pub mod types; diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 00a011504..7d6957491 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -9,6 +9,7 @@ use crate::pyhash; use crate::pyobject::{ IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, }; +use crate::sequence::PySequenceContainer; use crate::vm::{ReprGuard, VirtualMachine}; /// tuple() -> empty tuple @@ -57,13 +58,19 @@ impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4)); impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4), (F, 5)); impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4), (F, 5), (G, 6)); +impl PySequenceContainer for PyTuple { + fn as_slice(&self) -> &[PyObjectRef] { + &self.elements + } +} + impl PyTuple { pub fn fast_getitem(&self, idx: usize) -> PyObjectRef { self.elements[idx].clone() } pub fn as_slice(&self) -> &[PyObjectRef] { - &self.elements + ::as_slice(self) } } diff --git a/vm/src/sequence.rs b/vm/src/sequence.rs new file mode 100644 index 000000000..f6e494c7d --- /dev/null +++ b/vm/src/sequence.rs @@ -0,0 +1,22 @@ +use crate::pyobject::{PyObjectRef, PyResult, PyValue}; +use crate::vm::VirtualMachine; + +pub trait PySequenceContainer +where + Self: PyValue, +{ + fn as_slice(&self) -> &[PyObjectRef]; + + #[inline] + fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult + where + F: Fn(&[PyObjectRef], &[PyObjectRef]) -> PyResult, + { + let r = if let Some(other) = other.payload_if_subclass::(vm) { + vm.new_bool(op(self.as_slice(), other.as_slice())?) + } else { + vm.ctx.not_implemented() + }; + Ok(r) + } +} From 0e583b28294b8fb3afa7fab7ad9bf1fb32ddd2f6 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 7 Jan 2020 20:42:25 +0900 Subject: [PATCH 05/11] Refactor PyList to have elements as private field --- vm/src/obj/objlist.rs | 344 +++++++++++++++++++----------------- vm/src/obj/objsequence.rs | 23 +-- vm/src/obj/objtuple.rs | 1 - vm/src/py_serde.rs | 8 +- vm/src/stdlib/subprocess.rs | 4 +- vm/src/vm.rs | 7 +- 6 files changed, 193 insertions(+), 194 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 5e88d55cb..e85a0e984 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -11,21 +11,26 @@ use super::objbyteinner; use super::objint::PyIntRef; use super::objiter; use super::objsequence::{ - get_elements_list, get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SequenceIndex, + get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SequenceIndex, }; use super::objslice::PySliceRef; -use super::objtype::{self, PyClassRef}; +use super::objtype::PyClassRef; use crate::function::OptionalArg; use crate::pyobject::{ - IdProtocol, PyClassImpl, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, - TryFromObject, TypeProtocol, + IdProtocol, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyContext, PyIterable, + PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; +use crate::sequence::PySequenceContainer; use crate::vm::{ReprGuard, VirtualMachine}; +/// Built-in mutable sequence. +/// +/// If no argument is given, the constructor creates a new empty list. +/// The argument must be an iterable if specified. +#[pyclass] #[derive(Default)] pub struct PyList { - // TODO: shouldn't be public - pub elements: RefCell>, + elements: RefCell>, } impl fmt::Debug for PyList { @@ -49,6 +54,19 @@ impl PyValue for PyList { } } +// impl PySequenceContainer for PyList { +// fn as_slice<'a>(&'a self) -> &'a [PyObjectRef] { +// let b = self.elements.borrow(); +// &b +// } +// } + +impl PyList { + pub fn elements<'a>(&'a self) -> impl std::ops::Deref> + 'a { + self.elements.borrow() + } +} + impl PyList { pub fn get_len(&self) -> usize { self.elements.borrow().len() @@ -130,18 +148,22 @@ struct SortOptions { pub type PyListRef = PyRef; -impl PyListRef { - pub fn append(self, x: PyObjectRef, _vm: &VirtualMachine) { +#[pyimpl] +impl PyList { + #[pymethod] + pub fn append(&self, x: PyObjectRef, _vm: &VirtualMachine) { self.elements.borrow_mut().push(x); } - fn extend(self, x: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + #[pymethod] + fn extend(&self, x: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut new_elements = vm.extract_elements(&x)?; self.elements.borrow_mut().append(&mut new_elements); Ok(()) } - fn insert(self, position: isize, element: PyObjectRef, _vm: &VirtualMachine) { + #[pymethod] + fn insert(&self, position: isize, element: PyObjectRef, _vm: &VirtualMachine) { let mut vec = self.elements.borrow_mut(); let vec_len = vec.len().to_isize().unwrap(); // This unbounded position can be < 0 or > vec.len() @@ -155,76 +177,88 @@ impl PyListRef { vec.insert(position, element.clone()); } - fn add(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.list_type()) { - let e1 = self.elements.borrow(); - let e2 = get_elements_list(&other); + #[pymethod(name = "__add__")] + fn add(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if let Some(other) = other.payload_if_subclass::(vm) { + let e1 = self.elements(); + let e2 = other.elements(); let elements = e1.iter().chain(e2.iter()).cloned().collect(); Ok(vm.ctx.new_list(elements)) } else { - Err(vm.new_type_error(format!("Cannot add {} and {}", self.as_object(), other))) + Err(vm.new_type_error(format!( + "Cannot add {} and {}", + Self::class(vm).name, + other.class().name + ))) } } - fn iadd(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + #[pymethod(name = "__iadd__")] + fn iadd(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Ok(new_elements) = vm.extract_elements(&other) { let mut e = new_elements; - self.elements.borrow_mut().append(&mut e); - Ok(self.into_object()) + zelf.elements.borrow_mut().append(&mut e); + Ok(zelf.into_object()) } else { Ok(vm.ctx.not_implemented()) } } - fn bool(self, _vm: &VirtualMachine) -> bool { + #[pymethod(name = "__bool__")] + fn bool(&self, _vm: &VirtualMachine) -> bool { !self.elements.borrow().is_empty() } - fn clear(self, _vm: &VirtualMachine) { + #[pymethod] + fn clear(&self, _vm: &VirtualMachine) { self.elements.borrow_mut().clear(); } - fn copy(self, vm: &VirtualMachine) -> PyObjectRef { + #[pymethod] + fn copy(&self, vm: &VirtualMachine) -> PyObjectRef { vm.ctx.new_list(self.elements.borrow().clone()) } - fn len(self, _vm: &VirtualMachine) -> usize { + #[pymethod(name = "__len__")] + fn len(&self, _vm: &VirtualMachine) -> usize { self.elements.borrow().len() } - fn sizeof(self, _vm: &VirtualMachine) -> usize { + #[pymethod(name = "__sizeof__")] + fn sizeof(&self, _vm: &VirtualMachine) -> usize { size_of::() + self.elements.borrow().capacity() * size_of::() } - fn reverse(self, _vm: &VirtualMachine) { + #[pymethod] + fn reverse(&self, _vm: &VirtualMachine) { self.elements.borrow_mut().reverse(); } - fn reversed(self, _vm: &VirtualMachine) -> PyListReverseIterator { - let final_position = self.elements.borrow().len(); + + #[pymethod(name = "__reversed__")] + fn reversed(zelf: PyRef, _vm: &VirtualMachine) -> PyListReverseIterator { + let final_position = zelf.elements.borrow().len(); PyListReverseIterator { position: Cell::new(final_position), - list: self, + list: zelf, } } - fn getitem(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - get_item( - vm, - self.as_object(), - &self.elements.borrow(), - needle.clone(), - ) + #[pymethod(name = "__getitem__")] + fn getitem(zelf: PyRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + get_item(vm, zelf.as_object(), &zelf.elements(), needle.clone()) } - fn iter(self, _vm: &VirtualMachine) -> PyListIterator { + #[pymethod(name = "__iter__")] + fn iter(zelf: PyRef, _vm: &VirtualMachine) -> PyListIterator { PyListIterator { position: Cell::new(0), - list: self, + list: zelf, } } + #[pymethod(name = "__setitem__")] fn setitem( - self, + &self, subscript: SequenceIndex, value: PyObjectRef, vm: &VirtualMachine, @@ -240,7 +274,7 @@ impl PyListRef { } } - fn setindex(self, index: i32, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn setindex(&self, index: i32, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(pos_index) = self.get_pos(index) { self.elements.borrow_mut()[pos_index] = value; Ok(vm.get_none()) @@ -249,7 +283,7 @@ impl PyListRef { } } - fn setslice(self, slice: PySliceRef, sec: PyIterable, vm: &VirtualMachine) -> PyResult { + fn setslice(&self, slice: PySliceRef, sec: PyIterable, vm: &VirtualMachine) -> PyResult { let step = slice.step_index(vm)?.unwrap_or_else(BigInt::one); if step.is_zero() { @@ -303,7 +337,7 @@ impl PyListRef { } } - fn _set_slice(self, range: Range, sec: PyIterable, vm: &VirtualMachine) -> PyResult { + fn _set_slice(&self, range: Range, sec: PyIterable, vm: &VirtualMachine) -> PyResult { // consume the iter, we need it's size // and if it's going to fail we want that to happen *before* we start modifing let items: Result, _> = sec.iter(vm)?.collect(); @@ -316,7 +350,7 @@ impl PyListRef { } fn _set_stepped_slice( - self, + &self, range: Range, step: usize, sec: PyIterable, @@ -358,7 +392,7 @@ impl PyListRef { } fn _set_stepped_slice_reverse( - self, + &self, range: Range, step: usize, sec: PyIterable, @@ -400,7 +434,7 @@ impl PyListRef { } } - fn _replace_indexes(self, indexes: I, items: &[PyObjectRef]) + fn _replace_indexes(&self, indexes: I, items: &[PyObjectRef]) where I: Iterator, { @@ -412,10 +446,11 @@ impl PyListRef { } } - fn repr(self, vm: &VirtualMachine) -> PyResult { - let s = if let Some(_guard) = ReprGuard::enter(self.as_object()) { - let mut str_parts = Vec::with_capacity(self.elements.borrow().len()); - for elem in self.elements.borrow().iter() { + #[pymethod(name = "__repr__")] + fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { + let s = if let Some(_guard) = ReprGuard::enter(zelf.as_object()) { + let mut str_parts = Vec::with_capacity(zelf.elements.borrow().len()); + for elem in zelf.elements.borrow().iter() { let s = vm.to_repr(elem)?; str_parts.push(s.as_str().to_string()); } @@ -426,30 +461,35 @@ impl PyListRef { Ok(s) } - fn hash(self, vm: &VirtualMachine) -> PyResult<()> { + #[pymethod(name = "__hash__")] + fn hash(&self, vm: &VirtualMachine) -> PyResult<()> { Err(vm.new_type_error("unhashable type".to_string())) } - fn mul(self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements.borrow().as_slice(), counter) + #[pymethod(name = "__mul__")] + fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { + let new_elements = seq_mul(&self.elements().as_slice(), counter) .cloned() .collect(); vm.ctx.new_list(new_elements) } - fn rmul(self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { + #[pymethod(name = "__rmul__")] + fn rmul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { self.mul(counter, &vm) } - fn imul(self, counter: isize, _vm: &VirtualMachine) -> Self { - let new_elements = seq_mul(&self.elements.borrow().as_slice(), counter) + #[pymethod(name = "__imul__")] + fn imul(zelf: PyRef, counter: isize, _vm: &VirtualMachine) -> PyRef { + let new_elements = seq_mul(&zelf.elements().as_slice(), counter) .cloned() .collect(); - self.elements.replace(new_elements); - self + zelf.elements.replace(new_elements); + zelf } - fn count(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + #[pymethod] + fn count(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { let mut count: usize = 0; for element in self.elements.borrow().iter() { if vm.identical_or_equal(element, &needle)? { @@ -459,7 +499,8 @@ impl PyListRef { Ok(count) } - fn contains(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + #[pymethod(name = "__contains__")] + fn contains(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { for element in self.elements.borrow().iter() { if vm.identical_or_equal(element, &needle)? { return Ok(true); @@ -469,7 +510,8 @@ impl PyListRef { Ok(false) } - fn index(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + #[pymethod] + fn index(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { for (index, element) in self.elements.borrow().iter().enumerate() { if vm.identical_or_equal(element, &needle)? { return Ok(index); @@ -479,7 +521,8 @@ impl PyListRef { Err(vm.new_value_error(format!("'{}' is not in list", needle_str.as_str()))) } - fn pop(self, i: OptionalArg, vm: &VirtualMachine) -> PyResult { + #[pymethod] + fn pop(&self, i: OptionalArg, vm: &VirtualMachine) -> PyResult { let mut i = i.into_option().unwrap_or(-1); let mut elements = self.elements.borrow_mut(); if i < 0 { @@ -494,7 +537,8 @@ impl PyListRef { } } - fn remove(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + #[pymethod] + fn remove(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut ri: Option = None; for (index, element) in self.elements.borrow().iter().enumerate() { if vm.identical_or_equal(element, &needle)? { @@ -512,86 +556,100 @@ impl PyListRef { } } - fn eq(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - let value = if self.as_object().is(&other) { - vm.new_bool(true) - } else if objtype::isinstance(&other, &vm.ctx.list_type()) { - vm.new_bool(self.inner_eq(&other, vm)?) + #[pymethod(name = "__eq__")] + fn eq( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + let value = if zelf.as_object().is(&other) { + Implemented(true) + } else if let Some(other) = other.payload_if_subclass::(vm) { + Implemented(seq_equal( + vm, + &zelf.elements().as_slice(), + &other.elements().as_slice(), + )?) } else { - vm.ctx.not_implemented() + NotImplemented }; Ok(value) } - fn ne(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - let value = if self.as_object().is(&other) { - vm.new_bool(false) - } else if objtype::isinstance(&other, &vm.ctx.list_type()) { - vm.new_bool(!self.inner_eq(&other, vm)?) - } else { - vm.ctx.not_implemented() - }; - Ok(value) + #[pymethod(name = "__ne__")] + fn ne( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + Ok(PyList::eq(zelf, other, vm)?.map(|v| !v)) } - fn inner_eq(self, other: &PyObjectRef, vm: &VirtualMachine) -> PyResult { - let zelf = self.elements.borrow(); - let other = get_elements_list(other); - seq_equal(vm, &zelf.as_slice(), &other.as_slice()) - } - - fn lt(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.list_type()) { - let zelf = self.elements.borrow(); - let other = get_elements_list(&other); - let res = seq_lt(vm, &zelf.as_slice(), &other.as_slice())?; + #[pymethod(name = "__lt__")] + fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_lt( + vm, + &self.elements().as_slice(), + &other.elements().as_slice(), + )?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) } } - fn gt(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.list_type()) { - let zelf = self.elements.borrow(); - let other = get_elements_list(&other); - let res = seq_gt(vm, &zelf.as_slice(), &other.as_slice())?; + #[pymethod(name = "__gt__")] + fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_gt( + vm, + &self.elements().as_slice(), + &other.elements().as_slice(), + )?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) } } - fn ge(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.list_type()) { - let zelf = self.elements.borrow(); - let other = get_elements_list(&other); - let res = seq_ge(vm, &zelf.as_slice(), &other.as_slice())?; + #[pymethod(name = "__ge__")] + fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_ge( + vm, + &self.elements().as_slice(), + &other.elements().as_slice(), + )?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) } } - fn le(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&other, &vm.ctx.list_type()) { - let zelf = self.elements.borrow(); - let other = get_elements_list(&other); - let res = seq_le(vm, &zelf.as_slice(), &other.as_slice())?; + #[pymethod(name = "__le__")] + fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if let Some(other) = other.payload_if_subclass::(vm) { + let res = seq_le( + vm, + &self.elements().as_slice(), + &other.elements().as_slice(), + )?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) } } - fn delitem(self, subscript: SequenceIndex, vm: &VirtualMachine) -> PyResult<()> { + #[pymethod(name = "__delitem__")] + fn delitem(&self, subscript: SequenceIndex, vm: &VirtualMachine) -> PyResult<()> { match subscript { SequenceIndex::Int(index) => self.delindex(index, vm), SequenceIndex::Slice(slice) => self.delslice(slice, vm), } } - fn delindex(self, index: i32, vm: &VirtualMachine) -> PyResult<()> { + fn delindex(&self, index: i32, vm: &VirtualMachine) -> PyResult<()> { if let Some(pos_index) = self.get_pos(index) { self.elements.borrow_mut().remove(pos_index); Ok(()) @@ -600,7 +658,7 @@ impl PyListRef { } } - fn delslice(self, slice: PySliceRef, vm: &VirtualMachine) -> PyResult<()> { + fn delslice(&self, slice: PySliceRef, vm: &VirtualMachine) -> PyResult<()> { let start = slice.start_index(vm)?; let stop = slice.stop_index(vm)?; let step = slice.step_index(vm)?.unwrap_or_else(BigInt::one); @@ -669,11 +727,11 @@ impl PyListRef { } } - fn _del_slice(self, range: Range) { + fn _del_slice(&self, range: Range) { self.elements.borrow_mut().drain(range); } - fn _del_stepped_slice(self, range: Range, step: usize) { + fn _del_stepped_slice(&self, range: Range, step: usize) { // no easy way to delete stepped indexes so here is what we'll do let mut deleted = 0; let mut elements = self.elements.borrow_mut(); @@ -694,7 +752,7 @@ impl PyListRef { elements.drain((range.end - deleted)..range.end); } - fn _del_stepped_slice_reverse(self, range: Range, step: usize) { + fn _del_stepped_slice_reverse(&self, range: Range, step: usize) { // no easy way to delete stepped indexes so here is what we'll do let mut deleted = 0; let mut elements = self.elements.borrow_mut(); @@ -715,7 +773,8 @@ impl PyListRef { elements.drain(range.start..(range.start + deleted)); } - fn sort(self, options: SortOptions, vm: &VirtualMachine) -> PyResult<()> { + #[pymethod] + fn sort(&self, options: SortOptions, vm: &VirtualMachine) -> PyResult<()> { // replace list contents with [] for duration of sort. // this prevents keyfunc from messing with the list and makes it easy to // check if it tries to append elements to it. @@ -729,20 +788,21 @@ impl PyListRef { Ok(()) } -} -fn list_new( - cls: PyClassRef, - iterable: OptionalArg, - vm: &VirtualMachine, -) -> PyResult { - let elements = if let OptionalArg::Present(iterable) = iterable { - vm.extract_elements(&iterable)? - } else { - vec![] - }; + #[pyslot(new)] + fn list_new( + cls: PyClassRef, + iterable: OptionalArg, + vm: &VirtualMachine, + ) -> PyResult { + let elements = if let OptionalArg::Present(iterable) = iterable { + vm.extract_elements(&iterable)? + } else { + vec![] + }; - PyList::from(elements).into_ref_with_type(vm, cls) + PyList::from(elements).into_ref_with_type(vm, cls) + } } fn quicksort( @@ -875,51 +935,9 @@ impl PyListReverseIterator { } } -#[rustfmt::skip] // to avoid line splitting pub fn init(context: &PyContext) { let list_type = &context.types.list_type; - - let list_doc = "Built-in mutable sequence.\n\n\ - If no argument is given, the constructor creates a new empty list.\n\ - The argument must be an iterable if specified."; - - extend_class!(context, list_type, { - "__sizeof__" => context.new_rustfunc(PyListRef::sizeof), - "__add__" => context.new_rustfunc(PyListRef::add), - "__iadd__" => context.new_rustfunc(PyListRef::iadd), - "__bool__" => context.new_rustfunc(PyListRef::bool), - "__contains__" => context.new_rustfunc(PyListRef::contains), - "__delitem__" => context.new_rustfunc(PyListRef::delitem), - "__eq__" => context.new_rustfunc(PyListRef::eq), - "__ne__" => context.new_rustfunc(PyListRef::ne), - "__lt__" => context.new_rustfunc(PyListRef::lt), - "__gt__" => context.new_rustfunc(PyListRef::gt), - "__le__" => context.new_rustfunc(PyListRef::le), - "__ge__" => context.new_rustfunc(PyListRef::ge), - "__getitem__" => context.new_rustfunc(PyListRef::getitem), - "__iter__" => context.new_rustfunc(PyListRef::iter), - "__setitem__" => context.new_rustfunc(PyListRef::setitem), - "__reversed__" => context.new_rustfunc(PyListRef::reversed), - "__mul__" => context.new_rustfunc(PyListRef::mul), - "__rmul__" => context.new_rustfunc(PyListRef::rmul), - "__imul__" => context.new_rustfunc(PyListRef::imul), - "__len__" => context.new_rustfunc(PyListRef::len), - (slot new) => list_new, - "__repr__" => context.new_rustfunc(PyListRef::repr), - "__hash__" => context.new_rustfunc(PyListRef::hash), - "__doc__" => context.new_str(list_doc.to_string()), - "append" => context.new_rustfunc(PyListRef::append), - "clear" => context.new_rustfunc(PyListRef::clear), - "copy" => context.new_rustfunc(PyListRef::copy), - "count" => context.new_rustfunc(PyListRef::count), - "extend" => context.new_rustfunc(PyListRef::extend), - "index" => context.new_rustfunc(PyListRef::index), - "insert" => context.new_rustfunc(PyListRef::insert), - "reverse" => context.new_rustfunc(PyListRef::reverse), - "sort" => context.new_rustfunc(PyListRef::sort), - "pop" => context.new_rustfunc(PyListRef::pop), - "remove" => context.new_rustfunc(PyListRef::remove) - }); + PyList::extend_class(context, list_type); PyListIterator::extend_class(context, &context.types.listiterator_type); PyListReverseIterator::extend_class(context, &context.types.listreverseiterator_type); diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index 44100cb55..1f67106ec 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -1,6 +1,6 @@ use std::cell::RefCell; use std::marker::Sized; -use std::ops::{Deref, DerefMut, Range}; +use std::ops::Range; use num_bigint::{BigInt, ToBigInt}; use num_traits::{One, Signed, ToPrimitive, Zero}; @@ -389,27 +389,6 @@ pub fn seq_mul(seq: &impl SimpleSeq, repetitions: isize) -> SeqMul { } } -pub fn get_elements_cell<'a>(obj: &'a PyObjectRef) -> &'a RefCell> { - if let Some(list) = obj.payload::() { - return &list.elements; - } - panic!("Cannot extract elements from non-sequence"); -} - -pub fn get_elements_list<'a>(obj: &'a PyObjectRef) -> impl Deref> + 'a { - if let Some(list) = obj.payload::() { - return list.elements.borrow(); - } - panic!("Cannot extract elements from non-sequence"); -} - -pub fn get_mut_elements<'a>(obj: &'a PyObjectRef) -> impl DerefMut> + 'a { - if let Some(list) = obj.payload::() { - return list.elements.borrow_mut(); - } - panic!("Cannot extract elements from non-sequence"); -} - //Check if given arg could be used with PySliceableSequence.get_slice_range() pub fn is_valid_slice_arg( arg: OptionalArg, diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 7d6957491..90f969723 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -278,7 +278,6 @@ impl PyTupleIterator { } } -#[rustfmt::skip] // to avoid line splitting pub fn init(context: &PyContext) { let tuple_type = &context.types.tuple_type; PyTuple::extend_class(context, tuple_type); diff --git a/vm/src/py_serde.rs b/vm/src/py_serde.rs index dbcba3060..3c8b05a8c 100644 --- a/vm/src/py_serde.rs +++ b/vm/src/py_serde.rs @@ -5,7 +5,8 @@ use serde::de::{DeserializeSeed, Visitor}; use serde::ser::{Serialize, SerializeMap, SerializeSeq}; use crate::obj::{ - objbool, objdict::PyDictRef, objfloat, objint, objsequence, objstr, objtuple::PyTuple, objtype, + objbool, objdict::PyDictRef, objfloat, objint, objlist::PyList, objstr, objtuple::PyTuple, + objtype, }; use crate::pyobject::{IdProtocol, ItemProtocol, PyObjectRef, TypeProtocol}; use crate::VirtualMachine; @@ -86,9 +87,8 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { } else { serializer.serialize_i64(v.to_i64().ok_or_else(int_too_large)?) } - } else if objtype::isinstance(self.pyobject, &self.vm.ctx.list_type()) { - let elements = objsequence::get_elements_list(self.pyobject); - serialize_seq_elements(serializer, &elements) + } else if let Some(list) = self.pyobject.payload_if_subclass::(self.vm) { + serialize_seq_elements(serializer, &list.elements()) } else if let Some(tuple) = self.pyobject.payload_if_subclass::(self.vm) { serialize_seq_elements(serializer, tuple.as_slice()) } else if objtype::isinstance(self.pyobject, &self.vm.ctx.dict_type()) { diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index bf6738694..2f0760eae 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -8,7 +8,6 @@ use subprocess; use crate::function::OptionalArg; use crate::obj::objbytes::PyBytesRef; use crate::obj::objlist::PyListRef; -use crate::obj::objsequence; use crate::obj::objstr::{self, PyStringRef}; use crate::obj::objtype::PyClassRef; use crate::pyobject::{Either, IntoPyObject, PyObjectRef, PyRef, PyResult, PyValue}; @@ -107,7 +106,8 @@ impl PopenRef { let stderr = convert_redirection(args.stderr, vm)?; let command_list = match &args.args { Either::A(command) => vec![command.as_str().to_string()], - Either::B(command_list) => objsequence::get_elements_list(command_list.as_object()) + Either::B(command_list) => command_list + .elements() .iter() .map(|x| objstr::get_value(x)) .collect(), diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 8abdf94d8..fd2fdf85d 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -33,8 +33,8 @@ use crate::obj::objfunction::{PyFunction, PyMethod}; use crate::obj::objgenerator::PyGenerator; use crate::obj::objint::PyInt; use crate::obj::objiter; +use crate::obj::objlist::PyList; use crate::obj::objmodule::{self, PyModule}; -use crate::obj::objsequence; use crate::obj::objstr::{PyString, PyStringRef}; use crate::obj::objtuple::{PyTuple, PyTupleRef}; use crate::obj::objtype::{self, PyClassRef}; @@ -913,7 +913,10 @@ impl VirtualMachine { .map(|obj| T::try_from_object(self, obj.clone())) .collect() } else if cls.is(&self.ctx.list_type()) { - objsequence::get_elements_list(value) + value + .payload::() + .unwrap() + .elements() .iter() .map(|obj| T::try_from_object(self, obj.clone())) .collect() From 12725a308eab610f05c0ed32eaefa132f30d2e94 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 7 Jan 2020 21:37:52 +0900 Subject: [PATCH 06/11] PyList::borrow_sequence to borrow SimpleSeq --- vm/src/obj/objlist.rs | 63 +++++++++++++------------------------ vm/src/obj/objsequence.rs | 27 +++++++++++++--- vm/src/obj/objtuple.rs | 52 +++++++++++++++--------------- vm/src/py_serde.rs | 2 +- vm/src/sequence.rs | 20 ------------ vm/src/stdlib/subprocess.rs | 2 +- vm/src/vm.rs | 2 +- 7 files changed, 72 insertions(+), 96 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index e85a0e984..b874103b3 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -11,7 +11,7 @@ use super::objbyteinner; use super::objint::PyIntRef; use super::objiter; use super::objsequence::{ - get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SequenceIndex, + get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SequenceIndex, SimpleSeq, }; use super::objslice::PySliceRef; use super::objtype::PyClassRef; @@ -20,7 +20,6 @@ use crate::pyobject::{ IdProtocol, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; -use crate::sequence::PySequenceContainer; use crate::vm::{ReprGuard, VirtualMachine}; /// Built-in mutable sequence. @@ -54,15 +53,12 @@ impl PyValue for PyList { } } -// impl PySequenceContainer for PyList { -// fn as_slice<'a>(&'a self) -> &'a [PyObjectRef] { -// let b = self.elements.borrow(); -// &b -// } -// } - impl PyList { - pub fn elements<'a>(&'a self) -> impl std::ops::Deref> + 'a { + pub fn borrow_sequence<'a>(&'a self) -> impl SimpleSeq + 'a { + self.elements.borrow() + } + + pub fn borrow_elements<'a>(&'a self) -> impl std::ops::Deref> + 'a { self.elements.borrow() } } @@ -180,8 +176,8 @@ impl PyList { #[pymethod(name = "__add__")] fn add(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let e1 = self.elements(); - let e2 = other.elements(); + let e1 = self.borrow_sequence(); + let e2 = other.borrow_sequence(); let elements = e1.iter().chain(e2.iter()).cloned().collect(); Ok(vm.ctx.new_list(elements)) } else { @@ -245,7 +241,12 @@ impl PyList { #[pymethod(name = "__getitem__")] fn getitem(zelf: PyRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - get_item(vm, zelf.as_object(), &zelf.elements(), needle.clone()) + get_item( + vm, + zelf.as_object(), + &zelf.elements.borrow(), + needle.clone(), + ) } #[pymethod(name = "__iter__")] @@ -468,9 +469,7 @@ impl PyList { #[pymethod(name = "__mul__")] fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements().as_slice(), counter) - .cloned() - .collect(); + let new_elements = seq_mul(&self.borrow_sequence(), counter).cloned().collect(); vm.ctx.new_list(new_elements) } @@ -481,9 +480,7 @@ impl PyList { #[pymethod(name = "__imul__")] fn imul(zelf: PyRef, counter: isize, _vm: &VirtualMachine) -> PyRef { - let new_elements = seq_mul(&zelf.elements().as_slice(), counter) - .cloned() - .collect(); + let new_elements = seq_mul(&zelf.borrow_sequence(), counter).cloned().collect(); zelf.elements.replace(new_elements); zelf } @@ -567,8 +564,8 @@ impl PyList { } else if let Some(other) = other.payload_if_subclass::(vm) { Implemented(seq_equal( vm, - &zelf.elements().as_slice(), - &other.elements().as_slice(), + &zelf.borrow_sequence(), + &other.borrow_sequence(), )?) } else { NotImplemented @@ -588,11 +585,7 @@ impl PyList { #[pymethod(name = "__lt__")] fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_lt( - vm, - &self.elements().as_slice(), - &other.elements().as_slice(), - )?; + let res = seq_lt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -602,11 +595,7 @@ impl PyList { #[pymethod(name = "__gt__")] fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_gt( - vm, - &self.elements().as_slice(), - &other.elements().as_slice(), - )?; + let res = seq_gt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -616,11 +605,7 @@ impl PyList { #[pymethod(name = "__ge__")] fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_ge( - vm, - &self.elements().as_slice(), - &other.elements().as_slice(), - )?; + let res = seq_ge(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -630,11 +615,7 @@ impl PyList { #[pymethod(name = "__le__")] fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_le( - vm, - &self.elements().as_slice(), - &other.elements().as_slice(), - )?; + let res = seq_le(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index 1f67106ec..3692371dc 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::marker::Sized; use std::ops::Range; @@ -255,12 +254,30 @@ pub trait SimpleSeq { fn iter(&self) -> DynPyIter; } -impl SimpleSeq for &[PyObjectRef] { +// impl SimpleSeq for &[PyObjectRef] { +// fn len(&self) -> usize { +// (&**self).len() +// } +// fn iter(&self) -> DynPyIter { +// Box::new((&**self).iter()) +// } +// } + +impl SimpleSeq for Vec { fn len(&self) -> usize { - (&**self).len() + self.len() } fn iter(&self) -> DynPyIter { - Box::new((&**self).iter()) + Box::new(self.as_slice().iter()) + } +} + +impl SimpleSeq for std::cell::Ref<'_, Vec> { + fn len(&self) -> usize { + self.as_slice().len() + } + fn iter(&self) -> DynPyIter { + Box::new(self.as_slice().iter()) } } @@ -381,7 +398,7 @@ impl<'a> Iterator for SeqMul<'a> { } impl ExactSizeIterator for SeqMul<'_> {} -pub fn seq_mul(seq: &impl SimpleSeq, repetitions: isize) -> SeqMul { +pub fn seq_mul<'a>(seq: &'a impl SimpleSeq, repetitions: isize) -> SeqMul<'a> { SeqMul { seq, repetitions: repetitions.max(0) as usize, diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 90f969723..dcf58239b 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -2,14 +2,14 @@ use std::cell::Cell; use std::fmt; use super::objiter; -use super::objsequence::{get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul}; +use super::objsequence::{get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SimpleSeq}; use super::objtype::PyClassRef; use crate::function::OptionalArg; use crate::pyhash; use crate::pyobject::{ - IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, + IntoPyObject, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyContext, PyObjectRef, + PyRef, PyResult, PyValue, }; -use crate::sequence::PySequenceContainer; use crate::vm::{ReprGuard, VirtualMachine}; /// tuple() -> empty tuple @@ -58,19 +58,17 @@ impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4)); impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4), (F, 5)); impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4), (F, 5), (G, 6)); -impl PySequenceContainer for PyTuple { - fn as_slice(&self) -> &[PyObjectRef] { - &self.elements - } -} - impl PyTuple { pub fn fast_getitem(&self, idx: usize) -> PyObjectRef { self.elements[idx].clone() } pub fn as_slice(&self) -> &[PyObjectRef] { - ::as_slice(self) + &self.elements + } + + pub fn as_sequence<'a>(&'a self) -> &'a impl SimpleSeq { + &self.elements } } @@ -83,36 +81,36 @@ pub fn get_value(obj: &PyObjectRef) -> &[PyObjectRef] { #[pyimpl] impl PyTuple { #[inline] - fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult + fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult where - F: Fn(&[PyObjectRef], &[PyObjectRef]) -> PyResult, + F: Fn(&Vec, &Vec) -> PyResult, { let r = if let Some(other) = other.payload_if_subclass::(vm) { - vm.new_bool(op(self.as_slice(), other.as_slice())?) + Implemented(op(&self.elements, &other.elements)?) } else { - vm.ctx.not_implemented() + NotImplemented }; Ok(r) } #[pymethod(name = "__lt__")] - fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_lt(vm, &a, &b), vm) + fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| seq_lt(vm, a, b), vm) } #[pymethod(name = "__gt__")] - fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_gt(vm, &a, &b), vm) + fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| seq_gt(vm, a, b), vm) } #[pymethod(name = "__ge__")] - fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_ge(vm, &a, &b), vm) + fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| seq_ge(vm, a, b), vm) } #[pymethod(name = "__le__")] - fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_le(vm, &a, &b), vm) + fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| seq_le(vm, a, b), vm) } #[pymethod(name = "__add__")] @@ -147,13 +145,13 @@ impl PyTuple { } #[pymethod(name = "__eq__")] - fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_equal(vm, &a, &b), vm) + fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| seq_equal(vm, a, b), vm) } #[pymethod(name = "__ne__")] - fn ne(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| Ok(!seq_equal(vm, &a, &b)?), vm) + fn ne(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + Ok(self.eq(other, vm)?.map(|v| !v)) } #[pymethod(name = "__hash__")] @@ -196,7 +194,7 @@ impl PyTuple { #[pymethod(name = "__mul__")] fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.as_slice(), counter).cloned().collect(); + let new_elements = seq_mul(&self.elements, counter).cloned().collect(); vm.ctx.new_tuple(new_elements) } diff --git a/vm/src/py_serde.rs b/vm/src/py_serde.rs index 3c8b05a8c..c26d03608 100644 --- a/vm/src/py_serde.rs +++ b/vm/src/py_serde.rs @@ -88,7 +88,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { serializer.serialize_i64(v.to_i64().ok_or_else(int_too_large)?) } } else if let Some(list) = self.pyobject.payload_if_subclass::(self.vm) { - serialize_seq_elements(serializer, &list.elements()) + serialize_seq_elements(serializer, &list.borrow_elements()) } else if let Some(tuple) = self.pyobject.payload_if_subclass::(self.vm) { serialize_seq_elements(serializer, tuple.as_slice()) } else if objtype::isinstance(self.pyobject, &self.vm.ctx.dict_type()) { diff --git a/vm/src/sequence.rs b/vm/src/sequence.rs index f6e494c7d..c0d618ee9 100644 --- a/vm/src/sequence.rs +++ b/vm/src/sequence.rs @@ -1,22 +1,2 @@ use crate::pyobject::{PyObjectRef, PyResult, PyValue}; use crate::vm::VirtualMachine; - -pub trait PySequenceContainer -where - Self: PyValue, -{ - fn as_slice(&self) -> &[PyObjectRef]; - - #[inline] - fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult - where - F: Fn(&[PyObjectRef], &[PyObjectRef]) -> PyResult, - { - let r = if let Some(other) = other.payload_if_subclass::(vm) { - vm.new_bool(op(self.as_slice(), other.as_slice())?) - } else { - vm.ctx.not_implemented() - }; - Ok(r) - } -} diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index 2f0760eae..0f6306d08 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -107,7 +107,7 @@ impl PopenRef { let command_list = match &args.args { Either::A(command) => vec![command.as_str().to_string()], Either::B(command_list) => command_list - .elements() + .borrow_elements() .iter() .map(|x| objstr::get_value(x)) .collect(), diff --git a/vm/src/vm.rs b/vm/src/vm.rs index fd2fdf85d..cd3784405 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -916,7 +916,7 @@ impl VirtualMachine { value .payload::() .unwrap() - .elements() + .borrow_elements() .iter() .map(|obj| T::try_from_object(self, obj.clone())) .collect() From 18fcd995844498328f7b23603ed39c0037f8e45c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 7 Jan 2020 22:11:26 +0900 Subject: [PATCH 07/11] SimpleSeq for PyDeque --- vm/src/obj/objsequence.rs | 20 ++++++++++---------- vm/src/stdlib/collections.rs | 26 ++++++++++++++++---------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index 3692371dc..7b0a70ebb 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -1,5 +1,5 @@ use std::marker::Sized; -use std::ops::Range; +use std::ops::{Deref, Range}; use num_bigint::{BigInt, ToBigInt}; use num_traits::{One, Signed, ToPrimitive, Zero}; @@ -272,15 +272,6 @@ impl SimpleSeq for Vec { } } -impl SimpleSeq for std::cell::Ref<'_, Vec> { - fn len(&self) -> usize { - self.as_slice().len() - } - fn iter(&self) -> DynPyIter { - Box::new(self.as_slice().iter()) - } -} - impl SimpleSeq for std::collections::VecDeque { fn len(&self) -> usize { self.len() @@ -290,6 +281,15 @@ impl SimpleSeq for std::collections::VecDeque { } } +impl SimpleSeq for std::cell::Ref<'_, T> where T: SimpleSeq { + fn len(&self) -> usize { + self.deref().len() + } + fn iter(&self) -> DynPyIter { + self.deref().iter() + } +} + // impl<'a, I> pub fn seq_equal( diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index eeabf0692..fbe329ac0 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -27,6 +27,12 @@ struct PyDequeOptions { maxlen: Option, } +impl PyDeque { + pub fn borrow_sequence<'a>(&'a self) -> impl objsequence::SimpleSeq + 'a { + self.deque.borrow() + } +} + #[pyimpl] impl PyDeque { #[pyslot(new)] @@ -238,8 +244,8 @@ impl PyDeque { _ => return Ok(vm.ctx.not_implemented()), }); - let lhs: &VecDeque<_> = &zelf.deque.borrow(); - let rhs: &VecDeque<_> = &other.deque.borrow(); + let lhs = &zelf.borrow_sequence(); + let rhs = &other.borrow_sequence(); let eq = objsequence::seq_equal(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) @@ -256,8 +262,8 @@ impl PyDeque { _ => return Ok(vm.ctx.not_implemented()), }); - let lhs: &VecDeque<_> = &zelf.deque.borrow(); - let rhs: &VecDeque<_> = &other.deque.borrow(); + let lhs = &zelf.borrow_sequence(); + let rhs = &other.borrow_sequence(); let eq = objsequence::seq_lt(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) @@ -274,8 +280,8 @@ impl PyDeque { _ => return Ok(vm.ctx.not_implemented()), }); - let lhs: &VecDeque<_> = &zelf.deque.borrow(); - let rhs: &VecDeque<_> = &other.deque.borrow(); + let lhs = &zelf.borrow_sequence(); + let rhs = &other.borrow_sequence(); let eq = objsequence::seq_gt(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) @@ -292,8 +298,8 @@ impl PyDeque { _ => return Ok(vm.ctx.not_implemented()), }); - let lhs: &VecDeque<_> = &zelf.deque.borrow(); - let rhs: &VecDeque<_> = &other.deque.borrow(); + let lhs = &zelf.borrow_sequence(); + let rhs = &other.borrow_sequence(); let eq = objsequence::seq_le(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) @@ -310,8 +316,8 @@ impl PyDeque { _ => return Ok(vm.ctx.not_implemented()), }); - let lhs: &VecDeque<_> = &zelf.deque.borrow(); - let rhs: &VecDeque<_> = &other.deque.borrow(); + let lhs = &zelf.borrow_sequence(); + let rhs = &other.borrow_sequence(); let eq = objsequence::seq_ge(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) From 226ebfa0c65ac8db5e63e008d32481865eb61345 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 7 Jan 2020 22:24:05 +0900 Subject: [PATCH 08/11] vm::sequence for SimpleSeq utilities --- vm/src/obj/objlist.rs | 23 ++--- vm/src/obj/objsequence.rs | 164 +---------------------------------- vm/src/obj/objtuple.rs | 17 ++-- vm/src/sequence.rs | 146 ++++++++++++++++++++++++++++++- vm/src/stdlib/collections.rs | 17 ++-- 5 files changed, 179 insertions(+), 188 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index b874103b3..5ce46c1b4 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -10,9 +10,7 @@ use super::objbool; use super::objbyteinner; use super::objint::PyIntRef; use super::objiter; -use super::objsequence::{ - get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SequenceIndex, SimpleSeq, -}; +use super::objsequence::{get_item, SequenceIndex}; use super::objslice::PySliceRef; use super::objtype::PyClassRef; use crate::function::OptionalArg; @@ -20,6 +18,7 @@ use crate::pyobject::{ IdProtocol, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; +use crate::sequence::{self, SimpleSeq}; use crate::vm::{ReprGuard, VirtualMachine}; /// Built-in mutable sequence. @@ -469,7 +468,9 @@ impl PyList { #[pymethod(name = "__mul__")] fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.borrow_sequence(), counter).cloned().collect(); + let new_elements = sequence::seq_mul(&self.borrow_sequence(), counter) + .cloned() + .collect(); vm.ctx.new_list(new_elements) } @@ -480,7 +481,9 @@ impl PyList { #[pymethod(name = "__imul__")] fn imul(zelf: PyRef, counter: isize, _vm: &VirtualMachine) -> PyRef { - let new_elements = seq_mul(&zelf.borrow_sequence(), counter).cloned().collect(); + let new_elements = sequence::seq_mul(&zelf.borrow_sequence(), counter) + .cloned() + .collect(); zelf.elements.replace(new_elements); zelf } @@ -562,7 +565,7 @@ impl PyList { let value = if zelf.as_object().is(&other) { Implemented(true) } else if let Some(other) = other.payload_if_subclass::(vm) { - Implemented(seq_equal( + Implemented(sequence::eq( vm, &zelf.borrow_sequence(), &other.borrow_sequence(), @@ -585,7 +588,7 @@ impl PyList { #[pymethod(name = "__lt__")] fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_lt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; + let res = sequence::lt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -595,7 +598,7 @@ impl PyList { #[pymethod(name = "__gt__")] fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_gt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; + let res = sequence::gt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -605,7 +608,7 @@ impl PyList { #[pymethod(name = "__ge__")] fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_ge(vm, &self.borrow_sequence(), &other.borrow_sequence())?; + let res = sequence::ge(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -615,7 +618,7 @@ impl PyList { #[pymethod(name = "__le__")] fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if let Some(other) = other.payload_if_subclass::(vm) { - let res = seq_le(vm, &self.borrow_sequence(), &other.borrow_sequence())?; + let res = sequence::le(vm, &self.borrow_sequence(), &other.borrow_sequence())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index 7b0a70ebb..4b1590f20 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -1,5 +1,5 @@ use std::marker::Sized; -use std::ops::{Deref, Range}; +use std::ops::Range; use num_bigint::{BigInt, ToBigInt}; use num_traits::{One, Signed, ToPrimitive, Zero}; @@ -10,7 +10,7 @@ use super::objnone::PyNone; use super::objslice::{PySlice, PySliceRef}; use super::objtuple::PyTuple; use crate::function::OptionalArg; -use crate::pyobject::{IdProtocol, PyObject, PyObjectRef, PyResult, TryFromObject, TypeProtocol}; +use crate::pyobject::{PyObject, PyObjectRef, PyResult, TryFromObject, TypeProtocol}; use crate::vm::VirtualMachine; pub trait PySliceableSequence { @@ -246,166 +246,6 @@ pub fn get_item( } } -type DynPyIter<'a> = Box + 'a>; - -#[allow(clippy::len_without_is_empty)] -pub trait SimpleSeq { - fn len(&self) -> usize; - fn iter(&self) -> DynPyIter; -} - -// impl SimpleSeq for &[PyObjectRef] { -// fn len(&self) -> usize { -// (&**self).len() -// } -// fn iter(&self) -> DynPyIter { -// Box::new((&**self).iter()) -// } -// } - -impl SimpleSeq for Vec { - fn len(&self) -> usize { - self.len() - } - fn iter(&self) -> DynPyIter { - Box::new(self.as_slice().iter()) - } -} - -impl SimpleSeq for std::collections::VecDeque { - fn len(&self) -> usize { - self.len() - } - fn iter(&self) -> DynPyIter { - Box::new(self.iter()) - } -} - -impl SimpleSeq for std::cell::Ref<'_, T> where T: SimpleSeq { - fn len(&self) -> usize { - self.deref().len() - } - fn iter(&self) -> DynPyIter { - self.deref().iter() - } -} - -// impl<'a, I> - -pub fn seq_equal( - vm: &VirtualMachine, - zelf: &impl SimpleSeq, - other: &impl SimpleSeq, -) -> PyResult { - if zelf.len() == other.len() { - for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { - if a.is(b) { - continue; - } - if !vm.bool_eq(a.clone(), b.clone())? { - return Ok(false); - } - } - Ok(true) - } else { - Ok(false) - } -} - -pub fn seq_lt( - vm: &VirtualMachine, - zelf: &impl SimpleSeq, - other: &impl SimpleSeq, -) -> PyResult { - for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { - if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { - return Ok(v); - } - } - Ok(zelf.len() < other.len()) -} - -pub fn seq_gt( - vm: &VirtualMachine, - zelf: &impl SimpleSeq, - other: &impl SimpleSeq, -) -> PyResult { - for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { - if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { - return Ok(v); - } - } - Ok(zelf.len() > other.len()) -} - -pub fn seq_ge( - vm: &VirtualMachine, - zelf: &impl SimpleSeq, - other: &impl SimpleSeq, -) -> PyResult { - for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { - if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { - return Ok(v); - } - } - - Ok(zelf.len() >= other.len()) -} - -pub fn seq_le( - vm: &VirtualMachine, - zelf: &impl SimpleSeq, - other: &impl SimpleSeq, -) -> PyResult { - for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { - if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { - return Ok(v); - } - } - - Ok(zelf.len() <= other.len()) -} - -pub struct SeqMul<'a> { - seq: &'a dyn SimpleSeq, - repetitions: usize, - iter: Option>, -} -impl<'a> Iterator for SeqMul<'a> { - type Item = &'a PyObjectRef; - fn next(&mut self) -> Option { - if self.seq.len() == 0 { - return None; - } - match self.iter.as_mut().and_then(Iterator::next) { - Some(item) => Some(item), - None => { - if self.repetitions == 0 { - None - } else { - self.repetitions -= 1; - self.iter = Some(self.seq.iter()); - self.next() - } - } - } - } - fn size_hint(&self) -> (usize, Option) { - let size = self.iter.as_ref().map_or(0, ExactSizeIterator::len) - + (self.repetitions * self.seq.len()); - (size, Some(size)) - } -} -impl ExactSizeIterator for SeqMul<'_> {} - -pub fn seq_mul<'a>(seq: &'a impl SimpleSeq, repetitions: isize) -> SeqMul<'a> { - SeqMul { - seq, - repetitions: repetitions.max(0) as usize, - iter: None, - } -} - //Check if given arg could be used with PySliceableSequence.get_slice_range() pub fn is_valid_slice_arg( arg: OptionalArg, diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index dcf58239b..77e383a73 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -2,7 +2,7 @@ use std::cell::Cell; use std::fmt; use super::objiter; -use super::objsequence::{get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul, SimpleSeq}; +use super::objsequence::get_item; use super::objtype::PyClassRef; use crate::function::OptionalArg; use crate::pyhash; @@ -10,6 +10,7 @@ use crate::pyobject::{ IntoPyObject, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, }; +use crate::sequence::{self, SimpleSeq}; use crate::vm::{ReprGuard, VirtualMachine}; /// tuple() -> empty tuple @@ -95,22 +96,22 @@ impl PyTuple { #[pymethod(name = "__lt__")] fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_lt(vm, a, b), vm) + self.cmp(other, |a, b| sequence::lt(vm, a, b), vm) } #[pymethod(name = "__gt__")] fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_gt(vm, a, b), vm) + self.cmp(other, |a, b| sequence::gt(vm, a, b), vm) } #[pymethod(name = "__ge__")] fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_ge(vm, a, b), vm) + self.cmp(other, |a, b| sequence::ge(vm, a, b), vm) } #[pymethod(name = "__le__")] fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_le(vm, a, b), vm) + self.cmp(other, |a, b| sequence::le(vm, a, b), vm) } #[pymethod(name = "__add__")] @@ -146,7 +147,7 @@ impl PyTuple { #[pymethod(name = "__eq__")] fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self.cmp(other, |a, b| seq_equal(vm, a, b), vm) + self.cmp(other, |a, b| sequence::eq(vm, a, b), vm) } #[pymethod(name = "__ne__")] @@ -194,7 +195,9 @@ impl PyTuple { #[pymethod(name = "__mul__")] fn mul(&self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements, counter).cloned().collect(); + let new_elements = sequence::seq_mul(&self.elements, counter) + .cloned() + .collect(); vm.ctx.new_tuple(new_elements) } diff --git a/vm/src/sequence.rs b/vm/src/sequence.rs index c0d618ee9..31c631571 100644 --- a/vm/src/sequence.rs +++ b/vm/src/sequence.rs @@ -1,2 +1,146 @@ -use crate::pyobject::{PyObjectRef, PyResult, PyValue}; +use crate::pyobject::{IdProtocol, PyObjectRef, PyResult}; use crate::vm::VirtualMachine; +use std::ops::Deref; + +type DynPyIter<'a> = Box + 'a>; + +#[allow(clippy::len_without_is_empty)] +pub trait SimpleSeq { + fn len(&self) -> usize; + fn iter(&self) -> DynPyIter; +} + +// impl SimpleSeq for &[PyObjectRef] { +// fn len(&self) -> usize { +// (&**self).len() +// } +// fn iter(&self) -> DynPyIter { +// Box::new((&**self).iter()) +// } +// } + +impl SimpleSeq for Vec { + fn len(&self) -> usize { + self.len() + } + fn iter(&self) -> DynPyIter { + Box::new(self.as_slice().iter()) + } +} + +impl SimpleSeq for std::collections::VecDeque { + fn len(&self) -> usize { + self.len() + } + fn iter(&self) -> DynPyIter { + Box::new(self.iter()) + } +} + +impl SimpleSeq for std::cell::Ref<'_, T> +where + T: SimpleSeq, +{ + fn len(&self) -> usize { + self.deref().len() + } + fn iter(&self) -> DynPyIter { + self.deref().iter() + } +} + +// impl<'a, I> + +pub fn eq(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { + if zelf.len() == other.len() { + for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { + if a.is(b) { + continue; + } + if !vm.bool_eq(a.clone(), b.clone())? { + return Ok(false); + } + } + Ok(true) + } else { + Ok(false) + } +} + +pub fn lt(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { + for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { + if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { + return Ok(v); + } + } + Ok(zelf.len() < other.len()) +} + +pub fn gt(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { + for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { + if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { + return Ok(v); + } + } + Ok(zelf.len() > other.len()) +} + +pub fn ge(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { + for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { + if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { + return Ok(v); + } + } + + Ok(zelf.len() >= other.len()) +} + +pub fn le(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { + for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { + if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { + return Ok(v); + } + } + + Ok(zelf.len() <= other.len()) +} + +pub struct SeqMul<'a> { + seq: &'a dyn SimpleSeq, + repetitions: usize, + iter: Option>, +} +impl<'a> Iterator for SeqMul<'a> { + type Item = &'a PyObjectRef; + fn next(&mut self) -> Option { + if self.seq.len() == 0 { + return None; + } + match self.iter.as_mut().and_then(Iterator::next) { + Some(item) => Some(item), + None => { + if self.repetitions == 0 { + None + } else { + self.repetitions -= 1; + self.iter = Some(self.seq.iter()); + self.next() + } + } + } + } + fn size_hint(&self) -> (usize, Option) { + let size = self.iter.as_ref().map_or(0, ExactSizeIterator::len) + + (self.repetitions * self.seq.len()); + (size, Some(size)) + } +} +impl ExactSizeIterator for SeqMul<'_> {} + +pub fn seq_mul<'a>(seq: &'a impl SimpleSeq, repetitions: isize) -> SeqMul<'a> { + SeqMul { + seq, + repetitions: repetitions.max(0) as usize, + iter: None, + } +} diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index fbe329ac0..b307b4c33 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -1,6 +1,7 @@ use crate::function::OptionalArg; -use crate::obj::{objiter, objsequence, objtype::PyClassRef}; +use crate::obj::{objiter, objtype::PyClassRef}; use crate::pyobject::{IdProtocol, PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::sequence::{self, SimpleSeq}; use crate::vm::ReprGuard; use crate::VirtualMachine; use itertools::Itertools; @@ -28,7 +29,7 @@ struct PyDequeOptions { } impl PyDeque { - pub fn borrow_sequence<'a>(&'a self) -> impl objsequence::SimpleSeq + 'a { + pub fn borrow_sequence<'a>(&'a self) -> impl SimpleSeq + 'a { self.deque.borrow() } } @@ -247,7 +248,7 @@ impl PyDeque { let lhs = &zelf.borrow_sequence(); let rhs = &other.borrow_sequence(); - let eq = objsequence::seq_equal(vm, lhs, rhs)?; + let eq = sequence::eq(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) } @@ -265,7 +266,7 @@ impl PyDeque { let lhs = &zelf.borrow_sequence(); let rhs = &other.borrow_sequence(); - let eq = objsequence::seq_lt(vm, lhs, rhs)?; + let eq = sequence::lt(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) } @@ -283,7 +284,7 @@ impl PyDeque { let lhs = &zelf.borrow_sequence(); let rhs = &other.borrow_sequence(); - let eq = objsequence::seq_gt(vm, lhs, rhs)?; + let eq = sequence::gt(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) } @@ -301,7 +302,7 @@ impl PyDeque { let lhs = &zelf.borrow_sequence(); let rhs = &other.borrow_sequence(); - let eq = objsequence::seq_le(vm, lhs, rhs)?; + let eq = sequence::le(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) } @@ -319,14 +320,14 @@ impl PyDeque { let lhs = &zelf.borrow_sequence(); let rhs = &other.borrow_sequence(); - let eq = objsequence::seq_ge(vm, lhs, rhs)?; + let eq = sequence::ge(vm, lhs, rhs)?; Ok(vm.new_bool(eq)) } #[pymethod(name = "__mul__")] fn mul(&self, n: isize, _vm: &VirtualMachine) -> Self { let deque: &VecDeque<_> = &self.deque.borrow(); - let mul = objsequence::seq_mul(deque, n); + let mul = sequence::seq_mul(deque, n); let skipped = if let Some(maxlen) = self.maxlen.get() { mul.len() - maxlen } else { From db67018decbf59e599fe4f82c9bc539350a8bf9d Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 8 Jan 2020 01:36:23 +0900 Subject: [PATCH 09/11] PyList::cmp --- vm/src/obj/objlist.rs | 64 +++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 5ce46c1b4..36e6d3a7f 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -556,24 +556,30 @@ impl PyList { } } + #[inline] + fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult + where + F: Fn(&Vec, &Vec) -> PyResult, + { + let r = if let Some(other) = other.payload_if_subclass::(vm) { + Implemented(op(&*self.borrow_elements(), &*other.borrow_elements())?) + } else { + NotImplemented + }; + Ok(r) + } + #[pymethod(name = "__eq__")] fn eq( zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine, ) -> PyResult { - let value = if zelf.as_object().is(&other) { - Implemented(true) - } else if let Some(other) = other.payload_if_subclass::(vm) { - Implemented(sequence::eq( - vm, - &zelf.borrow_sequence(), - &other.borrow_sequence(), - )?) + if zelf.as_object().is(&other) { + Ok(Implemented(true)) } else { - NotImplemented - }; - Ok(value) + zelf.cmp(other, |a, b| sequence::eq(vm, a, b), vm) + } } #[pymethod(name = "__ne__")] @@ -586,43 +592,23 @@ impl PyList { } #[pymethod(name = "__lt__")] - fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = sequence::lt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| sequence::lt(vm, a, b), vm) } #[pymethod(name = "__gt__")] - fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = sequence::gt(vm, &self.borrow_sequence(), &other.borrow_sequence())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| sequence::gt(vm, a, b), vm) } #[pymethod(name = "__ge__")] - fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = sequence::ge(vm, &self.borrow_sequence(), &other.borrow_sequence())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| sequence::ge(vm, a, b), vm) } #[pymethod(name = "__le__")] - fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(other) = other.payload_if_subclass::(vm) { - let res = sequence::le(vm, &self.borrow_sequence(), &other.borrow_sequence())?; - Ok(vm.new_bool(res)) - } else { - Ok(vm.ctx.not_implemented()) - } + fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.cmp(other, |a, b| sequence::le(vm, a, b), vm) } #[pymethod(name = "__delitem__")] From 5a3937fd15c58a7b53a66e085e7fc183188d4c9b Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 8 Jan 2020 02:00:57 +0900 Subject: [PATCH 10/11] Refactor deque and fix comparison bug --- tests/snippets/stdlib_collections.py | 7 ++ vm/src/obj/objtuple.rs | 2 +- vm/src/sequence.rs | 2 +- vm/src/stdlib/collections.rs | 133 ++++++++++++++------------- 4 files changed, 76 insertions(+), 68 deletions(-) diff --git a/tests/snippets/stdlib_collections.py b/tests/snippets/stdlib_collections.py index ecc1945cc..5c1583103 100644 --- a/tests/snippets/stdlib_collections.py +++ b/tests/snippets/stdlib_collections.py @@ -37,3 +37,10 @@ assert deque(maxlen=3) == deque() assert deque([1, 2, 3, 4], maxlen=2) == deque([3, 4]) assert len(deque([1, 2, 3, 4])) == 4 + +assert d >= d +assert not (d > d) +assert d <= d +assert not (d < d) +assert d == d +assert not (d != d) diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 77e383a73..919ec3723 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -68,7 +68,7 @@ impl PyTuple { &self.elements } - pub fn as_sequence<'a>(&'a self) -> &'a impl SimpleSeq { + pub fn as_sequence(&self) -> &impl SimpleSeq { &self.elements } } diff --git a/vm/src/sequence.rs b/vm/src/sequence.rs index 31c631571..8cf53af8a 100644 --- a/vm/src/sequence.rs +++ b/vm/src/sequence.rs @@ -137,7 +137,7 @@ impl<'a> Iterator for SeqMul<'a> { } impl ExactSizeIterator for SeqMul<'_> {} -pub fn seq_mul<'a>(seq: &'a impl SimpleSeq, repetitions: isize) -> SeqMul<'a> { +pub fn seq_mul(seq: &impl SimpleSeq, repetitions: isize) -> SeqMul { SeqMul { seq, repetitions: repetitions.max(0) as usize, diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index b307b4c33..8f3da7ad1 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -1,6 +1,9 @@ use crate::function::OptionalArg; use crate::obj::{objiter, objtype::PyClassRef}; -use crate::pyobject::{IdProtocol, PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::pyobject::{ + IdProtocol, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyIterable, PyObjectRef, + PyRef, PyResult, PyValue, +}; use crate::sequence::{self, SimpleSeq}; use crate::vm::ReprGuard; use crate::VirtualMachine; @@ -29,7 +32,7 @@ struct PyDequeOptions { } impl PyDeque { - pub fn borrow_sequence<'a>(&'a self) -> impl SimpleSeq + 'a { + pub fn borrow_deque<'a>(&'a self) -> impl std::ops::Deref> + 'a { self.deque.borrow() } } @@ -208,6 +211,7 @@ impl PyDeque { fn maxlen(&self, _vm: &VirtualMachine) -> Option { self.maxlen.get() } + #[pyproperty(setter)] fn set_maxlen(&self, maxlen: Option, _vm: &VirtualMachine) { self.maxlen.set(maxlen); @@ -234,94 +238,91 @@ impl PyDeque { Ok(repr) } + #[inline] + fn cmp(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult + where + F: Fn(&VecDeque, &VecDeque) -> PyResult, + { + let r = if let Some(other) = other.payload_if_subclass::(vm) { + Implemented(op(&*self.borrow_deque(), &*other.borrow_deque())?) + } else { + NotImplemented + }; + Ok(r) + } + #[pymethod(name = "__eq__")] - fn eq(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn eq( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { if zelf.as_object().is(&other) { - return Ok(vm.new_bool(true)); + Ok(Implemented(true)) + } else { + zelf.cmp(other, |a, b| sequence::eq(vm, a, b), vm) } + } - let other = match_class!(match other { - other @ Self => other, - _ => return Ok(vm.ctx.not_implemented()), - }); - - let lhs = &zelf.borrow_sequence(); - let rhs = &other.borrow_sequence(); - - let eq = sequence::eq(vm, lhs, rhs)?; - Ok(vm.new_bool(eq)) + #[pymethod(name = "__ne__")] + fn ne( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + Ok(PyDeque::eq(zelf, other, vm)?.map(|v| !v)) } #[pymethod(name = "__lt__")] - fn lt(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn lt( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { if zelf.as_object().is(&other) { - return Ok(vm.new_bool(true)); + Ok(Implemented(false)) + } else { + zelf.cmp(other, |a, b| sequence::lt(vm, a, b), vm) } - - let other = match_class!(match other { - other @ Self => other, - _ => return Ok(vm.ctx.not_implemented()), - }); - - let lhs = &zelf.borrow_sequence(); - let rhs = &other.borrow_sequence(); - - let eq = sequence::lt(vm, lhs, rhs)?; - Ok(vm.new_bool(eq)) } #[pymethod(name = "__gt__")] - fn gt(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn gt( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { if zelf.as_object().is(&other) { - return Ok(vm.new_bool(true)); + Ok(Implemented(false)) + } else { + zelf.cmp(other, |a, b| sequence::gt(vm, a, b), vm) } - - let other = match_class!(match other { - other @ Self => other, - _ => return Ok(vm.ctx.not_implemented()), - }); - - let lhs = &zelf.borrow_sequence(); - let rhs = &other.borrow_sequence(); - - let eq = sequence::gt(vm, lhs, rhs)?; - Ok(vm.new_bool(eq)) } #[pymethod(name = "__le__")] - fn le(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn le( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { if zelf.as_object().is(&other) { - return Ok(vm.new_bool(true)); + Ok(Implemented(true)) + } else { + zelf.cmp(other, |a, b| sequence::le(vm, a, b), vm) } - - let other = match_class!(match other { - other @ Self => other, - _ => return Ok(vm.ctx.not_implemented()), - }); - - let lhs = &zelf.borrow_sequence(); - let rhs = &other.borrow_sequence(); - - let eq = sequence::le(vm, lhs, rhs)?; - Ok(vm.new_bool(eq)) } #[pymethod(name = "__ge__")] - fn ge(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn ge( + zelf: PyRef, + other: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { if zelf.as_object().is(&other) { - return Ok(vm.new_bool(true)); + Ok(Implemented(true)) + } else { + zelf.cmp(other, |a, b| sequence::ge(vm, a, b), vm) } - - let other = match_class!(match other { - other @ Self => other, - _ => return Ok(vm.ctx.not_implemented()), - }); - - let lhs = &zelf.borrow_sequence(); - let rhs = &other.borrow_sequence(); - - let eq = sequence::ge(vm, lhs, rhs)?; - Ok(vm.new_bool(eq)) } #[pymethod(name = "__mul__")] From 5e2035f9799b3e9f900441fe38c1f03f433464b7 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 8 Jan 2020 02:28:58 +0900 Subject: [PATCH 11/11] remove unnessessary pub --- vm/src/obj/objlist.rs | 15 ++++++++------ vm/src/obj/objtuple.rs | 8 ++------ vm/src/sequence.rs | 38 ++++++++++++++++++++++++++++-------- vm/src/stdlib/collections.rs | 2 +- 4 files changed, 42 insertions(+), 21 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 36e6d3a7f..d4bd8e02c 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -63,11 +63,11 @@ impl PyList { } impl PyList { - pub fn get_len(&self) -> usize { + fn get_len(&self) -> usize { self.elements.borrow().len() } - pub fn get_pos(&self, p: i32) -> Option { + fn get_pos(&self, p: i32) -> Option { // convert a (potentially negative) positon into a real index if p < 0 { if -p as usize > self.get_len() { @@ -82,7 +82,7 @@ impl PyList { } } - pub fn get_slice_pos(&self, slice_pos: &BigInt) -> usize { + fn get_slice_pos(&self, slice_pos: &BigInt) -> usize { if let Some(pos) = slice_pos.to_i32() { if let Some(index) = self.get_pos(pos) { // within bounds @@ -99,7 +99,7 @@ impl PyList { } } - pub fn get_slice_range(&self, start: &Option, stop: &Option) -> Range { + fn get_slice_range(&self, start: &Option, stop: &Option) -> Range { let start = start.as_ref().map(|x| self.get_slice_pos(x)).unwrap_or(0); let stop = stop .as_ref() @@ -109,7 +109,10 @@ impl PyList { start..stop } - pub fn get_byte_inner(&self, vm: &VirtualMachine) -> PyResult { + pub(crate) fn get_byte_inner( + &self, + vm: &VirtualMachine, + ) -> PyResult { let mut elements = Vec::::with_capacity(self.get_len()); for elem in self.elements.borrow().iter() { match PyIntRef::try_from_object(vm, elem.clone()) { @@ -146,7 +149,7 @@ pub type PyListRef = PyRef; #[pyimpl] impl PyList { #[pymethod] - pub fn append(&self, x: PyObjectRef, _vm: &VirtualMachine) { + pub(crate) fn append(&self, x: PyObjectRef, _vm: &VirtualMachine) { self.elements.borrow_mut().push(x); } diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 919ec3723..8e2e525a4 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -60,22 +60,18 @@ impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4), (F, 5)); impl_intopyobj_tuple!((A, 0), (B, 1), (C, 2), (D, 3), (E, 4), (F, 5), (G, 6)); impl PyTuple { - pub fn fast_getitem(&self, idx: usize) -> PyObjectRef { + pub(crate) fn fast_getitem(&self, idx: usize) -> PyObjectRef { self.elements[idx].clone() } pub fn as_slice(&self) -> &[PyObjectRef] { &self.elements } - - pub fn as_sequence(&self) -> &impl SimpleSeq { - &self.elements - } } pub type PyTupleRef = PyRef; -pub fn get_value(obj: &PyObjectRef) -> &[PyObjectRef] { +pub(crate) fn get_value(obj: &PyObjectRef) -> &[PyObjectRef] { obj.payload::().unwrap().as_slice() } diff --git a/vm/src/sequence.rs b/vm/src/sequence.rs index 8cf53af8a..2855c711a 100644 --- a/vm/src/sequence.rs +++ b/vm/src/sequence.rs @@ -51,7 +51,11 @@ where // impl<'a, I> -pub fn eq(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { +pub(crate) fn eq( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if a.is(b) { @@ -67,7 +71,11 @@ pub fn eq(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> } } -pub fn lt(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { +pub(crate) fn lt( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { return Ok(v); @@ -76,7 +84,11 @@ pub fn lt(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> Ok(zelf.len() < other.len()) } -pub fn gt(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { +pub(crate) fn gt( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { return Ok(v); @@ -85,7 +97,11 @@ pub fn gt(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> Ok(zelf.len() > other.len()) } -pub fn ge(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { +pub(crate) fn ge( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_gt(a.clone(), b.clone())? { return Ok(v); @@ -95,7 +111,11 @@ pub fn ge(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> Ok(zelf.len() >= other.len()) } -pub fn le(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> PyResult { +pub(crate) fn le( + vm: &VirtualMachine, + zelf: &impl SimpleSeq, + other: &impl SimpleSeq, +) -> PyResult { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { if let Some(v) = vm.bool_seq_lt(a.clone(), b.clone())? { return Ok(v); @@ -105,11 +125,14 @@ pub fn le(vm: &VirtualMachine, zelf: &impl SimpleSeq, other: &impl SimpleSeq) -> Ok(zelf.len() <= other.len()) } -pub struct SeqMul<'a> { +pub(crate) struct SeqMul<'a> { seq: &'a dyn SimpleSeq, repetitions: usize, iter: Option>, } + +impl ExactSizeIterator for SeqMul<'_> {} + impl<'a> Iterator for SeqMul<'a> { type Item = &'a PyObjectRef; fn next(&mut self) -> Option { @@ -135,9 +158,8 @@ impl<'a> Iterator for SeqMul<'a> { (size, Some(size)) } } -impl ExactSizeIterator for SeqMul<'_> {} -pub fn seq_mul(seq: &impl SimpleSeq, repetitions: isize) -> SeqMul { +pub(crate) fn seq_mul(seq: &impl SimpleSeq, repetitions: isize) -> SeqMul { SeqMul { seq, repetitions: repetitions.max(0) as usize, diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 8f3da7ad1..4bf1a0e76 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -32,7 +32,7 @@ struct PyDequeOptions { } impl PyDeque { - pub fn borrow_deque<'a>(&'a self) -> impl std::ops::Deref> + 'a { + fn borrow_deque<'a>(&'a self) -> impl std::ops::Deref> + 'a { self.deque.borrow() } }