From ac4ee2841b62da127a0e754b43cb5392f08f4460 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:19:53 +0100 Subject: [PATCH 1/2] implement `PySequenceMethods` --- src/instance.rs | 2 +- src/prelude.rs | 5 +- src/types/mod.rs | 2 +- src/types/sequence.rs | 413 +++++++++++++++++++++++++++++++++--------- 4 files changed, 331 insertions(+), 91 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index d80f6d519e1..c1a3e48e0d1 100644 --- a/src/instance.rs +++ b/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> diff --git a/src/prelude.rs b/src/prelude.rs index 1ef60bc659a..e1a4699a9f4 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -24,5 +24,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; diff --git a/src/types/mod.rs b/src/types/mod.rs index b20fdf37305..ef2390cc809 100644 --- a/src/types/mod.rs +++ b/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; diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 6515f6b4bf3..fe166592794 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -3,10 +3,11 @@ use crate::exceptions::PyTypeError; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::internal_tricks::get_ssize_index; +use crate::prelude::*; 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 +21,13 @@ impl PySequence { /// This is equivalent to the Python expression `len(self)`. #[inline] pub fn len(&self) -> PyResult { - let v = unsafe { ffi::PySequence_Size(self.as_ptr()) }; - crate::err::error_on_minusone(self.py(), v)?; - Ok(v as usize) + Py2::::borrowed_from_gil_ref(&self).len() } /// Returns whether the sequence is empty. #[inline] pub fn is_empty(&self) -> PyResult { - self.len().map(|l| l == 0) + Py2::::borrowed_from_gil_ref(&self).is_empty() } /// Returns the concatenation of `self` and `other`. @@ -36,10 +35,9 @@ impl PySequence { /// This is equivalent to the Python expression `self + other`. #[inline] pub fn concat(&self, other: &PySequence) -> PyResult<&PySequence> { - unsafe { - self.py() - .from_owned_ptr_or_err(ffi::PySequence_Concat(self.as_ptr(), other.as_ptr())) - } + Py2::::borrowed_from_gil_ref(&self) + .concat(Py2::::borrowed_from_gil_ref(&other)) + .map(Py2::into_gil_ref) } /// Returns the result of repeating a sequence object `count` times. @@ -47,12 +45,9 @@ impl PySequence { /// This is equivalent to the Python expression `self * count`. #[inline] pub fn repeat(&self, count: usize) -> PyResult<&PySequence> { - unsafe { - self.py().from_owned_ptr_or_err(ffi::PySequence_Repeat( - self.as_ptr(), - get_ssize_index(count), - )) - } + Py2::::borrowed_from_gil_ref(&self) + .repeat(count) + .map(Py2::into_gil_ref) } /// Concatenates `self` and `other`, in place if possible. @@ -64,10 +59,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 { - self.py() - .from_owned_ptr_or_err(ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr())) - } + Py2::::borrowed_from_gil_ref(&self) + .in_place_concat(Py2::::borrowed_from_gil_ref(&other)) + .map(Py2::into_gil_ref) } /// Repeats the sequence object `count` times and updates `self`, if possible. @@ -79,13 +73,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 { - self.py() - .from_owned_ptr_or_err(ffi::PySequence_InPlaceRepeat( - self.as_ptr(), - get_ssize_index(count), - )) - } + Py2::::borrowed_from_gil_ref(&self) + .in_place_repeat(count) + .map(Py2::into_gil_ref) } /// Returns the `index`th element of the Sequence. @@ -93,12 +83,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 { - self.py().from_owned_ptr_or_err(ffi::PySequence_GetItem( - self.as_ptr(), - get_ssize_index(index), - )) - } + Py2::::borrowed_from_gil_ref(&self) + .get_item(index) + .map(|py2| py2.into_gil_ref()) } /// Returns the slice of sequence object between `begin` and `end`. @@ -106,13 +93,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 { - self.py().from_owned_ptr_or_err(ffi::PySequence_GetSlice( - self.as_ptr(), - get_ssize_index(begin), - get_ssize_index(end), - )) - } + Py2::::borrowed_from_gil_ref(&self) + .get_slice(begin, end) + .map(Py2::into_gil_ref) } /// Assigns object `item` to the `i`th element of self. @@ -123,13 +106,7 @@ impl PySequence { where I: ToPyObject, { - fn inner(seq: &PySequence, i: usize, item: PyObject) -> PyResult<()> { - 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())) + Py2::::borrowed_from_gil_ref(&self).set_item(i, item) } /// Deletes the `i`th element of self. @@ -137,9 +114,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 { - ffi::PySequence_DelItem(self.as_ptr(), get_ssize_index(i)) - }) + Py2::::borrowed_from_gil_ref(&self).del_item(i) } /// Assigns the sequence `v` to the slice of `self` from `i1` to `i2`. @@ -147,6 +122,289 @@ 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<()> { + Py2::::borrowed_from_gil_ref(&self).set_slice( + i1, + i2, + Py2::borrowed_from_gil_ref(&v), + ) + } + + /// Deletes the slice from `i1` to `i2` from `self`. + /// + /// This is equivalent to the Python statement `del self[i1:i2]`. + #[inline] + pub fn del_slice(&self, i1: usize, i2: usize) -> PyResult<()> { + Py2::::borrowed_from_gil_ref(&self).del_slice(i1, i2) + } + + /// Returns the number of occurrences of `value` in self, that is, return the + /// number of keys for which `self[key] == value`. + #[inline] + #[cfg(not(PyPy))] + pub fn count(&self, value: V) -> PyResult + where + V: ToPyObject, + { + Py2::::borrowed_from_gil_ref(&self).count(value) + } + + /// Determines if self contains `value`. + /// + /// This is equivalent to the Python expression `value in self`. + #[inline] + pub fn contains(&self, value: V) -> PyResult + where + V: ToPyObject, + { + Py2::::borrowed_from_gil_ref(&self).contains(value) + } + + /// Returns the first index `i` for which `self[i] == value`. + /// + /// This is equivalent to the Python expression `self.index(value)`. + #[inline] + pub fn index(&self, value: V) -> PyResult + where + V: ToPyObject, + { + Py2::::borrowed_from_gil_ref(&self).index(value) + } + + /// Returns a fresh list based on the Sequence. + #[inline] + pub fn to_list(&self) -> PyResult<&PyList> { + Py2::::borrowed_from_gil_ref(&self) + .to_list() + .map(|py2| py2.into_gil_ref()) + } + + /// Returns a fresh tuple based on the Sequence. + #[inline] + pub fn to_tuple(&self) -> PyResult<&PyTuple> { + Py2::::borrowed_from_gil_ref(&self) + .to_tuple() + .map(|py2| py2.into_gil_ref()) + } + + /// Register a pyclass as a subclass of `collections.abc.Sequence` (from the Python standard + /// library). This is equvalent to `collections.abc.Sequence.register(T)` in Python. + /// This registration is required for a pyclass to be downcastable from `PyAny` to `PySequence`. + pub fn register(py: Python<'_>) -> PyResult<()> { + let ty = T::type_object(py); + get_sequence_abc(py)?.call_method1("register", (ty,))?; + Ok(()) + } +} + +/// 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; + + /// Returns whether the sequence is empty. + fn is_empty(&self) -> PyResult; + + /// Returns the concatenation of `self` and `other`. + /// + /// This is equivalent to the Python expression `self + other`. + fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult>; + + /// 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>; + + /// 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>; + + /// 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>; + + /// 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>; + + /// 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>; + + /// Assigns object `item` to the `i`th element of self. + /// + /// This is equivalent to the Python statement `self[i] = v`. + fn set_item(&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(&self, value: V) -> PyResult + where + V: ToPyObject; + + /// Determines if self contains `value`. + /// + /// This is equivalent to the Python expression `value in self`. + fn contains(&self, value: V) -> PyResult + 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(&self, value: V) -> PyResult + where + V: ToPyObject; + + /// Returns a fresh list based on the Sequence. + fn to_list(&self) -> PyResult>; + + /// Returns a fresh tuple based on the Sequence. + fn to_tuple(&self) -> PyResult>; +} + +impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> { + #[inline] + fn len(&self) -> PyResult { + 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 { + self.len().map(|l| l == 0) + } + + #[inline] + fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult> { + unsafe { + Py2::from_owned_ptr_or_err( + self.py(), + ffi::PySequence_Concat(self.as_ptr(), other.as_ptr()), + ) + .map(|py2| py2.downcast_into_unchecked()) + } + } + + #[inline] + fn repeat(&self, count: usize) -> PyResult> { + 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()) + } + } + + #[inline] + fn in_place_concat(&self, other: &Py2<'_, PySequence>) -> PyResult> { + unsafe { + Py2::from_owned_ptr_or_err( + self.py(), + ffi::PySequence_InPlaceConcat(self.as_ptr(), other.as_ptr()), + ) + .map(|py2| py2.downcast_into_unchecked()) + } + } + + #[inline] + fn in_place_repeat(&self, count: usize) -> PyResult> { + 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()) + } + } + + #[inline] + fn get_item(&self, index: usize) -> PyResult> { + unsafe { + Py2::from_owned_ptr_or_err( + self.py(), + ffi::PySequence_GetItem(self.as_ptr(), get_ssize_index(index)), + ) + } + } + + #[inline] + fn get_slice(&self, begin: usize, end: usize) -> PyResult> { + 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()) + } + } + + #[inline] + fn set_item(&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(), @@ -157,42 +415,35 @@ impl PySequence { }) } - /// Deletes the slice from `i1` to `i2` from `self`. - /// - /// This is equivalent to the Python statement `del self[i1:i2]`. #[inline] - pub fn del_slice(&self, i1: usize, i2: usize) -> PyResult<()> { + 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)) }) } - /// Returns the number of occurrences of `value` in self, that is, return the - /// number of keys for which `self[key] == value`. #[inline] #[cfg(not(PyPy))] - pub fn count(&self, value: V) -> PyResult + fn count(&self, value: V) -> PyResult where V: ToPyObject, { - fn inner(seq: &PySequence, value: PyObject) -> PyResult { + fn inner(seq: &Py2<'_, PySequence>, value: Py2<'_, PyAny>) -> PyResult { 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())) + let py = self.py(); + inner(self, value.to_object(py).attach_into(py)) } - /// Determines if self contains `value`. - /// - /// This is equivalent to the Python expression `value in self`. #[inline] - pub fn contains(&self, value: V) -> PyResult + fn contains(&self, value: V) -> PyResult where V: ToPyObject, { - fn inner(seq: &PySequence, value: PyObject) -> PyResult { + fn inner(seq: &Py2<'_, PySequence>, value: Py2<'_, PyAny>) -> PyResult { let r = unsafe { ffi::PySequence_Contains(seq.as_ptr(), value.as_ptr()) }; match r { 0 => Ok(false), @@ -201,52 +452,40 @@ impl PySequence { } } - inner(self, value.to_object(self.py())) + let py = self.py(); + inner(self, value.to_object(py).attach_into(py)) } - /// Returns the first index `i` for which `self[i] == value`. - /// - /// This is equivalent to the Python expression `self.index(value)`. #[inline] - pub fn index(&self, value: V) -> PyResult + fn index(&self, value: V) -> PyResult where V: ToPyObject, { - fn inner(seq: &PySequence, value: PyObject) -> PyResult { + fn inner(seq: &Py2<'_, PySequence>, value: Py2<'_, PyAny>) -> PyResult { 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())) + let py = self.py(); + inner(self, value.to_object(self.py()).attach_into(py)) } - /// Returns a fresh list based on the Sequence. #[inline] - pub fn to_list(&self) -> PyResult<&PyList> { + fn to_list(&self) -> PyResult> { unsafe { - self.py() - .from_owned_ptr_or_err(ffi::PySequence_List(self.as_ptr())) + Py2::from_owned_ptr_or_err(self.py(), ffi::PySequence_List(self.as_ptr())) + .map(|py2| py2.downcast_into_unchecked()) } } - /// Returns a fresh tuple based on the Sequence. #[inline] - pub fn to_tuple(&self) -> PyResult<&PyTuple> { + fn to_tuple(&self) -> PyResult> { unsafe { - self.py() - .from_owned_ptr_or_err(ffi::PySequence_Tuple(self.as_ptr())) + Py2::from_owned_ptr_or_err(self.py(), ffi::PySequence_Tuple(self.as_ptr())) + .map(|py2| py2.downcast_into_unchecked()) } } - - /// Register a pyclass as a subclass of `collections.abc.Sequence` (from the Python standard - /// library). This is equvalent to `collections.abc.Sequence.register(T)` in Python. - /// This registration is required for a pyclass to be downcastable from `PyAny` to `PySequence`. - pub fn register(py: Python<'_>) -> PyResult<()> { - let ty = T::type_object(py); - get_sequence_abc(py)?.call_method1("register", (ty,))?; - Ok(()) - } } #[inline] From 82ac801be40a2f3717a1503e7068942f2d0b9f5d Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 14 Dec 2023 10:51:18 +0000 Subject: [PATCH 2/2] introduce traits to make ffi ptr handling cleaner --- src/ffi_ptr_ext.rs | 28 +++++++++++++++++++ src/lib.rs | 2 ++ src/prelude.rs | 4 +-- src/py_result_ext.rs | 22 +++++++++++++++ src/types/any.rs | 52 ++++++++++++++++------------------- src/types/iterator.rs | 9 +++--- src/types/sequence.rs | 64 ++++++++++++++++++------------------------- 7 files changed, 109 insertions(+), 72 deletions(-) create mode 100644 src/ffi_ptr_ext.rs create mode 100644 src/py_result_ext.rs diff --git a/src/ffi_ptr_ext.rs b/src/ffi_ptr_ext.rs new file mode 100644 index 00000000000..34f95cd2bdc --- /dev/null +++ b/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>; + 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::from_owned_ptr_or_err(py, self) + } + + #[inline] + unsafe fn assume_owned(self, py: Python<'_>) -> Py2<'_, PyAny> { + Py2::from_owned_ptr(py, self) + } +} diff --git a/src/lib.rs b/src/lib.rs index fcf75610656..985ec0aa4ff 100644 --- a/src/lib.rs +++ b/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. /// diff --git a/src/prelude.rs b/src/prelude.rs index e1a4699a9f4..e6a3cb7fe72 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -24,6 +24,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; diff --git a/src/py_result_ext.rs b/src/py_result_ext.rs new file mode 100644 index 00000000000..0ded64cb1cd --- /dev/null +++ b/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> {} +} + +use sealed::Sealed; + +pub(crate) trait PyResultExt<'py>: Sealed { + unsafe fn downcast_into_unchecked(self) -> PyResult>; +} + +impl<'py> PyResultExt<'py> for PyResult> { + #[inline] + unsafe fn downcast_into_unchecked(self) -> PyResult> { + self.map(|instance| instance.downcast_into_unchecked()) + } +} diff --git a/src/types/any.rs b/src/types/any.rs index dc29b1dea56..6d6da10764d 100644 --- a/src/types/any.rs +++ b/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> { 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_err(any.py()) } } @@ -1772,12 +1772,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { O: ToPyObject, { fn inner(any: &Py2<'_, PyAny>, other: Py2<'_, PyAny>) -> PyResult { - 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> { 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_err(any.py()) } } @@ -1881,14 +1879,12 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { kwargs: Option<&PyDict>, ) -> PyResult> { 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_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> { 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_err(any.py()) } } @@ -2114,15 +2107,17 @@ impl<'py> PyAnyMethods<'py> for Py2<'py, PyAny> { fn repr(&self) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } fn str(&self) -> PyResult> { 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_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() } } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 310dbdf8204..13c4e4bab43 100644 --- a/src/types/iterator.rs +++ b/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> { 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_err(obj.py()) + .downcast_into_unchecked() } } } diff --git a/src/types/sequence.rs b/src/types/sequence.rs index fe166592794..0b0a4cfeaa5 100644 --- a/src/types/sequence.rs +++ b/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}; @@ -315,69 +317,53 @@ impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> { #[inline] fn concat(&self, other: &Py2<'_, PySequence>) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } #[inline] fn repeat(&self, count: usize) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } #[inline] fn in_place_concat(&self, other: &Py2<'_, PySequence>) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } #[inline] fn in_place_repeat(&self, count: usize) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } #[inline] fn get_item(&self, index: usize) -> PyResult> { 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_err(self.py()) } } #[inline] fn get_slice(&self, begin: usize, end: usize) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } @@ -474,16 +460,18 @@ impl<'py> PySequenceMethods<'py> for Py2<'py, PySequence> { #[inline] fn to_list(&self) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } #[inline] fn to_tuple(&self) -> PyResult> { 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_err(self.py()) + .downcast_into_unchecked() } } }