Skip to content

Commit

Permalink
Support passing name and doc to PyCFunction::new_closure. Fixes PyO3#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dalcde committed Oct 29, 2022
1 parent 125af9b commit e84dd7f
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 11 deletions.
1 change: 1 addition & 0 deletions newsfragments/2686.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change PyCFunction::new_closure to take name and doc arguments.
13 changes: 9 additions & 4 deletions src/types/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,16 @@ impl PyCFunction {
/// let i = args.extract::<(i64,)>()?.0;
/// Ok(i+1)
/// };
/// let add_one = types::PyCFunction::new_closure(add_one, py).unwrap();
/// let add_one = types::PyCFunction::new_closure(py, None, None, add_one).unwrap();
/// py_run!(py, add_one, "assert add_one(42) == 43");
/// });
/// ```
pub fn new_closure<F, R>(f: F, py: Python<'_>) -> PyResult<&PyCFunction>
pub fn new_closure<'a, F, R>(
py: Python<'a>,
name: Option<&'static str>,
doc: Option<&'static str>,
f: F,
) -> PyResult<&'a PyCFunction>
where
F: Fn(&types::PyTuple, Option<&types::PyDict>) -> R + Send + 'static,
R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>,
Expand All @@ -123,9 +128,9 @@ impl PyCFunction {
)?
};
let method_def = pymethods::PyMethodDef::cfunction_with_keywords(
"pyo3-closure",
name.unwrap_or("pyo3-closure\0"),
pymethods::PyCFunctionWithKeywords(run_closure::<F, R>),
"",
doc.unwrap_or("\0"),
);
Self::internal_new_from_pointers(&method_def, py, capsule.as_ptr(), std::ptr::null_mut())
}
Expand Down
7 changes: 5 additions & 2 deletions tests/test_pyfunction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,11 @@ fn test_closure() {
Ok(res)
})
};
let closure_py = PyCFunction::new_closure(f, py).unwrap();
let closure_py = PyCFunction::new_closure(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'");
py_assert!(py, closure_py, "closure_py.__doc__ == 'test_fn doc'");
py_assert!(
py,
closure_py,
Expand All @@ -439,7 +441,8 @@ fn test_closure_counter() {
*counter += 1;
Ok(*counter)
};
let counter_py = PyCFunction::new_closure(counter_fn, py).unwrap();
let counter_py =
PyCFunction::new_closure(py, None, None, counter_fn).unwrap();

py_assert!(py, counter_py, "counter_py() == 1");
py_assert!(py, counter_py, "counter_py() == 2");
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/invalid_closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn main() {
println!("This is five: {:?}", ref_.len());
Ok(())
};
PyCFunction::new_closure(closure_fn, py).unwrap().into()
PyCFunction::new_closure(py, None, None, closure_fn).unwrap().into()
});

Python::with_gil(|py| {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/invalid_closure.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,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(closure_fn, py).unwrap().into()
| ---------------------------------------- argument requires that `local_data` is borrowed for `'static`
13 | PyCFunction::new_closure(py, None, None, closure_fn).unwrap().into()
| ---------------------------------------------------- argument requires that `local_data` is borrowed for `'static`
14 | });
| - `local_data` dropped here while still borrowed

Expand All @@ -20,8 +20,8 @@ error[E0373]: closure may outlive the current function, but it borrows `ref_`, w
note: function requires argument type to outlive `'static`
--> tests/ui/invalid_closure.rs:13:9
|
13 | PyCFunction::new_closure(closure_fn, py).unwrap().into()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13 | PyCFunction::new_closure(py, None, None, closure_fn).unwrap().into()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to force the closure to take ownership of `ref_` (and any other referenced variables), use the `move` keyword
|
9 | let closure_fn = move |_args: &PyTuple, _kwargs: Option<&PyDict>| -> PyResult<()> {
Expand Down

0 comments on commit e84dd7f

Please sign in to comment.