introduce traits to make ffi ptr handling cleaner

src/ffi_ptr_ext.rs

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

src/lib.rs

@@ -317,6 +317,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

@@ -23,6 +23,6 @@ pub use pyo3_macros::{pyclass, pyfunction, pymethods, pymodule, FromPyObject};
 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::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 py_result_ext_sealed {
+    use super::*;
+
+    pub trait Sealed {}
+
+    impl Sealed for PyResult<Py2<'_, PyAny>> {}
+}
+
+use py_result_ext_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,6 +2,7 @@ use crate::class::basic::CompareOp;
 use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, PyTryFrom, ToPyObject};
 use crate::err::{PyDowncastError, PyErr, PyResult};
 use crate::exceptions::{PyAttributeError, PyTypeError};
+use crate::ffi_ptr_ext::FfiPtrExt;
 use crate::instance::Py2;
 use crate::type_object::PyTypeInfo;
 #[cfg(not(PyPy))]
@@ -12,6 +13,8 @@ use std::cell::UnsafeCell;
 use std::cmp::Ordering;
 use std::os::raw::c_int;
 
+use crate::py_result_ext::PyResultExt;
+
 /// Represents any Python object.
 ///
 /// It currently only appears as a *reference*, `&PyAny`,
@@ -1707,10 +1710,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             attr_name: Py2<'_, PyString>,
         ) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
-                    any.py(),
-                    ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr()),
-                )
+                ffi::PyObject_GetAttr(any.as_ptr(), attr_name.as_ptr())
+                    .assume_owned_or_ffi_error(any.py())
             }
         }
 
@@ -1760,12 +1761,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_ffi_error(any.py())
                     .and_then(|obj| obj.is_true())
             };
             if do_compare(other, ffi::Py_EQ)? {
@@ -1795,10 +1796,8 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             compare_op: CompareOp,
         ) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
-                    any.py(),
-                    ffi::PyObject_RichCompare(any.as_ptr(), other.as_ptr(), compare_op as c_int),
-                )
+                ffi::PyObject_RichCompare(any.as_ptr(), other.as_ptr(), compare_op as c_int)
+                    .assume_owned_or_ffi_error(any.py())
             }
         }
 
@@ -1869,14 +1868,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> {
             kwargs: Option<&PyDict>,
         ) -> PyResult<Py2<'py, PyAny>> {
             unsafe {
-                Py2::from_owned_ptr_or_err(
-                    any.py(),
-                    ffi::PyObject_Call(
-                        any.as_ptr(),
-                        args.as_ptr(),
-                        kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()),
-                    ),
+                ffi::PyObject_Call(
+                    any.as_ptr(),
+                    args.as_ptr(),
+                    kwargs.map_or(std::ptr::null_mut(), |dict| dict.as_ptr()),
                 )
+                .assume_owned_or_ffi_error(any.py())
             }
         }
 
@@ -1892,7 +1889,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_ffi_error(self.py())
                 }
             } else {
                 self.call((), None)
@@ -1929,7 +1926,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_ffi_error(py)
                 }
             } else {
                 self.call_method(name, (), None)
@@ -1970,10 +1967,8 @@ 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(
-                    any.py(),
-                    ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr()),
-                )
+                ffi::PyObject_GetItem(any.as_ptr(), key.as_ptr())
+                    .assume_owned_or_ffi_error(any.py())
             }
         }
 
@@ -2102,15 +2097,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()))
-                .map(|any| any.downcast_into_unchecked())
+            ffi::PyObject_Repr(self.as_ptr())
+                .assume_owned_or_ffi_error(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()))
-                .map(|any| any.downcast_into_unchecked())
+            ffi::PyObject_Str(self.as_ptr())
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
@@ -2128,7 +2125,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_and_infallible(self.py())
                 .downcast_into_unchecked()
         }
     }

src/types/iterator.rs

@@ -1,8 +1,8 @@
+use crate::ffi_ptr_ext::FfiPtrExt;
+use crate::py_result_ext::PyResultExt;
 use crate::{ffi, AsPyPointer, Py, Py2, PyAny, PyErr, PyNativeType, PyResult, Python};
 use crate::{PyDowncastError, PyTryFrom};
 
-use super::any::PyAnyMethods;
-
 /// A Python iterator object.
 ///
 /// # Examples
@@ -42,8 +42,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()))
-                .map(|any| any.downcast_into_unchecked())
+            ffi::PyObject_GetIter(obj.as_ptr())
+                .assume_owned_or_ffi_error(obj.py())
+                .downcast_into_unchecked()
         }
     }
 }

src/types/sequence.rs

@@ -1,9 +1,11 @@
 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::prelude::*;
+use crate::py_result_ext::PyResultExt;
 use crate::sync::GILOnceCell;
 use crate::type_object::PyTypeInfo;
 use crate::types::{PyAny, PyList, PyString, PyTuple, PyType};
@@ -341,69 +343,53 @@ impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> {
     #[inline]
     fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(
-                self.py(),
-                ffi::PySequence_Concat(self.as_ptr(), other.as_ptr()),
-            )
-            .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_Concat(self.as_ptr(), other.as_ptr())
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
     #[inline]
     fn repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(
-                self.py(),
-                ffi::PySequence_Repeat(self.as_ptr(), get_ssize_index(count)),
-            )
-            .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_Repeat(self.as_ptr(), get_ssize_index(count))
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
     #[inline]
     fn in_place_concat(&self, other: &Py2<'_, PySequence>) -> PyResult<Py2<'py, PySequence>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(
-                self.py(),
-                ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr()),
-            )
-            .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr())
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
     #[inline]
     fn in_place_repeat(&self, count: usize) -> PyResult<Py2<'py, PySequence>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(
-                self.py(),
-                ffi::PySequence_InPlaceRepeat(self.as_ptr(), get_ssize_index(count)),
-            )
-            .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_InPlaceRepeat(self.as_ptr(), get_ssize_index(count))
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
     #[inline]
     fn get_item(&self, index: usize) -> PyResult<Py2<'py, PyAny>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(
-                self.py(),
-                ffi::PySequence_GetItem(self.as_ptr(), get_ssize_index(index)),
-            )
+            ffi::PySequence_GetItem(self.as_ptr(), get_ssize_index(index))
+                .assume_owned_or_ffi_error(self.py())
         }
     }
 
     #[inline]
     fn get_slice(&self, begin: usize, end: usize) -> PyResult<Py2<'py, PySequence>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(
-                self.py(),
-                ffi::PySequence_GetSlice(
-                    self.as_ptr(),
-                    get_ssize_index(begin),
-                    get_ssize_index(end),
-                ),
-            )
-            .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_GetSlice(self.as_ptr(), get_ssize_index(begin), get_ssize_index(end))
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
@@ -500,16 +486,18 @@ impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> {
     #[inline]
     fn to_list(&self) -> PyResult<Py2<'py, PyList>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(self.py(), ffi::PySequence_List(self.as_ptr()))
-                .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_List(self.as_ptr())
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 
     #[inline]
     fn to_tuple(&self) -> PyResult<Py2<'py, PyTuple>> {
         unsafe {
-            Py2::from_owned_ptr_or_err(self.py(), ffi::PySequence_Tuple(self.as_ptr()))
-                .map(|py2| py2.downcast_into_unchecked())
+            ffi::PySequence_Tuple(self.as_ptr())
+                .assume_owned_or_ffi_error(self.py())
+                .downcast_into_unchecked()
         }
     }
 }