From 2be70eb353e3277d18f7394cec907b5a956708c2 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Mon, 1 Jul 2019 19:15:58 -0500 Subject: [PATCH 01/12] Add deque to collections --- vm/src/stdlib/collections.rs | 195 +++++++++++++++++++++++++++++++++++ vm/src/stdlib/mod.rs | 4 +- 2 files changed, 198 insertions(+), 1 deletion(-) create mode 100644 vm/src/stdlib/collections.rs diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs new file mode 100644 index 000000000..47c0d33cb --- /dev/null +++ b/vm/src/stdlib/collections.rs @@ -0,0 +1,195 @@ +use crate::function::OptionalArg; +use crate::obj::{objbool, objint::PyIntRef, objtype::PyClassRef}; +use crate::pyobject::{PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::VirtualMachine; +use std::cell::RefCell; +use std::collections::VecDeque; + +#[pyclass(name = "deque")] +#[derive(Debug, Clone)] +struct PyDeque { + deque: RefCell>, +} + +impl PyValue for PyDeque { + fn class(vm: &VirtualMachine) -> PyClassRef { + vm.class("_collections", "deque") + } +} + +#[pyimpl] +impl PyDeque { + #[pymethod(name = "__new__")] + fn new( + cls: PyClassRef, + iter: OptionalArg, + _maxlen: OptionalArg, + vm: &VirtualMachine, + ) -> PyResult> { + let deque = if let OptionalArg::Present(iter) = iter { + iter.iter(vm)?.collect::>()? + } else { + VecDeque::new() + }; + PyDeque { + deque: RefCell::new(deque), + } + .into_ref_with_type(vm, cls) + } + + #[pymethod] + fn append(&self, obj: PyObjectRef, _vm: &VirtualMachine) { + self.deque.borrow_mut().push_back(obj) + } + + #[pymethod] + fn appendleft(&self, obj: PyObjectRef, _vm: &VirtualMachine) { + self.deque.borrow_mut().push_front(obj) + } + + #[pymethod] + fn clear(&self, _vm: &VirtualMachine) { + self.deque.borrow_mut().clear() + } + + #[pymethod] + fn copy(&self, _vm: &VirtualMachine) -> Self { + self.clone() + } + + #[pymethod] + fn count(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let mut count = 0; + for elem in self.deque.borrow().iter() { + if objbool::boolval(vm, vm._eq(elem.clone(), obj.clone())?)? { + count += 1; + } + } + Ok(count) + } + + #[pymethod] + fn extend(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> { + // TODO: use length_hint here and for extendleft + let mut deque = self.deque.borrow_mut(); + for elem in iter.iter(vm)? { + deque.push_back(elem?); + } + Ok(()) + } + + #[pymethod] + fn extendleft(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> { + let mut deque = self.deque.borrow_mut(); + for elem in iter.iter(vm)? { + deque.push_front(elem?); + } + Ok(()) + } + + #[pymethod] + fn index( + &self, + obj: PyObjectRef, + start: OptionalArg, + stop: OptionalArg, + vm: &VirtualMachine, + ) -> PyResult { + let deque = self.deque.borrow(); + let start = start.unwrap_or(0); + let stop = stop.unwrap_or_else(|| deque.len()); + for (i, elem) in deque.iter().skip(start).take(stop - start).enumerate() { + if objbool::boolval(vm, vm._eq(elem.clone(), obj.clone())?)? { + return Ok(i); + } + } + Err(vm.new_value_error( + vm.to_repr(&obj) + .map(|repr| format!("{} is not in deque", repr)) + .unwrap_or_else(|_| String::new()), + )) + } + + #[pymethod] + fn insert(&self, idx: i32, obj: PyObjectRef, _vm: &VirtualMachine) -> PyResult<()> { + let mut deque = self.deque.borrow_mut(); + + let idx = if idx < 0 { + if -idx as usize > deque.len() { + 0 + } else { + deque.len() - ((-idx) as usize) + } + } else if idx as usize >= deque.len() { + deque.len() - 1 + } else { + idx as usize + }; + + deque.insert(idx, obj); + + Ok(()) + } + + #[pymethod] + fn pop(&self, vm: &VirtualMachine) -> PyResult { + self.deque + .borrow_mut() + .pop_back() + .ok_or_else(|| vm.new_index_error("pop from an empty deque".to_string())) + } + + #[pymethod] + fn popleft(&self, vm: &VirtualMachine) -> PyResult { + self.deque + .borrow_mut() + .pop_front() + .ok_or_else(|| vm.new_index_error("pop from an empty deque".to_string())) + } + + #[pymethod] + fn remove(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let mut deque = self.deque.borrow_mut(); + let mut idx = None; + for (i, elem) in deque.iter().enumerate() { + if objbool::boolval(vm, vm._eq(elem.clone(), obj.clone())?)? { + idx = Some(i); + break; + } + } + idx.map(|idx| deque.remove(idx).unwrap()) + .ok_or_else(|| vm.new_value_error("deque.remove(x): x not in deque".to_string())) + } + + #[pymethod] + fn reverse(&self, _vm: &VirtualMachine) { + self.deque + .replace_with(|deque| deque.iter().cloned().rev().collect()); + } + + #[pymethod] + fn rotate(&self, mid: OptionalArg, _vm: &VirtualMachine) { + let mut deque = self.deque.borrow_mut(); + let mid = mid.unwrap_or(1); + // TODO: once `vecdeque_rotate` lands, use that instead + if mid < 0 { + for _ in 0..-mid { + if let Some(popped_front) = deque.pop_front() { + deque.push_back(popped_front); + } + } + } else { + for _ in 0..mid { + if let Some(popped_back) = deque.pop_back() { + deque.push_front(popped_back); + } + } + } + } +} + +pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { + py_module!(vm, "_collections", { + "deque" => PyDeque::make_class(&vm.ctx), + }) +} diff --git a/vm/src/stdlib/mod.rs b/vm/src/stdlib/mod.rs index 458a8cfb5..09249a375 100644 --- a/vm/src/stdlib/mod.rs +++ b/vm/src/stdlib/mod.rs @@ -1,6 +1,7 @@ #[cfg(feature = "rustpython-parser")] mod ast; mod binascii; +mod collections; mod dis; mod hashlib; mod imp; @@ -42,7 +43,8 @@ pub fn get_module_inits() -> HashMap { #[allow(unused_mut)] let mut modules = hashmap! { "binascii".to_string() => Box::new(binascii::make_module) as StdlibInitFunc, - "dis".to_string() => Box::new(dis::make_module) as StdlibInitFunc, + "dis".to_string() => Box::new(dis::make_module), + "_collections".to_string() => Box::new(collections::make_module), "hashlib".to_string() => Box::new(hashlib::make_module), "itertools".to_string() => Box::new(itertools::make_module), "json".to_string() => Box::new(json::make_module), From 5956bcbf2cd25b46eb1b66df3c780919023524c0 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Mon, 1 Jul 2019 19:49:21 -0500 Subject: [PATCH 02/12] Add maxlen argument and getter --- vm/src/stdlib/collections.rs | 43 ++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 47c0d33cb..2b9aea5be 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -1,5 +1,5 @@ use crate::function::OptionalArg; -use crate::obj::{objbool, objint::PyIntRef, objtype::PyClassRef}; +use crate::obj::{objbool, objtype::PyClassRef}; use crate::pyobject::{PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; use crate::VirtualMachine; use std::cell::RefCell; @@ -9,6 +9,7 @@ use std::collections::VecDeque; #[derive(Debug, Clone)] struct PyDeque { deque: RefCell>, + maxlen: Option, } impl PyValue for PyDeque { @@ -23,7 +24,7 @@ impl PyDeque { fn new( cls: PyClassRef, iter: OptionalArg, - _maxlen: OptionalArg, + maxlen: OptionalArg>, vm: &VirtualMachine, ) -> PyResult> { let deque = if let OptionalArg::Present(iter) = iter { @@ -33,18 +34,31 @@ impl PyDeque { }; PyDeque { deque: RefCell::new(deque), + maxlen: maxlen.into_option().and_then(|x| x), } .into_ref_with_type(vm, cls) } #[pymethod] fn append(&self, obj: PyObjectRef, _vm: &VirtualMachine) { - self.deque.borrow_mut().push_back(obj) + let mut deque = self.deque.borrow_mut(); + if let Some(maxlen) = self.maxlen { + if deque.len() == maxlen { + deque.pop_front(); + } + } + deque.push_back(obj); } #[pymethod] fn appendleft(&self, obj: PyObjectRef, _vm: &VirtualMachine) { - self.deque.borrow_mut().push_front(obj) + let mut deque = self.deque.borrow_mut(); + if let Some(maxlen) = self.maxlen { + if deque.len() == maxlen { + deque.pop_back(); + } + } + deque.push_front(obj); } #[pymethod] @@ -71,18 +85,16 @@ impl PyDeque { #[pymethod] fn extend(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> { // TODO: use length_hint here and for extendleft - let mut deque = self.deque.borrow_mut(); for elem in iter.iter(vm)? { - deque.push_back(elem?); + self.append(elem?, vm); } Ok(()) } #[pymethod] fn extendleft(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> { - let mut deque = self.deque.borrow_mut(); for elem in iter.iter(vm)? { - deque.push_front(elem?); + self.appendleft(elem?, vm); } Ok(()) } @@ -111,9 +123,15 @@ impl PyDeque { } #[pymethod] - fn insert(&self, idx: i32, obj: PyObjectRef, _vm: &VirtualMachine) -> PyResult<()> { + fn insert(&self, idx: i32, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut deque = self.deque.borrow_mut(); + if let Some(maxlen) = self.maxlen { + if deque.len() == maxlen { + return Err(vm.new_index_error("deque already at its maximum size".to_string())); + } + } + let idx = if idx < 0 { if -idx as usize > deque.len() { 0 @@ -186,6 +204,13 @@ impl PyDeque { } } } + + #[pyproperty] + fn maxlen(&self, _vm: &VirtualMachine) -> Option { + self.maxlen + } + + // TODO: proper repr } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { From 805d536fa80f79f70532708990faba76fee9cab2 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Jul 2019 16:40:06 -0500 Subject: [PATCH 03/12] Add repr to deque --- vm/src/stdlib/collections.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 2b9aea5be..4b502e5f8 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -1,7 +1,9 @@ use crate::function::OptionalArg; use crate::obj::{objbool, objtype::PyClassRef}; use crate::pyobject::{PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::vm::ReprGuard; use crate::VirtualMachine; +use itertools::Itertools; use std::cell::RefCell; use std::collections::VecDeque; @@ -18,6 +20,7 @@ impl PyValue for PyDeque { } } + #[pyimpl] impl PyDeque { #[pymethod(name = "__new__")] @@ -210,7 +213,25 @@ impl PyDeque { self.maxlen } - // TODO: proper repr + #[pymethod(name = "__repr__")] + fn repr(zelf: PyRef, vm: &VirtualMachine) -> PyResult { + let repr = if let Some(_guard) = ReprGuard::enter(zelf.as_object()) { + let elements = zelf + .deque + .borrow() + .iter() + .map(|obj| vm.to_repr(obj)) + .collect::, _>>()?; + let maxlen = zelf + .maxlen + .map(|maxlen| format!(", maxlen={}", maxlen)) + .unwrap_or_default(); + format!("deque([{}]{})", elements.into_iter().format(", "), maxlen) + } else { + "[...]".to_string() + }; + Ok(repr) + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { From 5153ad52caee07389213048f47e9560ecfa92e8d Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sat, 6 Jul 2019 17:28:57 -0500 Subject: [PATCH 04/12] Add cmp methods to deque --- vm/src/obj/objlist.rs | 10 ++-- vm/src/obj/objsequence.rs | 55 +++++++++++++++------ vm/src/obj/objtuple.rs | 10 ++-- vm/src/stdlib/collections.rs | 94 +++++++++++++++++++++++++++++++++++- 4 files changed, 142 insertions(+), 27 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 77c1b4c9f..92991bb21 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -491,7 +491,7 @@ impl PyListRef { if objtype::isinstance(&other, &vm.ctx.list_type()) { let zelf = self.elements.borrow(); let other = get_elements_list(&other); - let res = seq_equal(vm, &zelf, &other)?; + let res = seq_equal(vm, &zelf.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -502,7 +502,7 @@ impl PyListRef { 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, &other)?; + let res = seq_lt(vm, &zelf.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -513,7 +513,7 @@ impl PyListRef { 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, &other)?; + let res = seq_gt(vm, &zelf.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -524,7 +524,7 @@ impl PyListRef { 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, &other)?; + let res = seq_ge(vm, &zelf.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -535,7 +535,7 @@ impl PyListRef { 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, &other)?; + let res = seq_le(vm, &zelf.as_slice(), &other.as_slice())?; 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 d4226d51c..a141ca80f 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -216,10 +216,35 @@ pub fn get_item( } } -pub fn seq_equal( +pub trait SimpleSeq<'a> { + fn len(&self) -> usize; + fn iter(&'a self) -> Box + 'a>; +} + +impl<'a> SimpleSeq<'a> for &'a [PyObjectRef] { + fn len(&self) -> usize { + (&**self).len() + } + fn iter(&'a self) -> Box + 'a> { + Box::new((&**self).iter()) + } +} + +impl<'a> SimpleSeq<'a> for std::collections::VecDeque { + fn len(&self) -> usize { + self.len() + } + fn iter(&'a self) -> Box + 'a> { + Box::new(self.iter()) + } +} + +// impl<'a, I> + +pub fn seq_equal<'a>( vm: &VirtualMachine, - zelf: &[PyObjectRef], - other: &[PyObjectRef], + zelf: &'a dyn SimpleSeq<'a>, + other: &'a dyn SimpleSeq<'a>, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -237,10 +262,10 @@ pub fn seq_equal( } } -pub fn seq_lt( +pub fn seq_lt<'a>( vm: &VirtualMachine, - zelf: &[PyObjectRef], - other: &[PyObjectRef], + zelf: &'a dyn SimpleSeq<'a>, + other: &'a dyn SimpleSeq<'a>, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -277,10 +302,10 @@ pub fn seq_lt( } } -pub fn seq_gt( +pub fn seq_gt<'a>( vm: &VirtualMachine, - zelf: &[PyObjectRef], - other: &[PyObjectRef], + zelf: &'a dyn SimpleSeq<'a>, + other: &'a dyn SimpleSeq<'a>, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -316,18 +341,18 @@ pub fn seq_gt( } } -pub fn seq_ge( +pub fn seq_ge<'a>( vm: &VirtualMachine, - zelf: &[PyObjectRef], - other: &[PyObjectRef], + zelf: &'a dyn SimpleSeq<'a>, + other: &'a dyn SimpleSeq<'a>, ) -> Result { Ok(seq_gt(vm, zelf, other)? || seq_equal(vm, zelf, other)?) } -pub fn seq_le( +pub fn seq_le<'a>( vm: &VirtualMachine, - zelf: &[PyObjectRef], - other: &[PyObjectRef], + zelf: &'a dyn SimpleSeq<'a>, + other: &'a dyn SimpleSeq<'a>, ) -> Result { Ok(seq_lt(vm, zelf, other)? || seq_equal(vm, zelf, other)?) } diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index a6e181f3b..a4773e4e6 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -53,7 +53,7 @@ impl PyTupleRef { 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, &other)?; + let res = seq_lt(vm, &self.elements.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -63,7 +63,7 @@ impl PyTupleRef { 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, &other)?; + let res = seq_gt(vm, &self.elements.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -73,7 +73,7 @@ impl PyTupleRef { 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, &other)?; + let res = seq_ge(vm, &self.elements.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -83,7 +83,7 @@ impl PyTupleRef { 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, &other)?; + let res = seq_le(vm, &self.elements.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) @@ -122,7 +122,7 @@ impl PyTupleRef { fn eq(self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { if objtype::isinstance(&other, &vm.ctx.tuple_type()) { let other = get_elements_tuple(&other); - let res = seq_equal(vm, &self.elements, &other)?; + let res = seq_equal(vm, &self.elements.as_slice(), &other.as_slice())?; Ok(vm.new_bool(res)) } else { Ok(vm.ctx.not_implemented()) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 4b502e5f8..80646f65e 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -1,6 +1,6 @@ use crate::function::OptionalArg; -use crate::obj::{objbool, objtype::PyClassRef}; -use crate::pyobject::{PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::obj::{objbool, objsequence, objtype::PyClassRef}; +use crate::pyobject::{IdProtocol, PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; use crate::vm::ReprGuard; use crate::VirtualMachine; use itertools::Itertools; @@ -232,6 +232,96 @@ impl PyDeque { }; Ok(repr) } + + #[pymethod(name = "__eq__")] + fn eq(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if zelf.as_object().is(&other) { + return Ok(vm.new_bool(true)); + } + + let other = match_class!(other, + other @ Self => other, + _ => return Ok(vm.ctx.not_implemented()), + ); + + let lhs: &VecDeque<_> = &zelf.deque.borrow(); + let rhs: &VecDeque<_> = &other.deque.borrow(); + + let eq = objsequence::seq_equal(vm, lhs, rhs)?; + Ok(vm.new_bool(eq)) + } + + #[pymethod(name = "__lt__")] + fn lt(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if zelf.as_object().is(&other) { + return Ok(vm.new_bool(true)); + } + + let other = match_class!(other, + other @ Self => other, + _ => return Ok(vm.ctx.not_implemented()), + ); + + let lhs: &VecDeque<_> = &zelf.deque.borrow(); + let rhs: &VecDeque<_> = &other.deque.borrow(); + + let eq = objsequence::seq_lt(vm, lhs, rhs)?; + Ok(vm.new_bool(eq)) + } + + #[pymethod(name = "__gt__")] + fn gt(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if zelf.as_object().is(&other) { + return Ok(vm.new_bool(true)); + } + + let other = match_class!(other, + other @ Self => other, + _ => return Ok(vm.ctx.not_implemented()), + ); + + let lhs: &VecDeque<_> = &zelf.deque.borrow(); + let rhs: &VecDeque<_> = &other.deque.borrow(); + + let eq = objsequence::seq_gt(vm, lhs, rhs)?; + Ok(vm.new_bool(eq)) + } + + #[pymethod(name = "__le__")] + fn le(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if zelf.as_object().is(&other) { + return Ok(vm.new_bool(true)); + } + + let other = match_class!(other, + other @ Self => other, + _ => return Ok(vm.ctx.not_implemented()), + ); + + let lhs: &VecDeque<_> = &zelf.deque.borrow(); + let rhs: &VecDeque<_> = &other.deque.borrow(); + + let eq = objsequence::seq_le(vm, lhs, rhs)?; + Ok(vm.new_bool(eq)) + } + + #[pymethod(name = "__ge__")] + fn ge(zelf: PyRef, other: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if zelf.as_object().is(&other) { + return Ok(vm.new_bool(true)); + } + + let other = match_class!(other, + other @ Self => other, + _ => return Ok(vm.ctx.not_implemented()), + ); + + let lhs: &VecDeque<_> = &zelf.deque.borrow(); + let rhs: &VecDeque<_> = &other.deque.borrow(); + + let eq = objsequence::seq_ge(vm, lhs, rhs)?; + Ok(vm.new_bool(eq)) + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { From 0ca6d24902ebf51d718604c38ab8836c8ea1c608 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sat, 6 Jul 2019 17:49:03 -0500 Subject: [PATCH 05/12] Add deque tests --- tests/snippets/test_collections.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/snippets/test_collections.py diff --git a/tests/snippets/test_collections.py b/tests/snippets/test_collections.py new file mode 100644 index 000000000..4dd771a0c --- /dev/null +++ b/tests/snippets/test_collections.py @@ -0,0 +1,29 @@ +from collections import deque + + +d = deque([0, 1, 2]) + +d.append(1) +d.appendleft(3) + +assert d == deque([3, 0, 1, 2, 1]) + +assert d <= deque([4]) + +assert d.copy() is not d + +d = deque([1, 2, 3], 5) + +d.extend([4, 5, 6]) + +assert d == deque([2, 3, 4, 5, 6]) + +d.remove(4) + +assert d == deque([2, 3, 5, 6]) + +d.clear() + +assert d == deque() + +assert d == deque([], 4) From 1c1ee5bb69648bf2883b608c56f76282ecd22f4f Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sat, 6 Jul 2019 23:05:07 -0500 Subject: [PATCH 06/12] Add setter for maxlen --- vm/src/stdlib/collections.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 80646f65e..262aea94f 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -4,14 +4,14 @@ use crate::pyobject::{IdProtocol, PyClassImpl, PyIterable, PyObjectRef, PyRef, P use crate::vm::ReprGuard; use crate::VirtualMachine; use itertools::Itertools; -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::collections::VecDeque; #[pyclass(name = "deque")] #[derive(Debug, Clone)] struct PyDeque { deque: RefCell>, - maxlen: Option, + maxlen: Cell>, } impl PyValue for PyDeque { @@ -20,7 +20,6 @@ impl PyValue for PyDeque { } } - #[pyimpl] impl PyDeque { #[pymethod(name = "__new__")] @@ -37,7 +36,7 @@ impl PyDeque { }; PyDeque { deque: RefCell::new(deque), - maxlen: maxlen.into_option().and_then(|x| x), + maxlen: maxlen.into_option().and_then(|x| x).into(), } .into_ref_with_type(vm, cls) } @@ -45,22 +44,18 @@ impl PyDeque { #[pymethod] fn append(&self, obj: PyObjectRef, _vm: &VirtualMachine) { let mut deque = self.deque.borrow_mut(); - if let Some(maxlen) = self.maxlen { - if deque.len() == maxlen { + if self.maxlen.get() == Some(deque.len()) { deque.pop_front(); } - } deque.push_back(obj); } #[pymethod] fn appendleft(&self, obj: PyObjectRef, _vm: &VirtualMachine) { let mut deque = self.deque.borrow_mut(); - if let Some(maxlen) = self.maxlen { - if deque.len() == maxlen { + if self.maxlen.get() == Some(deque.len()) { deque.pop_back(); } - } deque.push_front(obj); } @@ -129,11 +124,9 @@ impl PyDeque { fn insert(&self, idx: i32, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut deque = self.deque.borrow_mut(); - if let Some(maxlen) = self.maxlen { - if deque.len() == maxlen { + if self.maxlen.get() == Some(deque.len()) { return Err(vm.new_index_error("deque already at its maximum size".to_string())); } - } let idx = if idx < 0 { if -idx as usize > deque.len() { @@ -210,7 +203,12 @@ impl PyDeque { #[pyproperty] fn maxlen(&self, _vm: &VirtualMachine) -> Option { - self.maxlen + self.maxlen.get() + } + #[pyproperty(setter)] + fn set_maxlen(&self, maxlen: Option, vm: &VirtualMachine) -> PyResult { + self.maxlen.set(maxlen); + Ok(vm.get_none()) } #[pymethod(name = "__repr__")] @@ -224,6 +222,7 @@ impl PyDeque { .collect::, _>>()?; let maxlen = zelf .maxlen + .get() .map(|maxlen| format!(", maxlen={}", maxlen)) .unwrap_or_default(); format!("deque([{}]{})", elements.into_iter().format(", "), maxlen) From 9d3fa18ec8eb8614efbeb72834940b7b012897b5 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sat, 6 Jul 2019 23:05:27 -0500 Subject: [PATCH 07/12] Add __mul__ to deque --- tests/snippets/test_collections.py | 4 ++ vm/src/obj/objlist.rs | 8 ++- vm/src/obj/objsequence.rs | 89 ++++++++++++++++++++---------- vm/src/obj/objtuple.rs | 7 ++- vm/src/stdlib/collections.rs | 28 ++++++++-- 5 files changed, 96 insertions(+), 40 deletions(-) diff --git a/tests/snippets/test_collections.py b/tests/snippets/test_collections.py index 4dd771a0c..820123c43 100644 --- a/tests/snippets/test_collections.py +++ b/tests/snippets/test_collections.py @@ -27,3 +27,7 @@ d.clear() assert d == deque() assert d == deque([], 4) + +assert deque([1, 2, 3]) * 2 == deque([1, 2, 3, 1, 2, 3]) + +assert deque([1, 2, 3], 4) * 2 == deque([3, 1, 2, 3]) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 92991bb21..b4d3a3f0f 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -392,12 +392,16 @@ impl PyListRef { } fn mul(self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements.borrow(), counter); + let new_elements = seq_mul(&self.elements.borrow().as_slice(), counter) + .cloned() + .collect(); vm.ctx.new_list(new_elements) } fn imul(self, counter: isize, _vm: &VirtualMachine) -> Self { - let new_elements = seq_mul(&self.elements.borrow(), counter); + let new_elements = seq_mul(&self.elements.borrow().as_slice(), counter) + .cloned() + .collect(); self.elements.replace(new_elements); self } diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index a141ca80f..b360a5d51 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -216,35 +216,37 @@ pub fn get_item( } } -pub trait SimpleSeq<'a> { +type DynPyIter<'a> = Box + 'a>; + +pub trait SimpleSeq { fn len(&self) -> usize; - fn iter(&'a self) -> Box + 'a>; + fn iter(&self) -> DynPyIter; } -impl<'a> SimpleSeq<'a> for &'a [PyObjectRef] { +impl SimpleSeq for &[PyObjectRef] { fn len(&self) -> usize { (&**self).len() } - fn iter(&'a self) -> Box + 'a> { + fn iter(&self) -> DynPyIter { Box::new((&**self).iter()) } } -impl<'a> SimpleSeq<'a> for std::collections::VecDeque { +impl SimpleSeq for std::collections::VecDeque { fn len(&self) -> usize { self.len() } - fn iter(&'a self) -> Box + 'a> { + fn iter(&self) -> DynPyIter { Box::new(self.iter()) } } // impl<'a, I> -pub fn seq_equal<'a>( +pub fn seq_equal( vm: &VirtualMachine, - zelf: &'a dyn SimpleSeq<'a>, - other: &'a dyn SimpleSeq<'a>, + zelf: &dyn SimpleSeq, + other: &dyn SimpleSeq, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -262,10 +264,10 @@ pub fn seq_equal<'a>( } } -pub fn seq_lt<'a>( +pub fn seq_lt( vm: &VirtualMachine, - zelf: &'a dyn SimpleSeq<'a>, - other: &'a dyn SimpleSeq<'a>, + zelf: &dyn SimpleSeq, + other: &dyn SimpleSeq, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -302,10 +304,10 @@ pub fn seq_lt<'a>( } } -pub fn seq_gt<'a>( +pub fn seq_gt( vm: &VirtualMachine, - zelf: &'a dyn SimpleSeq<'a>, - other: &'a dyn SimpleSeq<'a>, + zelf: &dyn SimpleSeq, + other: &dyn SimpleSeq, ) -> Result { if zelf.len() == other.len() { for (a, b) in Iterator::zip(zelf.iter(), other.iter()) { @@ -341,32 +343,61 @@ pub fn seq_gt<'a>( } } -pub fn seq_ge<'a>( +pub fn seq_ge( vm: &VirtualMachine, - zelf: &'a dyn SimpleSeq<'a>, - other: &'a dyn SimpleSeq<'a>, + zelf: &dyn SimpleSeq, + other: &dyn SimpleSeq, ) -> Result { Ok(seq_gt(vm, zelf, other)? || seq_equal(vm, zelf, other)?) } -pub fn seq_le<'a>( +pub fn seq_le( vm: &VirtualMachine, - zelf: &'a dyn SimpleSeq<'a>, - other: &'a dyn SimpleSeq<'a>, + zelf: &dyn SimpleSeq, + other: &dyn SimpleSeq, ) -> Result { Ok(seq_lt(vm, zelf, other)? || seq_equal(vm, zelf, other)?) } -pub fn seq_mul(elements: &[PyObjectRef], counter: isize) -> Vec { - let current_len = elements.len(); - let new_len = counter.max(0) as usize * current_len; - let mut new_elements = Vec::with_capacity(new_len); - - for _ in 0..counter { - new_elements.extend(elements.to_owned()); +pub struct SeqMul<'a> { + seq: &'a dyn SimpleSeq, + repetitions: usize, + iter: DynPyIter<'a>, +} +impl SeqMul<'_> { + fn next_iter(&mut self) { + self.repetitions -= 1; + self.iter = self.seq.iter(); } +} +impl<'a> Iterator for SeqMul<'a> { + type Item = &'a PyObjectRef; + fn next(&mut self) -> Option { + match self.iter.next() { + Some(item) => Some(item), + None => { + if self.repetitions == 0 { + None + } else { + self.next_iter(); + self.next() + } + } + } + } + fn size_hint(&self) -> (usize, Option) { + let size = self.iter.len() + (self.repetitions * self.seq.len()); + (size, Some(size)) + } +} +impl ExactSizeIterator for SeqMul<'_> {} - new_elements +pub fn seq_mul(seq: &dyn SimpleSeq, repetitions: isize) -> SeqMul { + SeqMul { + seq, + repetitions: repetitions.max(0) as usize - 1, + iter: seq.iter(), + } } pub fn get_elements_cell<'a>(obj: &'a PyObjectRef) -> &'a RefCell> { diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index a4773e4e6..e13d3160f 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -164,13 +164,14 @@ impl PyTupleRef { } fn mul(self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements, counter); + let new_elements = seq_mul(&self.elements.as_slice(), counter) + .cloned() + .collect(); vm.ctx.new_tuple(new_elements) } fn rmul(self, counter: isize, vm: &VirtualMachine) -> PyObjectRef { - let new_elements = seq_mul(&self.elements, counter); - vm.ctx.new_tuple(new_elements) + self.mul(counter, vm) } fn getitem(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 262aea94f..587d5ba1c 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -45,8 +45,8 @@ impl PyDeque { fn append(&self, obj: PyObjectRef, _vm: &VirtualMachine) { let mut deque = self.deque.borrow_mut(); if self.maxlen.get() == Some(deque.len()) { - deque.pop_front(); - } + deque.pop_front(); + } deque.push_back(obj); } @@ -54,8 +54,8 @@ impl PyDeque { fn appendleft(&self, obj: PyObjectRef, _vm: &VirtualMachine) { let mut deque = self.deque.borrow_mut(); if self.maxlen.get() == Some(deque.len()) { - deque.pop_back(); - } + deque.pop_back(); + } deque.push_front(obj); } @@ -125,8 +125,8 @@ impl PyDeque { let mut deque = self.deque.borrow_mut(); if self.maxlen.get() == Some(deque.len()) { - return Err(vm.new_index_error("deque already at its maximum size".to_string())); - } + return Err(vm.new_index_error("deque already at its maximum size".to_string())); + } let idx = if idx < 0 { if -idx as usize > deque.len() { @@ -321,6 +321,22 @@ impl PyDeque { let eq = objsequence::seq_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 skipped = if let Some(maxlen) = self.maxlen.get() { + mul.len() - maxlen + } else { + 0 + }; + let deque = mul.skip(skipped).cloned().collect(); + PyDeque { + deque: RefCell::new(deque), + maxlen: self.maxlen.clone(), + } + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { From d891c87c4f09479aef7861ea0e237ed9daa75a4a Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sat, 6 Jul 2019 23:29:31 -0500 Subject: [PATCH 08/12] Add maxlen as a named arg --- tests/snippets/test_collections.py | 4 ++++ vm/src/stdlib/collections.rs | 22 +++++++++++++--------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/snippets/test_collections.py b/tests/snippets/test_collections.py index 820123c43..8b44cefdb 100644 --- a/tests/snippets/test_collections.py +++ b/tests/snippets/test_collections.py @@ -31,3 +31,7 @@ assert d == deque([], 4) assert deque([1, 2, 3]) * 2 == deque([1, 2, 3, 1, 2, 3]) assert deque([1, 2, 3], 4) * 2 == deque([3, 1, 2, 3]) + +assert deque(maxlen=3) == deque() + +assert deque([1, 2, 3, 4], maxlen=2) == deque([3, 4]) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 587d5ba1c..9b3013e9c 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -20,25 +20,29 @@ impl PyValue for PyDeque { } } +#[derive(FromArgs)] +struct PyDequeOptions { + #[pyarg(positional_or_keyword, default = "None")] + maxlen: Option, +} + #[pyimpl] impl PyDeque { #[pymethod(name = "__new__")] fn new( cls: PyClassRef, iter: OptionalArg, - maxlen: OptionalArg>, + PyDequeOptions { maxlen }: PyDequeOptions, vm: &VirtualMachine, ) -> PyResult> { - let deque = if let OptionalArg::Present(iter) = iter { - iter.iter(vm)?.collect::>()? - } else { - VecDeque::new() + let py_deque = PyDeque { + deque: RefCell::default(), + maxlen: maxlen.into(), }; - PyDeque { - deque: RefCell::new(deque), - maxlen: maxlen.into_option().and_then(|x| x).into(), + if let OptionalArg::Present(iter) = iter { + py_deque.extend(iter, vm)?; } - .into_ref_with_type(vm, cls) + py_deque.into_ref_with_type(vm, cls) } #[pymethod] From deca878c4b1d21a110c52fed7e676e12804b7e0d Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sun, 7 Jul 2019 01:34:38 -0500 Subject: [PATCH 09/12] Use VecDeque::rotate_{left,right} --- vm/src/stdlib/collections.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 9b3013e9c..77c15f3d0 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -189,19 +189,10 @@ impl PyDeque { fn rotate(&self, mid: OptionalArg, _vm: &VirtualMachine) { let mut deque = self.deque.borrow_mut(); let mid = mid.unwrap_or(1); - // TODO: once `vecdeque_rotate` lands, use that instead if mid < 0 { - for _ in 0..-mid { - if let Some(popped_front) = deque.pop_front() { - deque.push_back(popped_front); - } - } + deque.rotate_left(-mid as usize); } else { - for _ in 0..mid { - if let Some(popped_back) = deque.pop_back() { - deque.push_front(popped_back); - } - } + deque.rotate_right(mid as usize); } } From fbeedb72bd91f2ea77d014ded0ce4a6abd8ae624 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sun, 7 Jul 2019 11:41:20 -0500 Subject: [PATCH 10/12] Fix seq_mul with negative numbers --- vm/src/obj/objsequence.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index b360a5d51..13e0761f6 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -362,31 +362,30 @@ pub fn seq_le( pub struct SeqMul<'a> { seq: &'a dyn SimpleSeq, repetitions: usize, - iter: DynPyIter<'a>, -} -impl SeqMul<'_> { - fn next_iter(&mut self) { - self.repetitions -= 1; - self.iter = self.seq.iter(); - } + iter: Option>, } impl<'a> Iterator for SeqMul<'a> { type Item = &'a PyObjectRef; fn next(&mut self) -> Option { - match self.iter.next() { + 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.next_iter(); + self.repetitions -= 1; + self.iter = Some(self.seq.iter()); self.next() } } } } fn size_hint(&self) -> (usize, Option) { - let size = self.iter.len() + (self.repetitions * self.seq.len()); + let size = self.iter.as_ref().map_or(0, ExactSizeIterator::len) + + (self.repetitions * self.seq.len()); (size, Some(size)) } } @@ -395,8 +394,8 @@ impl ExactSizeIterator for SeqMul<'_> {} pub fn seq_mul(seq: &dyn SimpleSeq, repetitions: isize) -> SeqMul { SeqMul { seq, - repetitions: repetitions.max(0) as usize - 1, - iter: seq.iter(), + repetitions: repetitions.max(0) as usize, + iter: None, } } From 1c1440eb781b66aaac5e517c36d78024949fd2c3 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Sun, 7 Jul 2019 15:34:12 -0500 Subject: [PATCH 11/12] Add __len__ to deque --- tests/snippets/test_collections.py | 2 ++ vm/src/stdlib/collections.rs | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/tests/snippets/test_collections.py b/tests/snippets/test_collections.py index 8b44cefdb..ecc1945cc 100644 --- a/tests/snippets/test_collections.py +++ b/tests/snippets/test_collections.py @@ -35,3 +35,5 @@ assert deque([1, 2, 3], 4) * 2 == deque([3, 1, 2, 3]) assert deque(maxlen=3) == deque() assert deque([1, 2, 3, 4], maxlen=2) == deque([3, 4]) + +assert len(deque([1, 2, 3, 4])) == 4 diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 77c15f3d0..f28912af1 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -332,6 +332,11 @@ impl PyDeque { maxlen: self.maxlen.clone(), } } + + #[pymethod(name = "__len__")] + fn len(&self, _vm: &VirtualMachine) -> usize { + self.deque.borrow().len() + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { From 5b32dfbed951cbacbd59b41d716fa0fc030c5a14 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Mon, 8 Jul 2019 13:24:06 -0500 Subject: [PATCH 12/12] Fix clippy and azure pipelines --- azure-pipelines.yml | 1 + vm/src/obj/objsequence.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 7271390c8..a7be2b4c9 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -22,6 +22,7 @@ jobs: "C:\Program Files\Git\mingw64\bin\curl.exe" -sSf -o rustup-init.exe https://win.rustup.rs/ .\rustup-init.exe -y set PATH=%PATH%;%USERPROFILE%\.cargo\bin + rustup update rustc -V cargo -V displayName: 'Installing Rust' diff --git a/vm/src/obj/objsequence.rs b/vm/src/obj/objsequence.rs index 13e0761f6..447322c8e 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/obj/objsequence.rs @@ -218,6 +218,7 @@ 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;