Merge pull request #423 from althonos/master

Fix derive of `PySequenceProtocol` trait.
2019-03-30 20:36:56 -07:00 · 2019-03-30 20:36:56 -07:00 · 3ce8886ad6
parent 9a9b519bcb dfd1fa55c0
commit 3ce8886ad6
5 changed files with 229 additions and 18 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## Fixed

 * `type_object::PyTypeObject` has been marked unsafe because breaking the contract `type_object::PyTypeObject::init_type` can lead to UB.
+ * Fixed automatic derive of `PySequenceProtocol` implementation in [#423](https://github.com/PyO3/pyo3/pull/423).

 ## [0.6.0] - 2018-03-28

@ -66,7 +67,7 @@ default = ["extension-module"]

 ### Fixed

- * A soudness hole where every instances of a `#[pyclass]` struct was considered to be part of a python object, even though you can create instances that are not part of the python heap. This was fixed through `PyRef` and `PyRefMut`. 
+ * A soudness hole where every instances of a `#[pyclass]` struct was considered to be part of a python object, even though you can create instances that are not part of the python heap. This was fixed through `PyRef` and `PyRefMut`.
 * Fix kwargs support in [#328](https://github.com/PyO3/pyo3/pull/328).
 * Add full support for `__dict__` in [#403](https://github.com/PyO3/pyo3/pull/403).

--- a/pyo3-derive-backend/src/defs.rs
+++ b/pyo3-derive-backend/src/defs.rs
@ -318,20 +318,22 @@ pub const SEQ: Proto = Proto {
            pyres: false,
            proto: "pyo3::class::sequence::PySequenceLenProtocol",
        },
-        MethodProto::Unary {
+        MethodProto::Binary {
            name: "__getitem__",
+            arg: "Index",
            pyres: true,
            proto: "pyo3::class::sequence::PySequenceGetItemProtocol",
        },
-        MethodProto::Binary {
+        MethodProto::Ternary {
            name: "__setitem__",
-            arg: "Value",
+            arg1: "Index",
+            arg2: "Value",
            pyres: false,
            proto: "pyo3::class::sequence::PySequenceSetItemProtocol",
        },
        MethodProto::Binary {
            name: "__delitem__",
-            arg: "Key",
+            arg: "Index",
            pyres: false,
            proto: "pyo3::class::sequence::PySequenceDelItemProtocol",
        },
@ -347,8 +349,9 @@ pub const SEQ: Proto = Proto {
            pyres: true,
            proto: "pyo3::class::sequence::PySequenceConcatProtocol",
        },
-        MethodProto::Unary {
+        MethodProto::Binary {
            name: "__repeat__",
+            arg: "Index",
            pyres: true,
            proto: "pyo3::class::sequence::PySequenceRepeatProtocol",
        },
@ -358,8 +361,9 @@ pub const SEQ: Proto = Proto {
            pyres: true,
            proto: "pyo3::class::sequence::PySequenceInplaceConcatProtocol",
        },
-        MethodProto::Unary {
+        MethodProto::Binary {
            name: "__inplace_repeat__",
+            arg: "Index",
            pyres: true,
            proto: "pyo3::class::sequence::PySequenceInplaceRepeatProtocol",
        },
--- a/src/class/macros.rs
+++ b/src/class/macros.rs
@ -183,7 +183,7 @@ macro_rules! py_ssizearg_func {
            let _pool = $crate::GILPool::new();
            let py = $crate::Python::assume_gil_acquired();
            let slf = py.mut_from_borrowed_ptr::<T>(slf);
-            let result = slf.$f(arg as isize).into();
+            let result = slf.$f(arg.into()).into();
            $crate::callback::cb_convert($conv, py, result)
        }
        Some(wrap::<$class>)
--- a/src/class/sequence.rs
+++ b/src/class/sequence.rs
@ -14,7 +14,7 @@ use crate::Python;
 use crate::{FromPyObject, IntoPyObject};
 use std::os::raw::c_int;

-/// Sequece interface
+/// Sequence interface
 #[allow(unused_variables)]
 pub trait PySequenceProtocol<'p>: PyTypeInfo + Sized {
    fn __len__(&'p self) -> Self::Result
@ -24,21 +24,21 @@ pub trait PySequenceProtocol<'p>: PyTypeInfo + Sized {
        unimplemented!()
    }

-    fn __getitem__(&'p self, key: isize) -> Self::Result
+    fn __getitem__(&'p self, idx: Self::Index) -> Self::Result
    where
        Self: PySequenceGetItemProtocol<'p>,
    {
        unimplemented!()
    }

-    fn __setitem__(&'p mut self, key: isize, value: Self::Value) -> Self::Result
+    fn __setitem__(&'p mut self, idx: Self::Index, value: Self::Value) -> Self::Result
    where
        Self: PySequenceSetItemProtocol<'p>,
    {
        unimplemented!()
    }

-    fn __delitem__(&'p mut self, key: isize) -> Self::Result
+    fn __delitem__(&'p mut self, idx: Self::Index) -> Self::Result
    where
        Self: PySequenceDelItemProtocol<'p>,
    {
@ -59,7 +59,7 @@ pub trait PySequenceProtocol<'p>: PyTypeInfo + Sized {
        unimplemented!()
    }

-    fn __repeat__(&'p self, count: isize) -> Self::Result
+    fn __repeat__(&'p self, count: Self::Index) -> Self::Result
    where
        Self: PySequenceRepeatProtocol<'p>,
    {
@ -73,7 +73,7 @@ pub trait PySequenceProtocol<'p>: PyTypeInfo + Sized {
        unimplemented!()
    }

-    fn __inplace_repeat__(&'p mut self, count: isize) -> Self::Result
+    fn __inplace_repeat__(&'p mut self, count: Self::Index) -> Self::Result
    where
        Self: PySequenceInplaceRepeatProtocol<'p>,
    {
@ -89,16 +89,19 @@ pub trait PySequenceLenProtocol<'p>: PySequenceProtocol<'p> {
 }

 pub trait PySequenceGetItemProtocol<'p>: PySequenceProtocol<'p> {
+    type Index: FromPyObject<'p> + From<isize>;
    type Success: IntoPyObject;
    type Result: Into<PyResult<Self::Success>>;
 }

 pub trait PySequenceSetItemProtocol<'p>: PySequenceProtocol<'p> {
+    type Index: FromPyObject<'p> + From<isize>;
    type Value: FromPyObject<'p>;
    type Result: Into<PyResult<()>>;
 }

 pub trait PySequenceDelItemProtocol<'p>: PySequenceProtocol<'p> {
+    type Index: FromPyObject<'p> + From<isize>;
    type Result: Into<PyResult<()>>;
 }

@ -114,6 +117,7 @@ pub trait PySequenceConcatProtocol<'p>: PySequenceProtocol<'p> {
 }

 pub trait PySequenceRepeatProtocol<'p>: PySequenceProtocol<'p> {
+    type Index: FromPyObject<'p> + From<isize>;
    type Success: IntoPyObject;
    type Result: Into<PyResult<Self::Success>>;
 }
@ -124,6 +128,7 @@ pub trait PySequenceInplaceConcatProtocol<'p>: PySequenceProtocol<'p> + IntoPyOb
 }

 pub trait PySequenceInplaceRepeatProtocol<'p>: PySequenceProtocol<'p> + IntoPyObject {
+    type Index: FromPyObject<'p> + From<isize>;
    type Result: Into<PyResult<Self>>;
 }

@ -228,7 +233,7 @@ where
            } else {
                let value = py.from_borrowed_ptr::<PyAny>(value);
                match value.extract() {
-                    Ok(value) => slf.__setitem__(key as isize, value).into(),
+                    Ok(value) => slf.__setitem__(key.into(), value).into(),
                    Err(e) => Err(e),
                }
            };
@ -295,7 +300,7 @@ mod sq_ass_item_impl {
                let slf = py.mut_from_borrowed_ptr::<T>(slf);

                let result = if value.is_null() {
-                    slf.__delitem__(key as isize).into()
+                    slf.__delitem__(key.into()).into()
                } else {
                    Err(PyErr::new::<exceptions::NotImplementedError, _>(format!(
                        "Item assignment not supported by {:?}",
@ -341,11 +346,11 @@ mod sq_ass_item_impl {
                let slf = py.mut_from_borrowed_ptr::<T>(slf);

                let result = if value.is_null() {
-                    slf.__delitem__(key as isize).into()
+                    slf.__delitem__(key.into()).into()
                } else {
                    let value = py.from_borrowed_ptr::<PyAny>(value);
                    match value.extract() {
-                        Ok(value) => slf.__setitem__(key as isize, value).into(),
+                        Ok(value) => slf.__setitem__(key.into(), value).into(),
                        Err(e) => Err(e),
                    }
                };
--- a/tests/test_sequence.rs
+++ b/tests/test_sequence.rs
@ -0,0 +1,201 @@
+use pyo3::class::PySequenceProtocol;
+use pyo3::exceptions::IndexError;
+use pyo3::exceptions::ValueError;
+use pyo3::prelude::*;
+use pyo3::types::IntoPyDict;
+use pyo3::types::PyAny;
+use pyo3::types::PyList;
+
+#[macro_use]
+mod common;
+
+#[pyclass]
+struct ByteSequence {
+    elements: Vec<u8>,
+}
+
+#[pymethods]
+impl ByteSequence {
+    #[new]
+    fn new(obj: &PyRawObject, elements: Option<&PyList>) -> PyResult<()> {
+        if let Some(pylist) = elements {
+            let mut elems = Vec::with_capacity(pylist.len());
+            for pyelem in pylist.into_iter() {
+                let elem = u8::extract(pyelem)?;
+                elems.push(elem);
+            }
+            obj.init(Self { elements: elems });
+        } else {
+            obj.init(Self {
+                elements: Vec::new(),
+            });
+        }
+        Ok(())
+    }
+}
+
+#[pyproto]
+impl PySequenceProtocol for ByteSequence {
+    fn __len__(&self) -> PyResult<usize> {
+        Ok(self.elements.len())
+    }
+
+    fn __getitem__(&self, idx: isize) -> PyResult<u8> {
+        self.elements
+            .get(idx as usize)
+            .map(|&byte| byte)
+            .ok_or(IndexError::py_err("list index out of range"))
+    }
+
+    fn __setitem__(&mut self, idx: isize, value: u8) -> PyResult<()> {
+        self.elements[idx as usize] = value;
+        Ok(())
+    }
+
+    fn __delitem__(&mut self, idx: isize) -> PyResult<()> {
+        if (idx < self.elements.len() as isize) && (idx >= 0) {
+            self.elements.remove(idx as usize);
+            Ok(())
+        } else {
+            Err(IndexError::py_err("list index out of range"))
+        }
+    }
+
+    fn __contains__(&self, other: &PyAny) -> PyResult<bool> {
+        match u8::extract(other) {
+            Ok(ref x) => Ok(self.elements.contains(x)),
+            Err(_) => Ok(false),
+        }
+    }
+
+    fn __concat__(&self, other: &Self) -> PyResult<Self> {
+        let mut elements = self.elements.clone();
+        elements.extend_from_slice(&other.elements);
+        Ok(Self { elements })
+    }
+
+    fn __repeat__(&self, count: isize) -> PyResult<Self> {
+        if count >= 0 {
+            let mut elements = Vec::with_capacity(self.elements.len() * count as usize);
+            for _ in 0..count {
+                elements.extend(&self.elements);
+            }
+            Ok(Self { elements })
+        } else {
+            Err(ValueError::py_err("invalid repeat count"))
+        }
+    }
+}
+
+#[test]
+fn test_getitem() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s = ByteSequence([1, 2, 3]); assert s[0] == 1");
+    run("s = ByteSequence([1, 2, 3]); assert s[1] == 2");
+    run("s = ByteSequence([1, 2, 3]); assert s[2] == 3");
+    err("s = ByteSequence([1, 2, 3]); print(s[-4])");
+    err("s = ByteSequence([1, 2, 3]); print(s[4])");
+}
+
+#[test]
+fn test_setitem() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s = ByteSequence([1, 2, 3]); s[0] = 4; assert list(s) == [4, 2, 3]");
+    err("s = ByteSequence([1, 2, 3]); s[0] = 'hello'");
+}
+
+#[test]
+fn test_delitem() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s = ByteSequence([1, 2, 3]); del s[0]; assert list(s) == [2, 3]");
+    run("s = ByteSequence([1, 2, 3]); del s[1]; assert list(s) == [1, 3]");
+    run("s = ByteSequence([1, 2, 3]); del s[-1]; assert list(s) == [1, 2]");
+    run("s = ByteSequence([1, 2, 3]); del s[-2]; assert list(s) == [1, 3]");
+    err("s = ByteSequence([1, 2, 3]); del s[-4]; print(list(s))");
+    err("s = ByteSequence([1, 2, 3]); del s[4]");
+}
+
+#[test]
+fn test_contains() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+
+    run("s = ByteSequence([1, 2, 3]); assert 1 in s");
+    run("s = ByteSequence([1, 2, 3]); assert 2 in s");
+    run("s = ByteSequence([1, 2, 3]); assert 3 in s");
+    run("s = ByteSequence([1, 2, 3]); assert 4 not in s");
+    run("s = ByteSequence([1, 2, 3]); assert 'hello' not in s");
+}
+
+#[test]
+fn test_concat() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s1 = ByteSequence([1, 2]); s2 = ByteSequence([3, 4]); assert list(s1+s2) == [1, 2, 3, 4]");
+    err("s1 = ByteSequence([1, 2]); s2 = 'hello'; s1 + s2");
+}
+
+#[test]
+fn test_inplace_concat() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s = ByteSequence([1, 2]); s += ByteSequence([3, 4]); assert list(s) == [1, 2, 3, 4]");
+    err("s = ByteSequence([1, 2]); s += 'hello'");
+}
+
+#[test]
+fn test_repeat() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s1 = ByteSequence([1, 2, 3]); s2 = s1*2; assert list(s2) == [1, 2, 3, 1, 2, 3]");
+    err("s1 = ByteSequence([1, 2, 3]); s2 = s1*-1; assert list(s2) == [1, 2, 3, 1, 2, 3]");
+}
+
+#[test]
+fn test_inplace_repeat() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+
+    let d = [("ByteSequence", py.get_type::<ByteSequence>())].into_py_dict(py);
+    let run = |code| py.run(code, None, Some(d)).unwrap();
+    let err = |code| py.run(code, None, Some(d)).unwrap_err();
+
+    run("s = ByteSequence([1, 2]); s *= 3; assert list(s) == [1, 2, 1, 2, 1, 2]");
+    err("s = ByteSequence([1, 2); s *= -1");
+}