impl sequence protocol for PyDeque

qingshi163 · youknowone · commit 0c9235ce0a4c · 2022-03-12T16:12:12.000+09:00
diff --git a/Lib/test/list_tests.py b/Lib/test/list_tests.py
@@ -32,12 +32,16 @@ def test_init(self):
         self.assertNotEqual(id(a), id(b))
         self.assertEqual(a, b)
 
+    # TODO: RUSTPYTHON
+    @unittest.expectedFailure
     def test_getitem_error(self):
         a = []
         msg = "list indices must be integers or slices"
         with self.assertRaisesRegex(TypeError, msg):
             a['a']
 
+    # TODO: RUSTPYTHON
+    @unittest.expectedFailure
     def test_setitem_error(self):
         a = []
         msg = "list indices must be integers or slices"
@@ -102,6 +106,8 @@ def test_reversed(self):
         # Bug 3689: make sure list-reversed-iterator doesn't have __len__
         self.assertRaises(TypeError, len, reversed([1,2,3]))
 
+    # TODO: RUSTPYTHON
+    @unittest.expectedFailure
     def test_setitem(self):
         a = self.type2test([0, 1])
         a[0] = 0
diff --git a/vm/src/builtins/pystr.rs b/vm/src/builtins/pystr.rs
@@ -1289,8 +1289,8 @@ impl PyStr {
 
 impl AsSequence for PyStr {
     fn as_sequence(
-       _zelf: &PyObjectView<Self>,
-       _vm: &VirtualMachine,
+        _zelf: &PyObjectView<Self>,
+        _vm: &VirtualMachine,
     ) -> std::borrow::Cow<'static, PySequenceMethods> {
         std::borrow::Cow::Borrowed(&Self::SEQUENCE_METHDOS)
     }
diff --git a/vm/src/protocol/mapping.rs b/vm/src/protocol/mapping.rs
@@ -33,15 +33,17 @@ impl<'a> From<&'a PyObject> for PyMapping<'a> {
         }
     }
 
+    pub fn length_opt(&self, vm: &VirtualMachine) -> Option<PyResult<usize>> {
+        self.methods(vm).length.map(|f| f(self.0.clone(), vm))
+    }
+
     pub fn length(&self, vm: &VirtualMachine) -> PyResult<usize> {
-        if let Some(f) = self.methods(vm).length {
-            f(self.0.clone(), vm)
-        } else {
-            Err(vm.new_type_error(format!(
+        self.length_opt(vm).ok_or_else(|| {
+            vm.new_type_error(format!(
                 "object of type '{}' has no len() or not a mapping",
                 self.0.class().name()
-            )))
-        }
+            ))
+        })?
     }
 }
 
diff --git a/vm/src/protocol/object.rs b/vm/src/protocol/object.rs
@@ -409,10 +409,16 @@ impl PyObject {
 
     pub fn length(&self, vm: &VirtualMachine) -> PyResult<usize> {
         let seq = PySequence::from(self);
-        if let Ok(len) = seq.length(vm) {
-            Ok(len)
+        if let Some(len) = seq.length_opt(vm) {
+            len
         } else {
-            PyMapping::try_from_object(vm, self.to_owned())?.length(vm)
+            // TODO: refactor PyMapping
+            if let Ok(mapping) = PyMapping::try_from_object(vm, self.to_owned()) {
+                if let Some(len) = mapping.length_opt(vm) {
+                    return len;
+                }
+            }
+            return Err(vm.new_type_error(format!("object of type '{}' has no len()", &self)));
         }
     }
 
diff --git a/vm/src/protocol/sequence.rs b/vm/src/protocol/sequence.rs
@@ -107,15 +107,17 @@ impl PySequence<'_> {
         })
     }
 
+    pub fn length_opt(&self, vm: &VirtualMachine) -> Option<PyResult<usize>> {
+        self.methods(vm).length.map(|f| f(self, vm))
+    }
+
     pub fn length(&self, vm: &VirtualMachine) -> PyResult<usize> {
-        if let Some(f) = self.methods(vm).length {
-            f(self, vm)
-        } else {
-            Err(vm.new_type_error(format!(
+        self.length_opt(vm).ok_or_else(|| {
+            vm.new_type_error(format!(
                 "'{}' is not a sequence or has no len()",
                 self.obj.class().name()
-            )))
-        }
+            ))
+        })?
     }
 
     pub fn concat(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult {
diff --git a/vm/src/sliceable.rs b/vm/src/sliceable.rs
@@ -290,6 +290,7 @@ pub enum SequenceIndex<'a> {
 impl<'a> SequenceIndex<'a> {
     pub fn try_borrow_from_object(vm: &VirtualMachine, obj: &'a PyObject) -> PyResult<Self> {
         if let Some(index) = obj.payload::<PyInt>() {
+            // TODO: replace by number protocol
             index
                 .as_bigint()
                 .to_isize()
@@ -300,7 +301,7 @@ impl<'a> SequenceIndex<'a> {
         } else if let Some(slice) = obj.payload::<PySlice>() {
             Ok(Self::Slice(slice))
         } else if let Some(index) = vm.to_index_opt(obj.to_owned()) {
-            // TODO: replace by number protocol
+            // TODO: __index__ for indice is no more supported
             index?
                 .as_bigint()
                 .to_isize()
diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs
@@ -9,12 +9,12 @@ mod _collections {
         },
         common::lock::{PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard},
         function::{FuncArgs, KwArgs, OptionalArg},
-        protocol::PyIterReturn,
+        protocol::{PyIterReturn, PySequenceMethods},
         sequence::{MutObjectSequenceOp, ObjectSequenceOp},
         sliceable,
         sliceable::saturate_index,
         types::{
-            Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable,
+            AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable,
             PyComparisonOp, Unhashable,
         },
         vm::ReprGuard,
@@ -55,7 +55,7 @@ mod _collections {
         }
     }
 
-    #[pyimpl(flags(BASETYPE), with(Comparable, Hashable, Iterable))]
+    #[pyimpl(flags(BASETYPE), with(AsSequence, Comparable, Hashable, Iterable))]
     impl PyDeque {
         #[pyslot]
         fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
@@ -172,9 +172,13 @@ mod _collections {
 
         #[pymethod]
         fn extend(&self, iter: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
+            self._extend(&iter, vm)
+        }
+
+        fn _extend(&self, iter: &PyObject, vm: &VirtualMachine) -> PyResult<()> {
             self.state.fetch_add(1);
             let max_len = self.maxlen;
-            let mut elements: Vec<PyObjectRef> = vm.extract_elements(&iter)?;
+            let mut elements: Vec<PyObjectRef> = vm.extract_elements(iter)?;
             if let Some(max_len) = max_len {
                 if max_len > elements.len() {
                     let mut deque = self.borrow_deque_mut();
@@ -383,8 +387,12 @@ mod _collections {
 
         #[pymethod(magic)]
         fn contains(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
+            self._contains(&needle, vm)
+        }
+
+        fn _contains(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
             let start_state = self.state.load();
-            let ret = self.mut_contains(vm, &needle)?;
+            let ret = self.mut_contains(vm, needle)?;
             if start_state != self.state.load() {
                 Err(vm.new_runtime_error("deque mutated during iteration".to_owned()))
             } else {
@@ -436,6 +444,10 @@ mod _collections {
 
         #[pymethod(magic)]
         fn add(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
+            self.concat(&other, vm)
+        }
+
+        fn concat(&self, other: &PyObject, vm: &VirtualMachine) -> PyResult<Self> {
             if let Some(o) = other.payload_if_subclass::<PyDeque>(vm) {
                 let mut deque = self.borrow_deque().clone();
                 let elements = o.borrow_deque().clone();
@@ -498,6 +510,51 @@ mod _collections {
         }
     }
 
+    impl AsSequence for PyDeque {
+        fn as_sequence(
+            _zelf: &crate::PyObjectView<Self>,
+            _vm: &VirtualMachine,
+        ) -> std::borrow::Cow<'static, PySequenceMethods> {
+            std::borrow::Cow::Borrowed(&Self::SEQUENCE_METHDOS)
+        }
+    }
+
+    impl PyDeque {
+        const SEQUENCE_METHDOS: PySequenceMethods = PySequenceMethods {
+            length: Some(|seq, _vm| Ok(seq.obj_as::<Self>().len())),
+            concat: Some(|seq, other, vm| {
+                seq.obj_as::<Self>()
+                    .concat(other, vm)
+                    .map(|x| x.into_ref(vm).into())
+            }),
+            repeat: Some(|seq, n, vm| {
+                seq.obj_as::<Self>()
+                    .mul(n as isize, vm)
+                    .map(|x| x.into_ref(vm).into())
+            }),
+            item: Some(|seq, i, vm| seq.obj_as::<Self>().getitem(i, vm)),
+            ass_item: Some(|seq, i, value, vm| {
+                let zelf = seq.obj_as::<Self>();
+                if let Some(value) = value {
+                    zelf.setitem(i, value, vm)
+                } else {
+                    zelf.delitem(i, vm)
+                }
+            }),
+            contains: Some(|seq, needle, vm| seq.obj_as::<Self>()._contains(needle, vm)),
+            inplace_concat: Some(|seq, other, vm| {
+                let zelf = seq.obj_as::<Self>();
+                zelf._extend(other, vm)?;
+                Ok(zelf.to_owned().into())
+            }),
+            inplace_repeat: Some(|seq, n, vm| {
+                let zelf = seq.obj_as::<Self>();
+                Self::imul(zelf.to_owned(), n as isize, vm).map(|x| x.into())
+            }),
+            ..*PySequenceMethods::not_implemented()
+        };
+    }
+
     impl Comparable for PyDeque {
         fn cmp(
             zelf: &crate::PyObjectView<Self>,

Original file line number	Diff line number	Diff line change
`@@ -33,15 +33,17 @@ impl<'a> From<&'a PyObject> for PyMapping<'a> {`
`33`	`33`	`}`
`34`	`34`	`}`
`35`	`35`
	`36`	`+ pub fn length_opt(&self, vm: &VirtualMachine) -> Option<PyResult<usize>> {`
	`37`	`+ self.methods(vm).length.map(\|f\| f(self.0.clone(), vm))`
	`38`	`+ }`
	`39`	`+`
`36`	`40`	`pub fn length(&self, vm: &VirtualMachine) -> PyResult<usize> {`
`37`		`- if let Some(f) = self.methods(vm).length {`
`38`		`- f(self.0.clone(), vm)`
`39`		`- } else {`
`40`		`- Err(vm.new_type_error(format!(`
	`41`	`+ self.length_opt(vm).ok_or_else(\|\| {`
	`42`	`+ vm.new_type_error(format!(`
`41`	`43`	`"object of type '{}' has no len() or not a mapping",`
`42`	`44`	`self.0.class().name()`
`43`		`- )))`
`44`		`- }`
	`45`	`+ ))`
	`46`	`+ })?`
`45`	`47`	`}`
`46`	`48`	`}`
`47`	`49`