Update new_closure_bound closure signature (#3883)

* Update new_closure_bound closure signature

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>

* Use anonymous lifetimes in closure bounds

Co-authored-by: David Hewitt <mail@davidhewitt.dev>

* Take &Bound in PyCFunction closures

---------

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>
Co-authored-by: David Hewitt <mail@davidhewitt.dev>
This commit is contained in:
Lily Foote 2024-02-23 14:07:54 +00:00 committed by GitHub
parent fbf2e91914
commit e145ae851a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 35 additions and 23 deletions

View File

@ -101,7 +101,10 @@ impl PyCFunction {
F: Fn(&PyTuple, Option<&PyDict>) -> R + Send + 'static, F: Fn(&PyTuple, Option<&PyDict>) -> R + Send + 'static,
R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>,
{ {
Self::new_closure_bound(py, name, doc, closure).map(Bound::into_gil_ref) Self::new_closure_bound(py, name, doc, move |args, kwargs| {
closure(args.as_gil_ref(), kwargs.map(Bound::as_gil_ref))
})
.map(Bound::into_gil_ref)
} }
/// Create a new function from a closure. /// Create a new function from a closure.
@ -113,7 +116,7 @@ impl PyCFunction {
/// # use pyo3::{py_run, types::{PyCFunction, PyDict, PyTuple}}; /// # use pyo3::{py_run, types::{PyCFunction, PyDict, PyTuple}};
/// ///
/// Python::with_gil(|py| { /// Python::with_gil(|py| {
/// let add_one = |args: &PyTuple, _kwargs: Option<&PyDict>| -> PyResult<_> { /// let add_one = |args: &Bound<'_, PyTuple>, _kwargs: Option<&Bound<'_, PyDict>>| -> PyResult<_> {
/// let i = args.extract::<(i64,)>()?.0; /// let i = args.extract::<(i64,)>()?.0;
/// Ok(i+1) /// Ok(i+1)
/// }; /// };
@ -121,14 +124,14 @@ impl PyCFunction {
/// py_run!(py, add_one, "assert add_one(42) == 43"); /// py_run!(py, add_one, "assert add_one(42) == 43");
/// }); /// });
/// ``` /// ```
pub fn new_closure_bound<'a, F, R>( pub fn new_closure_bound<'py, F, R>(
py: Python<'a>, py: Python<'py>,
name: Option<&'static str>, name: Option<&'static str>,
doc: Option<&'static str>, doc: Option<&'static str>,
closure: F, closure: F,
) -> PyResult<Bound<'a, Self>> ) -> PyResult<Bound<'py, Self>>
where where
F: Fn(&PyTuple, Option<&PyDict>) -> R + Send + 'static, F: Fn(&Bound<'_, PyTuple>, Option<&Bound<'_, PyDict>>) -> R + Send + 'static,
R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>,
{ {
let method_def = pymethods::PyMethodDef::cfunction_with_keywords( let method_def = pymethods::PyMethodDef::cfunction_with_keywords(
@ -199,9 +202,11 @@ unsafe extern "C" fn run_closure<F, R>(
kwargs: *mut ffi::PyObject, kwargs: *mut ffi::PyObject,
) -> *mut ffi::PyObject ) -> *mut ffi::PyObject
where where
F: Fn(&PyTuple, Option<&PyDict>) -> R + Send + 'static, F: Fn(&Bound<'_, PyTuple>, Option<&Bound<'_, PyDict>>) -> R + Send + 'static,
R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>,
{ {
use crate::types::any::PyAnyMethods;
crate::impl_::trampoline::cfunction_with_keywords( crate::impl_::trampoline::cfunction_with_keywords(
capsule_ptr, capsule_ptr,
args, args,
@ -210,9 +215,12 @@ where
let boxed_fn: &ClosureDestructor<F> = let boxed_fn: &ClosureDestructor<F> =
&*(ffi::PyCapsule_GetPointer(capsule_ptr, closure_capsule_name().as_ptr()) &*(ffi::PyCapsule_GetPointer(capsule_ptr, closure_capsule_name().as_ptr())
as *mut ClosureDestructor<F>); as *mut ClosureDestructor<F>);
let args = py.from_borrowed_ptr::<PyTuple>(args); let args = Bound::ref_from_ptr(py, &args).downcast_unchecked::<PyTuple>();
let kwargs = py.from_borrowed_ptr_or_opt::<PyDict>(kwargs); let kwargs = Bound::ref_from_ptr_or_opt(py, &kwargs)
crate::callback::convert(py, (boxed_fn.closure)(args, kwargs)) .as_ref()
.map(|b| b.downcast_unchecked::<PyDict>());
let result = (boxed_fn.closure)(args, kwargs);
crate::callback::convert(py, result)
}, },
) )
} }

View File

@ -401,7 +401,9 @@ fn test_pycfunction_new_with_keywords() {
#[test] #[test]
fn test_closure() { fn test_closure() {
Python::with_gil(|py| { Python::with_gil(|py| {
let f = |args: &types::PyTuple, _kwargs: Option<&types::PyDict>| -> PyResult<_> { let f = |args: &Bound<'_, types::PyTuple>,
_kwargs: Option<&Bound<'_, types::PyDict>>|
-> PyResult<_> {
Python::with_gil(|py| { Python::with_gil(|py| {
let res: Vec<_> = args let res: Vec<_> = args
.iter() .iter()
@ -439,8 +441,9 @@ fn test_closure() {
fn test_closure_counter() { fn test_closure_counter() {
Python::with_gil(|py| { Python::with_gil(|py| {
let counter = std::cell::RefCell::new(0); let counter = std::cell::RefCell::new(0);
let counter_fn = let counter_fn = move |_args: &Bound<'_, types::PyTuple>,
move |_args: &types::PyTuple, _kwargs: Option<&types::PyDict>| -> PyResult<i32> { _kwargs: Option<&Bound<'_, types::PyDict>>|
-> PyResult<i32> {
let mut counter = counter.borrow_mut(); let mut counter = counter.borrow_mut();
*counter += 1; *counter += 1;
Ok(*counter) Ok(*counter)

View File

@ -6,7 +6,8 @@ fn main() {
let local_data = vec![0, 1, 2, 3, 4]; let local_data = vec![0, 1, 2, 3, 4];
let ref_: &[u8] = &local_data; let ref_: &[u8] = &local_data;
let closure_fn = |_args: &PyTuple, _kwargs: Option<&PyDict>| -> PyResult<()> { let closure_fn =
|_args: &Bound<'_, PyTuple>, _kwargs: Option<&Bound<'_, PyDict>>| -> PyResult<()> {
println!("This is five: {:?}", ref_.len()); println!("This is five: {:?}", ref_.len());
Ok(()) Ok(())
}; };

View File

@ -6,8 +6,8 @@ error[E0597]: `local_data` does not live long enough
7 | let ref_: &[u8] = &local_data; 7 | let ref_: &[u8] = &local_data;
| ^^^^^^^^^^^ borrowed value does not live long enough | ^^^^^^^^^^^ borrowed value does not live long enough
... ...
13 | PyCFunction::new_closure_bound(py, None, None, closure_fn) 14 | PyCFunction::new_closure_bound(py, None, None, closure_fn)
| ---------------------------------------------------------- argument requires that `local_data` is borrowed for `'static` | ---------------------------------------------------------- argument requires that `local_data` is borrowed for `'static`
... ...
16 | }); 17 | });
| - `local_data` dropped here while still borrowed | - `local_data` dropped here while still borrowed