Skip to content

Commit

Permalink
remove internal APIs from pyo3-ffi (#4201)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* Update src/types/any.rs

Co-authored-by: David Hewitt <[email protected]>

* Adding details to changelog

* update docs

* remove PyREsultExt import for GraalPy

---------

Co-authored-by: David Hewitt <[email protected]>
  • Loading branch information
Cheukting and davidhewitt authored Jun 5, 2024
1 parent 93ef056 commit 37a5f6a
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 42 deletions.
1 change: 1 addition & 0 deletions newsfragments/4201.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove CPython internal ffi call for complex number including: add, sub, mul, div, neg, abs, pow. Added PyAnyMethods::{abs, pos, neg}
35 changes: 35 additions & 0 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Bound<'py, PyAny>>;

/// Computes the positive of self.
///
/// Equivalent to the Python expression `+self`.
fn pos(&self) -> PyResult<Bound<'py, PyAny>>;

/// Computes the absolute of self.
///
/// Equivalent to the Python expression `abs(self)`.
fn abs(&self) -> PyResult<Bound<'py, PyAny>>;

/// Tests whether this object is less than another.
///
/// This is equivalent to the Python expression `self < other`.
Expand Down Expand Up @@ -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<Bound<'py, PyAny>> {
unsafe { ffi::PyNumber_Negative(self.as_ptr()).assume_owned_or_err(self.py()) }
}

fn pos(&self) -> PyResult<Bound<'py, PyAny>> {
fn inner<'py>(any: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyAny>> {
unsafe { ffi::PyNumber_Positive(any.as_ptr()).assume_owned_or_err(any.py()) }
}

inner(self)
}

fn abs(&self) -> PyResult<Bound<'py, PyAny>> {
fn inner<'py>(any: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyAny>> {
unsafe { ffi::PyNumber_Absolute(any.as_ptr()).assume_owned_or_err(any.py()) }
}

inner(self)
}

fn lt<O>(&self, other: O) -> PyResult<bool>
where
O: ToPyObject,
Expand Down
68 changes: 26 additions & 42 deletions src/types/complex.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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::*;
Expand All @@ -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::<ffi::PyComplexObject>()).cval;
let r_val = (*r.as_ptr().cast::<ffi::PyComplexObject>()).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.")
)
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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::<ffi::PyComplexObject>()).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.")
}
}

Expand Down Expand Up @@ -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::<ffi::PyComplexObject>()).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.")
})
}
}

Expand Down

0 comments on commit 37a5f6a

Please sign in to comment.