Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove internal APIs from pyo3-ffi #4201

Merged
merged 12 commits into from
Jun 5, 2024
1 change: 1 addition & 0 deletions newsfragments/4201.changed.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add 4102.added.md please with something like "Added PyAnyMethods::{abs, pos, neg}."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

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)))]
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
Loading