From 21420cd5efd91bf34de89f24f08b5a7dfed4f378 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 25 Oct 2024 22:58:45 +0200 Subject: [PATCH] migrate `call` API to `IntoPyObject` --- src/conversion.rs | 136 +-------------------------------------------- src/instance.rs | 16 +++--- src/types/any.rs | 73 ++++++++++++------------ src/types/tuple.rs | 109 +----------------------------------- 4 files changed, 49 insertions(+), 285 deletions(-) diff --git a/src/conversion.rs b/src/conversion.rs index 4e0de44b4a1..dcf07a61eb3 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -1,11 +1,10 @@ //! Defines conversions between Rust and Python types. use crate::err::PyResult; -use crate::ffi_ptr_ext::FfiPtrExt; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::pyclass::boolean_struct::False; use crate::types::any::PyAnyMethods; -use crate::types::{PyDict, PyString, PyTuple}; +use crate::types::PyTuple; use crate::{ ffi, Borrowed, Bound, BoundObject, Py, PyAny, PyClass, PyErr, PyObject, PyRef, PyRefMut, Python, }; @@ -177,93 +176,6 @@ pub trait IntoPy: Sized { fn type_output() -> TypeInfo { TypeInfo::Any } - - // The following methods are helpers to use the vectorcall API where possible. - // They are overridden on tuples to perform a vectorcall. - // Be careful when you're implementing these: they can never refer to `Bound` call methods, - // as those refer to these methods, so this will create an infinite recursion. - #[doc(hidden)] - #[inline] - fn __py_call_vectorcall1<'py>( - self, - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - _: private::Token, - ) -> PyResult> - where - Self: IntoPy>, - { - #[inline] - fn inner<'py>( - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - args: Bound<'py, PyTuple>, - ) -> PyResult> { - unsafe { - ffi::PyObject_Call(function.as_ptr(), args.as_ptr(), std::ptr::null_mut()) - .assume_owned_or_err(py) - } - } - inner( - py, - function, - >>::into_py(self, py).into_bound(py), - ) - } - - #[doc(hidden)] - #[inline] - fn __py_call_vectorcall<'py>( - self, - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - kwargs: Option>, - _: private::Token, - ) -> PyResult> - where - Self: IntoPy>, - { - #[inline] - fn inner<'py>( - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - args: Bound<'py, PyTuple>, - kwargs: Option>, - ) -> PyResult> { - unsafe { - ffi::PyObject_Call( - function.as_ptr(), - args.as_ptr(), - kwargs.map_or_else(std::ptr::null_mut, |kwargs| kwargs.as_ptr()), - ) - .assume_owned_or_err(py) - } - } - inner( - py, - function, - >>::into_py(self, py).into_bound(py), - kwargs, - ) - } - - #[doc(hidden)] - #[inline] - fn __py_call_method_vectorcall1<'py>( - self, - _py: Python<'py>, - object: Borrowed<'_, 'py, PyAny>, - method_name: Borrowed<'_, 'py, PyString>, - _: private::Token, - ) -> PyResult> - where - Self: IntoPy>, - { - // Don't `self.into_py()`! This will lose the optimization of vectorcall. - object - .getattr(method_name) - .and_then(|method| method.call1(self)) - } } /// Defines a conversion from a Rust type to a Python object, which may fail. @@ -593,52 +505,6 @@ impl IntoPy> for () { fn into_py(self, py: Python<'_>) -> Py { PyTuple::empty(py).unbind() } - - #[inline] - fn __py_call_vectorcall1<'py>( - self, - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - _: private::Token, - ) -> PyResult> { - unsafe { ffi::compat::PyObject_CallNoArgs(function.as_ptr()).assume_owned_or_err(py) } - } - - #[inline] - fn __py_call_vectorcall<'py>( - self, - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - kwargs: Option>, - _: private::Token, - ) -> PyResult> { - unsafe { - match kwargs { - Some(kwargs) => ffi::PyObject_Call( - function.as_ptr(), - PyTuple::empty(py).as_ptr(), - kwargs.as_ptr(), - ) - .assume_owned_or_err(py), - None => ffi::compat::PyObject_CallNoArgs(function.as_ptr()).assume_owned_or_err(py), - } - } - } - - #[inline] - #[allow(clippy::used_underscore_binding)] - fn __py_call_method_vectorcall1<'py>( - self, - py: Python<'py>, - object: Borrowed<'_, 'py, PyAny>, - method_name: Borrowed<'_, 'py, PyString>, - _: private::Token, - ) -> PyResult> { - unsafe { - ffi::compat::PyObject_CallMethodNoArgs(object.as_ptr(), method_name.as_ptr()) - .assume_owned_or_err(py) - } - } } impl<'py> IntoPyObject<'py> for () { diff --git a/src/instance.rs b/src/instance.rs index a86bed94513..6b191abd5a2 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1495,7 +1495,7 @@ impl Py { kwargs: Option<&Bound<'py, PyDict>>, ) -> PyResult where - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { self.bind(py).as_any().call(args, kwargs).map(Bound::unbind) } @@ -1509,15 +1509,15 @@ impl Py { args: impl IntoPy>, kwargs: Option<&Bound<'_, PyDict>>, ) -> PyResult { - self.call(py, args, kwargs) + self.call(py, args.into_py(py), kwargs) } /// Calls the object with only positional arguments. /// /// This is equivalent to the Python expression `self(*args)`. - pub fn call1(&self, py: Python<'_>, args: N) -> PyResult + pub fn call1<'py, N>(&self, py: Python<'py>, args: N) -> PyResult where - N: IntoPy>, + N: IntoPyObject<'py, Target = PyTuple>, { self.bind(py).as_any().call1(args).map(Bound::unbind) } @@ -1540,11 +1540,11 @@ impl Py { py: Python<'py>, name: N, args: A, - kwargs: Option<&Bound<'_, PyDict>>, + kwargs: Option<&Bound<'py, PyDict>>, ) -> PyResult where N: IntoPyObject<'py, Target = PyString>, - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { self.bind(py) .as_any() @@ -1566,7 +1566,7 @@ impl Py { N: IntoPy>, A: IntoPy>, { - self.call_method(py, name.into_py(py), args, kwargs) + self.call_method(py, name.into_py(py), args.into_py(py), kwargs) } /// Calls a method on the object with only positional arguments. @@ -1578,7 +1578,7 @@ impl Py { pub fn call_method1<'py, N, A>(&self, py: Python<'py>, name: N, args: A) -> PyResult where N: IntoPyObject<'py, Target = PyString>, - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { self.bind(py) .as_any() diff --git a/src/types/any.rs b/src/types/any.rs index 63c38e11c51..62f7cdfc27e 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1,5 +1,5 @@ use crate::class::basic::CompareOp; -use crate::conversion::{private, AsPyPointer, FromPyObjectBound, IntoPy, IntoPyObject}; +use crate::conversion::{AsPyPointer, FromPyObjectBound, IntoPyObject}; use crate::err::{DowncastError, DowncastIntoError, PyErr, PyResult}; use crate::exceptions::{PyAttributeError, PyTypeError}; use crate::ffi_ptr_ext::FfiPtrExt; @@ -11,7 +11,7 @@ use crate::type_object::{PyTypeCheck, PyTypeInfo}; #[cfg(not(any(PyPy, GraalPy)))] use crate::types::PySuper; use crate::types::{PyDict, PyIterator, PyList, PyString, PyTuple, PyType}; -use crate::{err, ffi, Borrowed, BoundObject, Py, Python}; +use crate::{err, ffi, Borrowed, BoundObject, Python}; use std::cell::UnsafeCell; use std::cmp::Ordering; use std::os::raw::c_int; @@ -435,9 +435,9 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// }) /// # } /// ``` - fn call(&self, args: A, kwargs: Option<&Bound<'_, PyDict>>) -> PyResult> + fn call(&self, args: A, kwargs: Option<&Bound<'py, PyDict>>) -> PyResult> where - A: IntoPy>; + A: IntoPyObject<'py, Target = PyTuple>; /// Calls the object without arguments. /// @@ -492,7 +492,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// ``` fn call1(&self, args: A) -> PyResult> where - A: IntoPy>; + A: IntoPyObject<'py, Target = PyTuple>; /// Calls a method on the object. /// @@ -535,11 +535,11 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { &self, name: N, args: A, - kwargs: Option<&Bound<'_, PyDict>>, + kwargs: Option<&Bound<'py, PyDict>>, ) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPy>; + A: IntoPyObject<'py, Target = PyTuple>; /// Calls a method on the object without arguments. /// @@ -615,7 +615,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { fn call_method1(&self, name: N, args: A) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPy>; + A: IntoPyObject<'py, Target = PyTuple>; /// Returns whether the object is considered to be true. /// @@ -1245,15 +1245,30 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { unsafe { ffi::PyCallable_Check(self.as_ptr()) != 0 } } - fn call(&self, args: A, kwargs: Option<&Bound<'_, PyDict>>) -> PyResult> + fn call(&self, args: A, kwargs: Option<&Bound<'py, PyDict>>) -> PyResult> where - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { - args.__py_call_vectorcall( - self.py(), - self.as_borrowed(), - kwargs.map(Bound::as_borrowed), - private::Token, + fn inner<'py>( + any: &Bound<'py, PyAny>, + args: Borrowed<'_, 'py, PyTuple>, + kwargs: Option<&Bound<'py, PyDict>>, + ) -> PyResult> { + unsafe { + 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()) + } + } + + let py = self.py(); + inner( + self, + args.into_pyobject(py).map_err(Into::into)?.as_borrowed(), + kwargs, ) } @@ -1264,9 +1279,9 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { fn call1(&self, args: A) -> PyResult> where - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { - args.__py_call_vectorcall1(self.py(), self.as_borrowed(), private::Token) + self.call(args, None) } #[inline] @@ -1274,19 +1289,14 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { &self, name: N, args: A, - kwargs: Option<&Bound<'_, PyDict>>, + kwargs: Option<&Bound<'py, PyDict>>, ) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { - // Don't `args.into_py()`! This will lose the optimization of vectorcall. - match kwargs { - Some(_) => self - .getattr(name) - .and_then(|method| method.call(args, kwargs)), - None => self.call_method1(name, args), - } + self.getattr(name) + .and_then(|method| method.call(args, kwargs)) } #[inline] @@ -1305,16 +1315,9 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { fn call_method1(&self, name: N, args: A) -> PyResult> where N: IntoPyObject<'py, Target = PyString>, - A: IntoPy>, + A: IntoPyObject<'py, Target = PyTuple>, { - args.__py_call_method_vectorcall1( - self.py(), - self.as_borrowed(), - name.into_pyobject(self.py()) - .map_err(Into::into)? - .as_borrowed(), - private::Token, - ) + self.call_method(name, args, None) } fn is_truthy(&self) -> PyResult { diff --git a/src/types/tuple.rs b/src/types/tuple.rs index b2944741256..189cec126bd 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -1,15 +1,13 @@ use std::iter::FusedIterator; -use crate::conversion::{private, IntoPyObject}; +use crate::conversion::IntoPyObject; use crate::ffi::{self, Py_ssize_t}; use crate::ffi_ptr_ext::FfiPtrExt; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::instance::Borrowed; use crate::internal_tricks::get_ssize_index; -use crate::types::{ - any::PyAnyMethods, sequence::PySequenceMethods, PyDict, PyList, PySequence, PyString, -}; +use crate::types::{any::PyAnyMethods, sequence::PySequenceMethods, PyList, PySequence}; #[allow(deprecated)] use crate::ToPyObject; use crate::{ @@ -573,109 +571,6 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ fn type_output() -> TypeInfo { TypeInfo::Tuple(Some(vec![$( $T::type_output() ),+])) } - - #[inline] - fn __py_call_vectorcall1<'py>( - self, - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - _: private::Token, - ) -> PyResult> { - cfg_if::cfg_if! { - if #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] { - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_py(py).into_bound(py),)*]; - if $length == 1 { - unsafe { - ffi::PyObject_CallOneArg(function.as_ptr(), args_bound[0].as_ptr()).assume_owned_or_err(py) - } - } else { - // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. - let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; - unsafe { - ffi::PyObject_Vectorcall( - function.as_ptr(), - args.as_mut_ptr().add(1), - $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, - std::ptr::null_mut(), - ) - .assume_owned_or_err(py) - } - } - } else { - function.call1(>>::into_py(self, py).into_bound(py)) - } - } - } - - #[inline] - fn __py_call_vectorcall<'py>( - self, - py: Python<'py>, - function: Borrowed<'_, 'py, PyAny>, - kwargs: Option>, - _: private::Token, - ) -> PyResult> { - cfg_if::cfg_if! { - if #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] { - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_py(py).into_bound(py),)*]; - // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. - let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; - unsafe { - ffi::PyObject_VectorcallDict( - function.as_ptr(), - args.as_mut_ptr().add(1), - $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, - kwargs.map_or_else(std::ptr::null_mut, |kwargs| kwargs.as_ptr()), - ) - .assume_owned_or_err(py) - } - } else { - function.call(>>::into_py(self, py).into_bound(py), kwargs.as_deref()) - } - } - } - - #[inline] - fn __py_call_method_vectorcall1<'py>( - self, - py: Python<'py>, - object: Borrowed<'_, 'py, PyAny>, - method_name: Borrowed<'_, 'py, PyString>, - _: private::Token, - ) -> PyResult> { - cfg_if::cfg_if! { - if #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] { - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_py(py).into_bound(py),)*]; - if $length == 1 { - unsafe { - ffi::PyObject_CallMethodOneArg( - object.as_ptr(), - method_name.as_ptr(), - args_bound[0].as_ptr(), - ) - .assume_owned_or_err(py) - } - } else { - let mut args = [object.as_ptr(), $(args_bound[$n].as_ptr()),*]; - unsafe { - ffi::PyObject_VectorcallMethod( - method_name.as_ptr(), - args.as_mut_ptr(), - // +1 for the receiver. - 1 + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, - std::ptr::null_mut(), - ) - .assume_owned_or_err(py) - } - } - } else { - object.call_method1(method_name, >>::into_py(self, py).into_bound(py)) - } - } - } } impl<'py, $($T: FromPyObject<'py>),+> FromPyObject<'py> for ($($T,)+) {