Merge pull request #3572 from davidhewitt/sequence2-try2

implement `PySequenceMethods`, try 2

src/ffi_ptr_ext.rs

@@ -0,0 +1,28 @@
+use crate::{ffi, instance::Py2, PyAny, PyResult, Python};
+
+mod sealed {
+    use super::*;
+
+    pub trait Sealed {}
+
+    impl Sealed for *mut ffi::PyObject {}
+}
+
+use sealed::Sealed;
+
+pub(crate) trait FfiPtrExt: Sealed {
+    unsafe fn assume_owned_or_err(self, py: Python<'_>) -> PyResult<Py2<'_, PyAny>>;
+    unsafe fn assume_owned(self, py: Python<'_>) -> Py2<'_, PyAny>;
+}
+
+impl FfiPtrExt for *mut ffi::PyObject {
+    #[inline]
+    unsafe fn assume_owned_or_err(self, py: Python<'_>) -> PyResult<Py2<'_, PyAny>> {
+        Py2::from_owned_ptr_or_err(py, self)
+    }
+
+    #[inline]
+    unsafe fn assume_owned(self, py: Python<'_>) -> Py2<'_, PyAny> {
+        Py2::from_owned_ptr(py, self)
+    }
+}

src/instance.rs

@@ -174,7 +174,7 @@ impl<'py, T> Py2<'py, T> {
     #[doc(hidden)] // public and doc(hidden) to use in examples and tests for now
     pub fn borrowed_from_gil_ref<'a>(gil_ref: &'a &'py T::AsRefTarget) -> &'a Self
     where
-        T: PyTypeInfo,
+        T: HasPyGilRef,
     {
         // Safety: &'py T::AsRefTarget is expected to be a Python pointer,
         // so &'a &'py T::AsRefTarget has the same layout as &'a Py2<'py, T>

src/lib.rs

@@ -311,6 +311,8 @@ pub use crate::version::PythonVersionInfo;
 
 // Expected to become public API in 0.21 under a different name
 pub(crate) use crate::instance::Py2;
+pub(crate) mod ffi_ptr_ext;
+pub(crate) mod py_result_ext;
 
 /// Old module which contained some implementation details of the `#[pyproto]` module.
 ///

src/prelude.rs

@@ -26,3 +26,4 @@ pub use crate::wrap_pyfunction;
 // Expected to become public API in 0.21
 // pub(crate) use crate::instance::Py2; // Will be stabilized with a different name
 // pub(crate) use crate::types::any::PyAnyMethods;
+// pub(crate) use crate::types::sequence::PySequenceMethods;

src/py_result_ext.rs

@@ -0,0 +1,22 @@
+use crate::{types::any::PyAnyMethods, Py2, PyAny, PyResult};
+
+mod sealed {
+    use super::*;
+
+    pub trait Sealed {}
+
+    impl Sealed for PyResult<Py2<'_, PyAny>> {}
+}
+
+use sealed::Sealed;
+
+pub(crate) trait PyResultExt<'py>: Sealed {
+    unsafe fn downcast_into_unchecked<T>(self) -> PyResult<Py2<'py, T>>;
+}
+
+impl<'py> PyResultExt<'py> for PyResult<Py2<'py, PyAny>> {
+    #[inline]
+    unsafe fn downcast_into_unchecked<T>(self) -> PyResult<Py2<'py, T>> {
+        self.map(|instance| instance.downcast_into_unchecked())
+    }
+}

src/types/any.rs

@@ -2,7 +2,9 @@ use crate::class::basic::CompareOp;
 use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, ToPyObject};
 use crate::err::{PyDowncastError, PyErr, PyResult};
 use crate::exceptions::{PyAttributeError, PyTypeError};
+use crate::ffi_ptr_ext::FfiPtrExt;
 use crate::instance::Py2;
+use crate::py_result_ext::PyResultExt;
 use crate::type_object::{HasPyGilRef, PyTypeCheck, PyTypeInfo};
 #[cfg(not(PyPy))]
 use crate::types::PySuper;
@@ -1719,10 +1721,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             attr_name: Py2<'_, PyString>,
         ) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
+                ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr())
-                    any.py(),
+                    .assume_owned_or_err(any.py())
-                    ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr()),
-                )
             }
         }
 
@@ -1772,12 +1772,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
         O: ToPyObject,
     {
         fn inner(any: &Py2<'_, PyAny>, other: Py2<'_, PyAny>) -> PyResult<Ordering> {
-            let py = any.py();
             let other = other.as_ptr();
             // Almost the same as ffi::PyObject_RichCompareBool, but this one doesn't try self == other.
             // See https://github.com/PyO3/pyo3/issues/985 for more.
             let do_compare = |other, op| unsafe {
-                Py2::from_owned_ptr_or_err(py, ffi::PyObject_RichCompare(any.as_ptr(), other, op))
+                ffi::PyObject_RichCompare(any.as_ptr(), other, op)
+                    .assume_owned_or_err(any.py())
                     .and_then(|obj| obj.is_true())
             };
             if do_compare(other, ffi::Py_EQ)? {
@@ -1807,10 +1807,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             compare_op: CompareOp,
         ) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
+                ffi::PyObject_RichCompare(any.as_ptr(), other.as_ptr(), compare_op as c_int)
-                    any.py(),
+                    .assume_owned_or_err(any.py())
-                    ffi::PyObject_RichCompare(any.as_ptr(), other.as_ptr(), compare_op as c_int),
-                )
             }
         }
 
@@ -1881,14 +1879,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             kwargs: Option<&PyDict>,
         ) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
+                ffi::PyObject_Call(
-                    any.py(),
+                    any.as_ptr(),
-                    ffi::PyObject_Call(
+                    args.as_ptr(),
-                        any.as_ptr(),
+                    kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()),
-                        args.as_ptr(),
-                        kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()),
-                    ),
                 )
+                .assume_owned_or_err(any.py())
             }
         }
 
@@ -1904,7 +1900,7 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             ))] {
                 // Optimized path on python 3.9+
                 unsafe {
-                    Py2::from_owned_ptr_or_err(self.py(), ffi::PyObject_CallNoArgs(self.as_ptr()))
+                    ffi::PyObject_CallNoArgs(self.as_ptr()).assume_owned_or_err(self.py())
                 }
             } else {
                 self.call((), None)
@@ -1941,7 +1937,7 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
                 // Optimized path on python 3.9+
                 unsafe {
                     let name = name.into_py(py).attach_into(py);
-                    Py2::from_owned_ptr_or_err(py, ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr()))
+                    ffi::PyObject_CallMethodNoArgs(self.as_ptr(), name.as_ptr()).assume_owned_or_err(py)
                 }
             } else {
                 self.call_method(name, (), None)
@@ -1982,10 +1978,7 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
     {
         fn inner<'py>(any: &Py2<'py, PyAny>, key: Py2<'_, PyAny>) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
+                ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr()).assume_owned_or_err(any.py())
-                    any.py(),
-                    ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr()),
-                )
             }
         }
 
@@ -2114,15 +2107,17 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
 
     fn repr(&self) -> PyResult<Py2<'py, PyString>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(self.py(), ffi::PyObject_Repr(self.as_ptr()))
+            ffi::PyObject_Repr(self.as_ptr())
-                .map(|any| any.downcast_into_unchecked())
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
         }
     }
 
     fn str(&self) -> PyResult<Py2<'py, PyString>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(self.py(), ffi::PyObject_Str(self.as_ptr()))
+            ffi::PyObject_Str(self.as_ptr())
-                .map(|any| any.downcast_into_unchecked())
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
         }
     }
 
@@ -2140,7 +2135,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
 
     fn dir(&self) -> Py2<'py, PyList> {
         unsafe {
-            Py2::from_owned_ptr(self.py(), ffi::PyObject_Dir(self.as_ptr()))
+            ffi::PyObject_Dir(self.as_ptr())
+                .assume_owned(self.py())
                 .downcast_into_unchecked()
         }
     }

src/types/iterator.rs

@@ -1,9 +1,9 @@
+use crate::ffi_ptr_ext::FfiPtrExt;
+use crate::py_result_ext::PyResultExt;
 use crate::{
     ffi, AsPyPointer, Py2, PyAny, PyDowncastError, PyErr, PyNativeType, PyResult, PyTypeCheck,
 };
 
-use super::any::PyAnyMethods;
-
 /// A Python iterator object.
 ///
 /// # Examples
@@ -39,8 +39,9 @@ impl PyIterator {
 
     pub(crate) fn from_object2<'py>(obj: &Py2<'py, PyAny>) -> PyResult<Py2<'py, PyIterator>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(obj.py(), ffi::PyObject_GetIter(obj.as_ptr()))
+            ffi::PyObject_GetIter(obj.as_ptr())
-                .map(|any| any.downcast_into_unchecked())
+                .assume_owned_or_err(obj.py())
+                .downcast_into_unchecked()
         }
     }
 }

src/types/mod.rs

@@ -296,7 +296,7 @@ mod notimplemented;
 mod num;
 #[cfg(not(PyPy))]
 mod pysuper;
-mod sequence;
+pub(crate) mod sequence;
 pub(crate) mod set;
 mod slice;
 mod string;

src/types/sequence.rs

@@ -1,12 +1,15 @@
 use crate::err::{self, PyDowncastError, PyErr, PyResult};
 use crate::exceptions::PyTypeError;
+use crate::ffi_ptr_ext::FfiPtrExt;
 #[cfg(feature = "experimental-inspect")]
 use crate::inspect::types::TypeInfo;
+use crate::instance::Py2;
 use crate::internal_tricks::get_ssize_index;
+use crate::py_result_ext::PyResultExt;
 use crate::sync::GILOnceCell;
 use crate::type_object::PyTypeInfo;
 use crate::types::{PyAny, PyList, PyString, PyTuple, PyType};
-use crate::{ffi, FromPyObject, Py, PyNativeType, PyObject, PyTypeCheck, Python, ToPyObject};
+use crate::{ffi, FromPyObject, Py, PyNativeType, PyTypeCheck, Python, ToPyObject};
 
 /// Represents a reference to a Python object supporting the sequence protocol.
 #[repr(transparent)]
@@ -20,15 +23,13 @@ impl PySequence {
     /// This is equivalent to the Python expression `len(self)`.
     #[inline]
     pub fn len(&self) -> PyResult<usize> {
-        let v = unsafe { ffi::PySequence_Size(self.as_ptr()) };
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).len()
-        crate::err::error_on_minusone(self.py(), v)?;
-        Ok(v as usize)
     }
 
     /// Returns whether the sequence is empty.
     #[inline]
     pub fn is_empty(&self) -> PyResult<bool> {
-        self.len().map(|l| l == 0)
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).is_empty()
     }
 
     /// Returns the concatenation of `self` and `other`.
@@ -36,10 +37,9 @@ impl PySequence {
     /// This is equivalent to the Python expression `self + other`.
     #[inline]
     pub fn concat(&self, other: &PySequence) -> PyResult<&PySequence> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py()
+            .concat(Py2::<PySequence>::borrowed_from_gil_ref(&other))
-                .from_owned_ptr_or_err(ffi::PySequence_Concat(self.as_ptr(), other.as_ptr()))
+            .map(Py2::into_gil_ref)
-        }
     }
 
     /// Returns the result of repeating a sequence object `count` times.
@@ -47,12 +47,9 @@ impl PySequence {
     /// This is equivalent to the Python expression `self * count`.
     #[inline]
     pub fn repeat(&self, count: usize) -> PyResult<&PySequence> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py().from_owned_ptr_or_err(ffi::PySequence_Repeat(
+            .repeat(count)
-                self.as_ptr(),
+            .map(Py2::into_gil_ref)
-                get_ssize_index(count),
-            ))
-        }
     }
 
     /// Concatenates `self` and `other`, in place if possible.
@@ -64,10 +61,9 @@ impl PySequence {
     /// possible, but create and return a new object if not.
     #[inline]
     pub fn in_place_concat(&self, other: &PySequence) -> PyResult<&PySequence> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py()
+            .in_place_concat(Py2::<PySequence>::borrowed_from_gil_ref(&other))
-                .from_owned_ptr_or_err(ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr()))
+            .map(Py2::into_gil_ref)
-        }
     }
 
     /// Repeats the sequence object `count` times and updates `self`, if possible.
@@ -79,13 +75,9 @@ impl PySequence {
     /// possible, but create and return a new object if not.
     #[inline]
     pub fn in_place_repeat(&self, count: usize) -> PyResult<&PySequence> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py()
+            .in_place_repeat(count)
-                .from_owned_ptr_or_err(ffi::PySequence_InPlaceRepeat(
+            .map(Py2::into_gil_ref)
-                    self.as_ptr(),
-                    get_ssize_index(count),
-                ))
-        }
     }
 
     /// Returns the `index`th element of the Sequence.
@@ -93,12 +85,9 @@ impl PySequence {
     /// This is equivalent to the Python expression `self[index]` without support of negative indices.
     #[inline]
     pub fn get_item(&self, index: usize) -> PyResult<&PyAny> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py().from_owned_ptr_or_err(ffi::PySequence_GetItem(
+            .get_item(index)
-                self.as_ptr(),
+            .map(|py2| py2.into_gil_ref())
-                get_ssize_index(index),
-            ))
-        }
     }
 
     /// Returns the slice of sequence object between `begin` and `end`.
@@ -106,13 +95,9 @@ impl PySequence {
     /// This is equivalent to the Python expression `self[begin:end]`.
     #[inline]
     pub fn get_slice(&self, begin: usize, end: usize) -> PyResult<&PySequence> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py().from_owned_ptr_or_err(ffi::PySequence_GetSlice(
+            .get_slice(begin, end)
-                self.as_ptr(),
+            .map(Py2::into_gil_ref)
-                get_ssize_index(begin),
-                get_ssize_index(end),
-            ))
-        }
     }
 
     /// Assigns object `item` to the `i`th element of self.
@@ -123,13 +108,7 @@ impl PySequence {
     where
         I: ToPyObject,
     {
-        fn inner(seq: &PySequence, i: usize, item: PyObject) -> PyResult<()> {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).set_item(i, item)
-            err::error_on_minusone(seq.py(), unsafe {
-                ffi::PySequence_SetItem(seq.as_ptr(), get_ssize_index(i), item.as_ptr())
-            })
-        }
-
-        inner(self, i, item.to_object(self.py()))
     }
 
     /// Deletes the `i`th element of self.
@@ -137,9 +116,7 @@ impl PySequence {
     /// This is equivalent to the Python statement `del self[i]`.
     #[inline]
     pub fn del_item(&self, i: usize) -> PyResult<()> {
-        err::error_on_minusone(self.py(), unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).del_item(i)
-            ffi::PySequence_DelItem(self.as_ptr(), get_ssize_index(i))
-        })
     }
 
     /// Assigns the sequence `v` to the slice of `self` from `i1` to `i2`.
@@ -147,14 +124,11 @@ impl PySequence {
     /// This is equivalent to the Python statement `self[i1:i2] = v`.
     #[inline]
     pub fn set_slice(&self, i1: usize, i2: usize, v: &PyAny) -> PyResult<()> {
-        err::error_on_minusone(self.py(), unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).set_slice(
-            ffi::PySequence_SetSlice(
+            i1,
-                self.as_ptr(),
+            i2,
-                get_ssize_index(i1),
+            Py2::borrowed_from_gil_ref(&v),
-                get_ssize_index(i2),
+        )
-                v.as_ptr(),
-            )
-        })
     }
 
     /// Deletes the slice from `i1` to `i2` from `self`.
@@ -162,9 +136,7 @@ impl PySequence {
     /// This is equivalent to the Python statement `del self[i1:i2]`.
     #[inline]
     pub fn del_slice(&self, i1: usize, i2: usize) -> PyResult<()> {
-        err::error_on_minusone(self.py(), unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).del_slice(i1, i2)
-            ffi::PySequence_DelSlice(self.as_ptr(), get_ssize_index(i1), get_ssize_index(i2))
-        })
     }
 
     /// Returns the number of occurrences of `value` in self, that is, return the
@@ -175,13 +147,7 @@ impl PySequence {
     where
         V: ToPyObject,
     {
-        fn inner(seq: &PySequence, value: PyObject) -> PyResult<usize> {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).count(value)
-            let r = unsafe { ffi::PySequence_Count(seq.as_ptr(), value.as_ptr()) };
-            crate::err::error_on_minusone(seq.py(), r)?;
-            Ok(r as usize)
-        }
-
-        inner(self, value.to_object(self.py()))
     }
 
     /// Determines if self contains `value`.
@@ -192,16 +158,7 @@ impl PySequence {
     where
         V: ToPyObject,
     {
-        fn inner(seq: &PySequence, value: PyObject) -> PyResult<bool> {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).contains(value)
-            let r = unsafe { ffi::PySequence_Contains(seq.as_ptr(), value.as_ptr()) };
-            match r {
-                0 => Ok(false),
-                1 => Ok(true),
-                _ => Err(PyErr::fetch(seq.py())),
-            }
-        }
-
-        inner(self, value.to_object(self.py()))
     }
 
     /// Returns the first index `i` for which `self[i] == value`.
@@ -212,31 +169,23 @@ impl PySequence {
     where
         V: ToPyObject,
     {
-        fn inner(seq: &PySequence, value: PyObject) -> PyResult<usize> {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self).index(value)
-            let r = unsafe { ffi::PySequence_Index(seq.as_ptr(), value.as_ptr()) };
-            crate::err::error_on_minusone(seq.py(), r)?;
-            Ok(r as usize)
-        }
-
-        inner(self, value.to_object(self.py()))
     }
 
     /// Returns a fresh list based on the Sequence.
     #[inline]
     pub fn to_list(&self) -> PyResult<&PyList> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py()
+            .to_list()
-                .from_owned_ptr_or_err(ffi::PySequence_List(self.as_ptr()))
+            .map(|py2| py2.into_gil_ref())
-        }
     }
 
     /// Returns a fresh tuple based on the Sequence.
     #[inline]
     pub fn to_tuple(&self) -> PyResult<&PyTuple> {
-        unsafe {
+        Py2::<PySequence>::borrowed_from_gil_ref(&self)
-            self.py()
+            .to_tuple()
-                .from_owned_ptr_or_err(ffi::PySequence_Tuple(self.as_ptr()))
+            .map(|py2| py2.into_gil_ref())
-        }
     }
 
     /// Register a pyclass as a subclass of `collections.abc.Sequence` (from the Python standard
@@ -249,6 +198,284 @@ impl PySequence {
     }
 }
 
+/// Implementation of functionality for [`PySequence`].
+///
+/// These methods are defined for the `Py2<'py, PySequence>` smart pointer, so to use method call
+/// syntax these methods are separated into a trait, because stable Rust does not yet support
+/// `arbitrary_self_types`.
+#[doc(alias = "PySequence")]
+pub(crate) trait PySequenceMethods<'py> {
+    /// Returns the number of objects in sequence.
+    ///
+    /// This is equivalent to the Python expression `len(self)`.
+    fn len(&self) -> PyResult<usize>;
+
+    /// Returns whether the sequence is empty.
+    fn is_empty(&self) -> PyResult<bool>;
+
+    /// Returns the concatenation of `self` and `other`.
+    ///
+    /// This is equivalent to the Python expression `self + other`.
+    fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>>;
+
+    /// Returns the result of repeating a sequence object `count` times.
+    ///
+    /// This is equivalent to the Python expression `self * count`.
+    fn repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>>;
+
+    /// Concatenates `self` and `other`, in place if possible.
+    ///
+    /// This is equivalent to the Python expression `self.__iadd__(other)`.
+    ///
+    /// The Python statement `self += other` is syntactic sugar for `self =
+    /// self.__iadd__(other)`.  `__iadd__` should modify and return `self` if
+    /// possible, but create and return a new object if not.
+    fn in_place_concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>>;
+
+    /// Repeats the sequence object `count` times and updates `self`, if possible.
+    ///
+    /// This is equivalent to the Python expression `self.__imul__(other)`.
+    ///
+    /// The Python statement `self *= other` is syntactic sugar for `self =
+    /// self.__imul__(other)`.  `__imul__` should modify and return `self` if
+    /// possible, but create and return a new object if not.
+    fn in_place_repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>>;
+
+    /// Returns the `index`th element of the Sequence.
+    ///
+    /// This is equivalent to the Python expression `self[index]` without support of negative indices.
+    fn get_item(&self, index: usize) -> PyResult<Py2<'py, PyAny>>;
+
+    /// Returns the slice of sequence object between `begin` and `end`.
+    ///
+    /// This is equivalent to the Python expression `self[begin:end]`.
+    fn get_slice(&self, begin: usize, end: usize) -> PyResult<Py2<'py, PySequence>>;
+
+    /// Assigns object `item` to the `i`th element of self.
+    ///
+    /// This is equivalent to the Python statement `self[i] = v`.
+    fn set_item<I>(&self, i: usize, item: I) -> PyResult<()>
+    where
+        I: ToPyObject;
+
+    /// Deletes the `i`th element of self.
+    ///
+    /// This is equivalent to the Python statement `del self[i]`.
+    fn del_item(&self, i: usize) -> PyResult<()>;
+
+    /// Assigns the sequence `v` to the slice of `self` from `i1` to `i2`.
+    ///
+    /// This is equivalent to the Python statement `self[i1:i2] = v`.
+    fn set_slice(&self, i1: usize, i2: usize, v: &Py2<'_, PyAny>) -> PyResult<()>;
+
+    /// Deletes the slice from `i1` to `i2` from `self`.
+    ///
+    /// This is equivalent to the Python statement `del self[i1:i2]`.
+    fn del_slice(&self, i1: usize, i2: usize) -> PyResult<()>;
+
+    /// Returns the number of occurrences of `value` in self, that is, return the
+    /// number of keys for which `self[key] == value`.
+    #[cfg(not(PyPy))]
+    fn count<V>(&self, value: V) -> PyResult<usize>
+    where
+        V: ToPyObject;
+
+    /// Determines if self contains `value`.
+    ///
+    /// This is equivalent to the Python expression `value in self`.
+    fn contains<V>(&self, value: V) -> PyResult<bool>
+    where
+        V: ToPyObject;
+
+    /// Returns the first index `i` for which `self[i] == value`.
+    ///
+    /// This is equivalent to the Python expression `self.index(value)`.
+    fn index<V>(&self, value: V) -> PyResult<usize>
+    where
+        V: ToPyObject;
+
+    /// Returns a fresh list based on the Sequence.
+    fn to_list(&self) -> PyResult<Py2<'py, PyList>>;
+
+    /// Returns a fresh tuple based on the Sequence.
+    fn to_tuple(&self) -> PyResult<Py2<'py, PyTuple>>;
+}
+
+impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> {
+    #[inline]
+    fn len(&self) -> PyResult<usize> {
+        let v = unsafe { ffi::PySequence_Size(self.as_ptr()) };
+        crate::err::error_on_minusone(self.py(), v)?;
+        Ok(v as usize)
+    }
+
+    #[inline]
+    fn is_empty(&self) -> PyResult<bool> {
+        self.len().map(|l| l == 0)
+    }
+
+    #[inline]
+    fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>> {
+        unsafe {
+            ffi::PySequence_Concat(self.as_ptr(), other.as_ptr())
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+
+    #[inline]
+    fn repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>> {
+        unsafe {
+            ffi::PySequence_Repeat(self.as_ptr(), get_ssize_index(count))
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+
+    #[inline]
+    fn in_place_concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>> {
+        unsafe {
+            ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr())
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+
+    #[inline]
+    fn in_place_repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>> {
+        unsafe {
+            ffi::PySequence_InPlaceRepeat(self.as_ptr(), get_ssize_index(count))
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+
+    #[inline]
+    fn get_item(&self, index: usize) -> PyResult<Py2<'py, PyAny>> {
+        unsafe {
+            ffi::PySequence_GetItem(self.as_ptr(), get_ssize_index(index))
+                .assume_owned_or_err(self.py())
+        }
+    }
+
+    #[inline]
+    fn get_slice(&self, begin: usize, end: usize) -> PyResult<Py2<'py, PySequence>> {
+        unsafe {
+            ffi::PySequence_GetSlice(self.as_ptr(), get_ssize_index(begin), get_ssize_index(end))
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+
+    #[inline]
+    fn set_item<I>(&self, i: usize, item: I) -> PyResult<()>
+    where
+        I: ToPyObject,
+    {
+        fn inner(seq: &Py2<'_, PySequence>, i: usize, item: Py2<'_, PyAny>) -> PyResult<()> {
+            err::error_on_minusone(seq.py(), unsafe {
+                ffi::PySequence_SetItem(seq.as_ptr(), get_ssize_index(i), item.as_ptr())
+            })
+        }
+
+        let py = self.py();
+        inner(self, i, item.to_object(py).attach_into(py))
+    }
+
+    #[inline]
+    fn del_item(&self, i: usize) -> PyResult<()> {
+        err::error_on_minusone(self.py(), unsafe {
+            ffi::PySequence_DelItem(self.as_ptr(), get_ssize_index(i))
+        })
+    }
+
+    #[inline]
+    fn set_slice(&self, i1: usize, i2: usize, v: &Py2<'_, PyAny>) -> PyResult<()> {
+        err::error_on_minusone(self.py(), unsafe {
+            ffi::PySequence_SetSlice(
+                self.as_ptr(),
+                get_ssize_index(i1),
+                get_ssize_index(i2),
+                v.as_ptr(),
+            )
+        })
+    }
+
+    #[inline]
+    fn del_slice(&self, i1: usize, i2: usize) -> PyResult<()> {
+        err::error_on_minusone(self.py(), unsafe {
+            ffi::PySequence_DelSlice(self.as_ptr(), get_ssize_index(i1), get_ssize_index(i2))
+        })
+    }
+
+    #[inline]
+    #[cfg(not(PyPy))]
+    fn count<V>(&self, value: V) -> PyResult<usize>
+    where
+        V: ToPyObject,
+    {
+        fn inner(seq: &Py2<'_, PySequence>, value: Py2<'_, PyAny>) -> PyResult<usize> {
+            let r = unsafe { ffi::PySequence_Count(seq.as_ptr(), value.as_ptr()) };
+            crate::err::error_on_minusone(seq.py(), r)?;
+            Ok(r as usize)
+        }
+
+        let py = self.py();
+        inner(self, value.to_object(py).attach_into(py))
+    }
+
+    #[inline]
+    fn contains<V>(&self, value: V) -> PyResult<bool>
+    where
+        V: ToPyObject,
+    {
+        fn inner(seq: &Py2<'_, PySequence>, value: Py2<'_, PyAny>) -> PyResult<bool> {
+            let r = unsafe { ffi::PySequence_Contains(seq.as_ptr(), value.as_ptr()) };
+            match r {
+                0 => Ok(false),
+                1 => Ok(true),
+                _ => Err(PyErr::fetch(seq.py())),
+            }
+        }
+
+        let py = self.py();
+        inner(self, value.to_object(py).attach_into(py))
+    }
+
+    #[inline]
+    fn index<V>(&self, value: V) -> PyResult<usize>
+    where
+        V: ToPyObject,
+    {
+        fn inner(seq: &Py2<'_, PySequence>, value: Py2<'_, PyAny>) -> PyResult<usize> {
+            let r = unsafe { ffi::PySequence_Index(seq.as_ptr(), value.as_ptr()) };
+            crate::err::error_on_minusone(seq.py(), r)?;
+            Ok(r as usize)
+        }
+
+        let py = self.py();
+        inner(self, value.to_object(self.py()).attach_into(py))
+    }
+
+    #[inline]
+    fn to_list(&self) -> PyResult<Py2<'py, PyList>> {
+        unsafe {
+            ffi::PySequence_List(self.as_ptr())
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+
+    #[inline]
+    fn to_tuple(&self) -> PyResult<Py2<'py, PyTuple>> {
+        unsafe {
+            ffi::PySequence_Tuple(self.as_ptr())
+                .assume_owned_or_err(self.py())
+                .downcast_into_unchecked()
+        }
+    }
+}
+
 #[inline]
 fn sequence_len(seq: &PySequence) -> usize {
     seq.len().expect("failed to get sequence length")