From 294bf62c1e043c0600af348fe6f0690fd3de6914 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sat, 11 Jan 2025 14:21:44 +0100 Subject: [PATCH] pyo3-ffi: expose `PyObject_Vectorcall(Method)` on the stable abi on 3.12+ --- newsfragments/4853.added.md | 1 + pyo3-ffi/src/abstract_.rs | 24 ++++++ pyo3-ffi/src/cpython/abstract_.rs | 30 ++----- src/instance.rs | 16 ++++ src/types/tuple.rs | 137 ++++++++++++++++-------------- 5 files changed, 119 insertions(+), 89 deletions(-) create mode 100644 newsfragments/4853.added.md diff --git a/newsfragments/4853.added.md b/newsfragments/4853.added.md new file mode 100644 index 00000000000..d3df4d219cc --- /dev/null +++ b/newsfragments/4853.added.md @@ -0,0 +1 @@ +pyo3-ffi: expose `PyObject_Vectorcall(Method)` on the stable abi on 3.12+ \ No newline at end of file diff --git a/pyo3-ffi/src/abstract_.rs b/pyo3-ffi/src/abstract_.rs index ce6c9b94735..47c5c2d1a0c 100644 --- a/pyo3-ffi/src/abstract_.rs +++ b/pyo3-ffi/src/abstract_.rs @@ -1,5 +1,7 @@ use crate::object::*; use crate::pyport::Py_ssize_t; +#[cfg(any(Py_3_12, all(Py_3_8, not(Py_LIMITED_API))))] +use libc::size_t; use std::os::raw::{c_char, c_int}; #[inline] @@ -70,6 +72,28 @@ extern "C" { method: *mut PyObject, ... ) -> *mut PyObject; +} +#[cfg(any(Py_3_12, all(Py_3_8, not(Py_LIMITED_API))))] +pub const PY_VECTORCALL_ARGUMENTS_OFFSET: size_t = + 1 << (8 * std::mem::size_of::() as size_t - 1); + +extern "C" { + #[cfg_attr(PyPy, link_name = "PyPyObject_Vectorcall")] + #[cfg(any(Py_3_12, all(Py_3_9, not(Py_LIMITED_API))))] + pub fn PyObject_Vectorcall( + callable: *mut PyObject, + args: *const *mut PyObject, + nargsf: size_t, + kwnames: *mut PyObject, + ) -> *mut PyObject; + + #[cfg(any(Py_3_12, all(Py_3_9, not(any(Py_LIMITED_API, PyPy, GraalPy)))))] + pub fn PyObject_VectorcallMethod( + name: *mut PyObject, + args: *const *mut PyObject, + nargsf: size_t, + kwnames: *mut PyObject, + ) -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyObject_Type")] pub fn PyObject_Type(o: *mut PyObject) -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyObject_Size")] diff --git a/pyo3-ffi/src/cpython/abstract_.rs b/pyo3-ffi/src/cpython/abstract_.rs index 477ad02b747..67977e99d38 100644 --- a/pyo3-ffi/src/cpython/abstract_.rs +++ b/pyo3-ffi/src/cpython/abstract_.rs @@ -41,8 +41,8 @@ extern "C" { ) -> *mut PyObject; } -#[cfg(Py_3_8)] -pub const PY_VECTORCALL_ARGUMENTS_OFFSET: size_t = +#[cfg(Py_3_8)] // NB exported as public in abstract.rs from 3.12 +const PY_VECTORCALL_ARGUMENTS_OFFSET: size_t = 1 << (8 * std::mem::size_of::() as size_t - 1); #[cfg(Py_3_8)] @@ -91,7 +91,7 @@ pub unsafe fn _PyObject_VectorcallTstate( } } -#[cfg(all(Py_3_8, not(any(PyPy, GraalPy))))] +#[cfg(all(Py_3_8, not(any(PyPy, GraalPy, Py_3_9))))] #[inline(always)] pub unsafe fn PyObject_Vectorcall( callable: *mut PyObject, @@ -103,16 +103,6 @@ pub unsafe fn PyObject_Vectorcall( } extern "C" { - #[cfg(all(PyPy, Py_3_8))] - #[cfg_attr(not(Py_3_9), link_name = "_PyPyObject_Vectorcall")] - #[cfg_attr(Py_3_9, link_name = "PyPyObject_Vectorcall")] - pub fn PyObject_Vectorcall( - callable: *mut PyObject, - args: *const *mut PyObject, - nargsf: size_t, - kwnames: *mut PyObject, - ) -> *mut PyObject; - #[cfg(Py_3_8)] #[cfg_attr( all(not(any(PyPy, GraalPy)), not(Py_3_9)), @@ -187,23 +177,13 @@ pub unsafe fn PyObject_CallOneArg(func: *mut PyObject, arg: *mut PyObject) -> *m _PyObject_VectorcallTstate(tstate, func, args, nargsf, std::ptr::null_mut()) } -extern "C" { - #[cfg(all(Py_3_9, not(any(PyPy, GraalPy))))] - pub fn PyObject_VectorcallMethod( - name: *mut PyObject, - args: *const *mut PyObject, - nargsf: size_t, - kwnames: *mut PyObject, - ) -> *mut PyObject; -} - #[cfg(all(Py_3_9, not(any(PyPy, GraalPy))))] #[inline(always)] pub unsafe fn PyObject_CallMethodNoArgs( self_: *mut PyObject, name: *mut PyObject, ) -> *mut PyObject { - PyObject_VectorcallMethod( + crate::PyObject_VectorcallMethod( name, &self_, 1 | PY_VECTORCALL_ARGUMENTS_OFFSET, @@ -220,7 +200,7 @@ pub unsafe fn PyObject_CallMethodOneArg( ) -> *mut PyObject { let args = [self_, arg]; assert!(!arg.is_null()); - PyObject_VectorcallMethod( + crate::PyObject_VectorcallMethod( name, args.as_ptr(), 2 | PY_VECTORCALL_ARGUMENTS_OFFSET, diff --git a/src/instance.rs b/src/instance.rs index 840416116f3..be27ff8b0d7 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -2020,6 +2020,22 @@ mod tests { }) } + #[test] + fn test_call_tuple_ref() { + let assert_repr = |obj: &Bound<'_, PyAny>, expected: &str| { + assert_eq!(obj.repr().unwrap(), expected); + }; + Python::with_gil(|py| { + let ty_obj = py.get_type::().into_pyobject(py).unwrap(); + #[allow(clippy::needless_borrows_for_generic_args)] + let obj = ty_obj.call1(&((('x', 1),),)).unwrap(); + assert_repr(&obj, "{'x': 1}"); + #[allow(clippy::needless_borrows_for_generic_args)] + let v = obj.call_method1("pop", &("x",)).unwrap(); + assert_eq!(v.extract::().unwrap(), 1); + }) + } + #[test] fn test_call_for_non_existing_method() { Python::with_gil(|py| { diff --git a/src/types/tuple.rs b/src/types/tuple.rs index a5de938140a..8147b872af5 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -594,7 +594,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ } } - #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] + #[cfg(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)))] fn call_positional( self, function: Borrowed<'_, 'py, PyAny>, @@ -603,30 +603,32 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ let py = function.py(); // We need this to drop the arguments correctly. let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + + #[cfg(not(Py_LIMITED_API))] if $length == 1 { - unsafe { + return 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) - } + }; + } + + // 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) } } - #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] + #[cfg(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)))] fn call_method_positional( self, object: Borrowed<'_, 'py, PyAny>, @@ -636,28 +638,31 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ let py = object.py(); // We need this to drop the arguments correctly. let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + + #[cfg(not(Py_LIMITED_API))] if $length == 1 { - unsafe { + return 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) - } + }; + } + + 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) } + } #[cfg(not(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API)))))] @@ -670,7 +675,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ self.into_pyobject_or_pyerr(function.py())?.call(function, kwargs, token) } - #[cfg(not(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API)))))] + #[cfg(not(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12))))] fn call_positional( self, function: Borrowed<'_, 'py, PyAny>, @@ -679,7 +684,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ self.into_pyobject_or_pyerr(function.py())?.call_positional(function, token) } - #[cfg(not(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API)))))] + #[cfg(not(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12))))] fn call_method_positional( self, object: Borrowed<'_, 'py, PyAny>, @@ -719,7 +724,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ } } - #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] + #[cfg(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)))] fn call_positional( self, function: Borrowed<'_, 'py, PyAny>, @@ -728,30 +733,32 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ let py = function.py(); // We need this to drop the arguments correctly. let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + + #[cfg(not(Py_LIMITED_API))] if $length == 1 { - unsafe { + return 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) - } + }; + } + + // 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) } } - #[cfg(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API))))] + #[cfg(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12)))] fn call_method_positional( self, object: Borrowed<'_, 'py, PyAny>, @@ -761,27 +768,29 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ let py = object.py(); // We need this to drop the arguments correctly. let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + + #[cfg(not(Py_LIMITED_API))] if $length == 1 { - unsafe { + return 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) - } + }; + } + + 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) } } @@ -795,7 +804,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ self.into_pyobject_or_pyerr(function.py())?.call(function, kwargs, token) } - #[cfg(not(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API)))))] + #[cfg(not(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12))))] fn call_positional( self, function: Borrowed<'_, 'py, PyAny>, @@ -804,7 +813,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ self.into_pyobject_or_pyerr(function.py())?.call_positional(function, token) } - #[cfg(not(all(Py_3_9, not(any(PyPy, GraalPy, Py_LIMITED_API)))))] + #[cfg(not(all(not(any(PyPy, GraalPy)), any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12))))] fn call_method_positional( self, object: Borrowed<'_, 'py, PyAny>,