From 37a5f6a94e9dab31575b016a4295fb94322b9aba Mon Sep 17 00:00:00 2001 From: Cheuk Ting Ho Date: Wed, 5 Jun 2024 22:21:44 +0100 Subject: [PATCH] remove internal APIs from pyo3-ffi (#4201) * remove internal APIs from pyo3-ffi * fix formating * add conditional import * remove _Py_c_neg/abs/pow * fix formating * adding changelog * expose PyAnyMethods::neg/pos/abs and use them * Update src/types/any.rs Co-authored-by: David Hewitt * Update src/types/any.rs Co-authored-by: David Hewitt * Adding details to changelog * update docs * remove PyREsultExt import for GraalPy --------- Co-authored-by: David Hewitt --- newsfragments/4201.changed.md | 1 + src/types/any.rs | 35 ++++++++++++++++++ src/types/complex.rs | 68 ++++++++++++++--------------------- 3 files changed, 62 insertions(+), 42 deletions(-) create mode 100644 newsfragments/4201.changed.md diff --git a/newsfragments/4201.changed.md b/newsfragments/4201.changed.md new file mode 100644 index 00000000..c236dd27 --- /dev/null +++ b/newsfragments/4201.changed.md @@ -0,0 +1 @@ +Remove CPython internal ffi call for complex number including: add, sub, mul, div, neg, abs, pow. Added PyAnyMethods::{abs, pos, neg} diff --git a/src/types/any.rs b/src/types/any.rs index ba5ea01b..85e540f9 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1127,6 +1127,21 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { where O: ToPyObject; + /// Computes the negative of self. + /// + /// Equivalent to the Python expression `-self`. + fn neg(&self) -> PyResult>; + + /// Computes the positive of self. + /// + /// Equivalent to the Python expression `+self`. + fn pos(&self) -> PyResult>; + + /// Computes the absolute of self. + /// + /// Equivalent to the Python expression `abs(self)`. + fn abs(&self) -> PyResult>; + /// Tests whether this object is less than another. /// /// This is equivalent to the Python expression `self < other`. @@ -1862,6 +1877,26 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { inner(self, other.to_object(py).into_bound(py), compare_op) } + fn neg(&self) -> PyResult> { + unsafe { ffi::PyNumber_Negative(self.as_ptr()).assume_owned_or_err(self.py()) } + } + + fn pos(&self) -> PyResult> { + fn inner<'py>(any: &Bound<'py, PyAny>) -> PyResult> { + unsafe { ffi::PyNumber_Positive(any.as_ptr()).assume_owned_or_err(any.py()) } + } + + inner(self) + } + + fn abs(&self) -> PyResult> { + fn inner<'py>(any: &Bound<'py, PyAny>) -> PyResult> { + unsafe { ffi::PyNumber_Absolute(any.as_ptr()).assume_owned_or_err(any.py()) } + } + + inner(self) + } + fn lt(&self, other: O) -> PyResult where O: ToPyObject, diff --git a/src/types/complex.rs b/src/types/complex.rs index 65b08cc9..5ec9e2f0 100644 --- a/src/types/complex.rs +++ b/src/types/complex.rs @@ -1,3 +1,5 @@ +#[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))] +use crate::py_result_ext::PyResultExt; #[cfg(feature = "gil-refs")] use crate::PyNativeType; use crate::{ffi, types::any::PyAnyMethods, Bound, PyAny, Python}; @@ -59,7 +61,6 @@ impl PyComplex { #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))] mod not_limited_impls { - use crate::ffi_ptr_ext::FfiPtrExt; use crate::Borrowed; use super::*; @@ -77,27 +78,17 @@ mod not_limited_impls { } } - #[inline(always)] - pub(super) unsafe fn complex_operation<'py>( - l: Borrowed<'_, 'py, PyComplex>, - r: Borrowed<'_, 'py, PyComplex>, - operation: unsafe extern "C" fn(ffi::Py_complex, ffi::Py_complex) -> ffi::Py_complex, - ) -> *mut ffi::PyObject { - let l_val = (*l.as_ptr().cast::()).cval; - let r_val = (*r.as_ptr().cast::()).cval; - ffi::PyComplex_FromCComplex(operation(l_val, r_val)) - } - macro_rules! bin_ops { - ($trait:ident, $fn:ident, $op:tt, $ffi:path) => { + ($trait:ident, $fn:ident, $op:tt) => { impl<'py> $trait for Borrowed<'_, 'py, PyComplex> { type Output = Bound<'py, PyComplex>; fn $fn(self, other: Self) -> Self::Output { - unsafe { - complex_operation(self, other, $ffi) - .assume_owned(self.py()) - .downcast_into_unchecked() - } + PyAnyMethods::$fn(self.as_any(), other) + .downcast_into().expect( + concat!("Complex method ", + stringify!($fn), + " failed.") + ) } } @@ -139,10 +130,10 @@ mod not_limited_impls { }; } - bin_ops!(Add, add, +, ffi::_Py_c_sum); - bin_ops!(Sub, sub, -, ffi::_Py_c_diff); - bin_ops!(Mul, mul, *, ffi::_Py_c_prod); - bin_ops!(Div, div, /, ffi::_Py_c_quot); + bin_ops!(Add, add, +); + bin_ops!(Sub, sub, -); + bin_ops!(Mul, mul, *); + bin_ops!(Div, div, /); #[cfg(feature = "gil-refs")] impl<'py> Neg for &'py PyComplex { @@ -155,12 +146,9 @@ mod not_limited_impls { impl<'py> Neg for Borrowed<'_, 'py, PyComplex> { type Output = Bound<'py, PyComplex>; fn neg(self) -> Self::Output { - unsafe { - let val = (*self.as_ptr().cast::()).cval; - ffi::PyComplex_FromCComplex(ffi::_Py_c_neg(val)) - .assume_owned(self.py()) - .downcast_into_unchecked() - } + PyAnyMethods::neg(self.as_any()) + .downcast_into() + .expect("Complex method __neg__ failed.") } } @@ -289,24 +277,20 @@ impl<'py> PyComplexMethods<'py> for Bound<'py, PyComplex> { #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))] fn abs(&self) -> c_double { - unsafe { - let val = (*self.as_ptr().cast::()).cval; - ffi::_Py_c_abs(val) - } + PyAnyMethods::abs(self.as_any()) + .downcast_into() + .expect("Complex method __abs__ failed.") + .extract() + .expect("Failed to extract to c double.") } #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))] fn pow(&self, other: &Bound<'py, PyComplex>) -> Bound<'py, PyComplex> { - use crate::ffi_ptr_ext::FfiPtrExt; - unsafe { - not_limited_impls::complex_operation( - self.as_borrowed(), - other.as_borrowed(), - ffi::_Py_c_pow, - ) - .assume_owned(self.py()) - .downcast_into_unchecked() - } + Python::with_gil(|py| { + PyAnyMethods::pow(self.as_any(), other, py.None()) + .downcast_into() + .expect("Complex method __pow__ failed.") + }) } }