From aa3c938b5e4a1267b72c97f7f6d6e76b92455dd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20=C5=A0nuderl?= Date: Mon, 5 Feb 2024 08:51:18 +0100 Subject: [PATCH] PyCFunction bound api --- src/impl_/pyfunction.rs | 2 +- src/types/function.rs | 85 ++++++++++++++++++++++++++------- tests/test_pyfunction.rs | 8 ++-- tests/ui/invalid_closure.rs | 4 +- tests/ui/invalid_closure.stderr | 7 +-- 5 files changed, 81 insertions(+), 25 deletions(-) diff --git a/src/impl_/pyfunction.rs b/src/impl_/pyfunction.rs index 14cdbd48..464beeb8 100644 --- a/src/impl_/pyfunction.rs +++ b/src/impl_/pyfunction.rs @@ -6,5 +6,5 @@ pub fn _wrap_pyfunction<'a>( method_def: &PyMethodDef, py_or_module: impl Into>, ) -> PyResult<&'a PyCFunction> { - PyCFunction::internal_new(method_def, py_or_module.into()) + PyCFunction::internal_new(method_def, py_or_module.into()).map(|x| x.into_gil_ref()) } diff --git a/src/types/function.rs b/src/types/function.rs index c6f0f6b0..79f91cfe 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -1,6 +1,8 @@ use crate::derive_utils::PyFunctionArguments; +use crate::ffi_ptr_ext::FfiPtrExt; use crate::methods::PyMethodDefDestructor; use crate::prelude::*; +use crate::py_result_ext::PyResultExt; use crate::types::capsule::PyCapsuleMethods; use crate::{ ffi, @@ -17,13 +19,30 @@ pub struct PyCFunction(PyAny); pyobject_native_type_core!(PyCFunction, pyobject_native_static_type_object!(ffi::PyCFunction_Type), #checkfunction=ffi::PyCFunction_Check); impl PyCFunction { - /// Create a new built-in function with keywords (*args and/or **kwargs). + /// Deprecated form of [`PyCFunction::new_with_keywords_bound`] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyCFunction::new_with_keywords` will be replaced by `PyCFunction::new_with_keywords_bound` in a future PyO3 version" + ) + )] pub fn new_with_keywords<'a>( fun: ffi::PyCFunctionWithKeywords, name: &'static str, doc: &'static str, py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a Self> { + Self::new_with_keywords_bound(fun, name, doc, py_or_module).map(Bound::into_gil_ref) + } + + /// Create a new built-in function with keywords (*args and/or **kwargs). + pub fn new_with_keywords_bound<'a>( + fun: ffi::PyCFunctionWithKeywords, + name: &'static str, + doc: &'static str, + py_or_module: PyFunctionArguments<'a>, + ) -> PyResult> { Self::internal_new( &PyMethodDef::cfunction_with_keywords( name, @@ -34,19 +53,57 @@ impl PyCFunction { ) } - /// Create a new built-in function which takes no arguments. + /// Deprecated form of [`PyCFunction::new`] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyCFunction::new` will be replaced by `PyCFunction::new_bound` in a future PyO3 version" + ) + )] pub fn new<'a>( fun: ffi::PyCFunction, name: &'static str, doc: &'static str, py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a Self> { + Self::new_bound(fun, name, doc, py_or_module).map(Bound::into_gil_ref) + } + + /// Create a new built-in function which takes no arguments. + pub fn new_bound<'a>( + fun: ffi::PyCFunction, + name: &'static str, + doc: &'static str, + py_or_module: PyFunctionArguments<'a>, + ) -> PyResult> { Self::internal_new( &PyMethodDef::noargs(name, pymethods::PyCFunction(fun), doc), py_or_module, ) } + /// Deprecated form of [`PyCFunction::new_closure`] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyCFunction::new_closure` will be replaced by `PyCFunction::new_closure_bound` in a future PyO3 version" + ) + )] + pub fn new_closure<'a, F, R>( + py: Python<'a>, + name: Option<&'static str>, + doc: Option<&'static str>, + closure: F, + ) -> PyResult<&'a PyCFunction> + where + F: Fn(&PyTuple, Option<&PyDict>) -> R + Send + 'static, + R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, + { + Self::new_closure_bound(py, name, doc, closure).map(Bound::into_gil_ref) + } + /// Create a new function from a closure. /// /// # Examples @@ -60,16 +117,16 @@ impl PyCFunction { /// let i = args.extract::<(i64,)>()?.0; /// Ok(i+1) /// }; - /// let add_one = PyCFunction::new_closure(py, None, None, add_one).unwrap(); + /// let add_one = PyCFunction::new_closure_bound(py, None, None, add_one).unwrap(); /// py_run!(py, add_one, "assert add_one(42) == 43"); /// }); /// ``` - pub fn new_closure<'a, F, R>( + pub fn new_closure_bound<'a, F, R>( py: Python<'a>, name: Option<&'static str>, doc: Option<&'static str>, closure: F, - ) -> PyResult<&'a PyCFunction> + ) -> PyResult> where F: Fn(&PyTuple, Option<&PyDict>) -> R + Send + 'static, R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, @@ -95,11 +152,9 @@ impl PyCFunction { let data = unsafe { capsule.reference::>() }; unsafe { - py.from_owned_ptr_or_err::(ffi::PyCFunction_NewEx( - data.def.get(), - capsule.as_ptr(), - std::ptr::null_mut(), - )) + ffi::PyCFunction_NewEx(data.def.get(), capsule.as_ptr(), std::ptr::null_mut()) + .assume_owned_or_err(py) + .downcast_into_unchecked() } } @@ -107,7 +162,7 @@ impl PyCFunction { pub fn internal_new<'py>( method_def: &PyMethodDef, py_or_module: PyFunctionArguments<'py>, - ) -> PyResult<&'py Self> { + ) -> PyResult> { let (py, module) = py_or_module.into_py_and_maybe_module(); let (mod_ptr, module_name): (_, Option>) = if let Some(m) = module { let mod_ptr = m.as_ptr(); @@ -126,11 +181,9 @@ impl PyCFunction { .map_or(std::ptr::null_mut(), Py::as_ptr); unsafe { - py.from_owned_ptr_or_err::(ffi::PyCFunction_NewEx( - def, - mod_ptr, - module_name_ptr, - )) + ffi::PyCFunction_NewEx(def, mod_ptr, module_name_ptr) + .assume_owned_or_err(py) + .downcast_into_unchecked() } } } diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index f1116363..e06f7c2f 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -323,7 +323,7 @@ fn test_pycfunction_new() { ffi::PyLong_FromLong(4200) } - let py_fn = PyCFunction::new( + let py_fn = PyCFunction::new_bound( c_fn, "py_fn", "py_fn for test (this is the docstring)", @@ -380,7 +380,7 @@ fn test_pycfunction_new_with_keywords() { ffi::PyLong_FromLong(foo * bar) } - let py_fn = PyCFunction::new_with_keywords( + let py_fn = PyCFunction::new_with_keywords_bound( c_fn, "py_fn", "py_fn for test (this is the docstring)", @@ -422,7 +422,7 @@ fn test_closure() { }) }; let closure_py = - PyCFunction::new_closure(py, Some("test_fn"), Some("test_fn doc"), f).unwrap(); + PyCFunction::new_closure_bound(py, Some("test_fn"), Some("test_fn doc"), f).unwrap(); py_assert!(py, closure_py, "closure_py(42) == [43]"); py_assert!(py, closure_py, "closure_py.__name__ == 'test_fn'"); @@ -445,7 +445,7 @@ fn test_closure_counter() { *counter += 1; Ok(*counter) }; - let counter_py = PyCFunction::new_closure(py, None, None, counter_fn).unwrap(); + let counter_py = PyCFunction::new_closure_bound(py, None, None, counter_fn).unwrap(); py_assert!(py, counter_py, "counter_py() == 1"); py_assert!(py, counter_py, "counter_py() == 2"); diff --git a/tests/ui/invalid_closure.rs b/tests/ui/invalid_closure.rs index b724f31b..0613b027 100644 --- a/tests/ui/invalid_closure.rs +++ b/tests/ui/invalid_closure.rs @@ -10,7 +10,9 @@ fn main() { println!("This is five: {:?}", ref_.len()); Ok(()) }; - PyCFunction::new_closure(py, None, None, closure_fn).unwrap().into() + PyCFunction::new_closure_bound(py, None, None, closure_fn) + .unwrap() + .into() }); Python::with_gil(|py| { diff --git a/tests/ui/invalid_closure.stderr b/tests/ui/invalid_closure.stderr index 90240e5d..7fed8908 100644 --- a/tests/ui/invalid_closure.stderr +++ b/tests/ui/invalid_closure.stderr @@ -6,7 +6,8 @@ error[E0597]: `local_data` does not live long enough 7 | let ref_: &[u8] = &local_data; | ^^^^^^^^^^^ borrowed value does not live long enough ... -13 | PyCFunction::new_closure(py, None, None, closure_fn).unwrap().into() - | ---------------------------------------------------- argument requires that `local_data` is borrowed for `'static` -14 | }); +13 | PyCFunction::new_closure_bound(py, None, None, closure_fn) + | ---------------------------------------------------------- argument requires that `local_data` is borrowed for `'static` +... +16 | }); | - `local_data` dropped here while still borrowed