Skip to content

Commit

Permalink
cleans up PyCFunction::internal_new (#3910)
Browse files Browse the repository at this point in the history 
This deduplicates some code around `PyCFunction::internal_new`
  • Loading branch information
@Icxolu
Icxolu authored Feb 27, 2024
1 parent a3ad28b commit 6f03a54
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 43 deletions.
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/pyfunction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ pub fn impl_wrap_pyfunction(
pub fn add_to_module(module: &#krate::Bound<'_, #krate::types::PyModule>) -> #krate::PyResult<()> {
use #krate::prelude::PyModuleMethods;
use ::std::convert::Into;
module.add_function(#krate::types::PyCFunction::internal_new(&DEF, module.as_gil_ref().into())?)
module.add_function(#krate::types::PyCFunction::internal_new(module.py(), &DEF, module.into())?)
}
}

Expand Down
17 changes: 9 additions & 8 deletions src/impl_/pyfunction.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
types::{PyCFunction, PyModule},
Borrowed, Bound, PyResult, Python,
Borrowed, Bound, PyNativeType, PyResult, Python,
};

pub use crate::impl_::pymethods::PyMethodDef;
Expand All @@ -13,39 +13,40 @@ pub trait WrapPyFunctionArg<'py, T> {

impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for Bound<'py, PyModule> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new_bound(self.py(), method_def, Some(&self))
PyCFunction::internal_new(self.py(), method_def, Some(&self))
}
}

impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for &'_ Bound<'py, PyModule> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new_bound(self.py(), method_def, Some(self))
PyCFunction::internal_new(self.py(), method_def, Some(self))
}
}

impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for Borrowed<'_, 'py, PyModule> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new_bound(self.py(), method_def, Some(&self))
PyCFunction::internal_new(self.py(), method_def, Some(&self))
}
}

impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for &'_ Borrowed<'_, 'py, PyModule> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new_bound(self.py(), method_def, Some(self))
PyCFunction::internal_new(self.py(), method_def, Some(self))
}
}

// For Python<'py>, only the GIL Ref form exists to avoid causing type inference to kick in.
// The `wrap_pyfunction_bound!` macro is needed for the Bound form.
impl<'py> WrapPyFunctionArg<'py, &'py PyCFunction> for Python<'py> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<&'py PyCFunction> {
PyCFunction::internal_new(method_def, self.into()).map(Bound::into_gil_ref)
PyCFunction::internal_new(self, method_def, None).map(Bound::into_gil_ref)
}
}

impl<'py> WrapPyFunctionArg<'py, &'py PyCFunction> for &'py PyModule {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<&'py PyCFunction> {
PyCFunction::internal_new(method_def, self.into()).map(Bound::into_gil_ref)
PyCFunction::internal_new(self.py(), method_def, Some(&self.as_borrowed()))
.map(Bound::into_gil_ref)
}
}

Expand All @@ -63,6 +64,6 @@ where

impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for OnlyBound<Python<'py>> {
fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<Bound<'py, PyCFunction>> {
PyCFunction::internal_new_bound(self.0, method_def, None)
PyCFunction::internal_new(self.0, method_def, None)
}
}
43 changes: 9 additions & 34 deletions src/types/function.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
impl_::pymethods::{self, PyMethodDef},
types::{PyCapsule, PyDict, PyModule, PyString, PyTuple},
};
use crate::{Bound, IntoPy, Py, PyAny, PyResult, Python};
use crate::{Bound, IntoPy, Py, PyAny, PyNativeType, PyResult, Python};
use std::cell::UnsafeCell;
use std::ffi::CStr;

Expand All @@ -34,13 +34,15 @@ impl PyCFunction {
doc: &'static str,
py_or_module: PyFunctionArguments<'a>,
) -> PyResult<&'a Self> {
let (py, module) = py_or_module.into_py_and_maybe_module();
Self::internal_new(
py,
&PyMethodDef::cfunction_with_keywords(
name,
pymethods::PyCFunctionWithKeywords(fun),
doc,
),
py_or_module,
module.map(PyNativeType::as_borrowed).as_deref(),
)
.map(Bound::into_gil_ref)
}
Expand All @@ -53,7 +55,7 @@ impl PyCFunction {
doc: &'static str,
module: Option<&Bound<'py, PyModule>>,
) -> PyResult<Bound<'py, Self>> {
Self::internal_new_bound(
Self::internal_new(
py,
&PyMethodDef::cfunction_with_keywords(
name,
Expand All @@ -78,9 +80,11 @@ impl PyCFunction {
doc: &'static str,
py_or_module: PyFunctionArguments<'a>,
) -> PyResult<&'a Self> {
let (py, module) = py_or_module.into_py_and_maybe_module();
Self::internal_new(
py,
&PyMethodDef::noargs(name, pymethods::PyCFunction(fun), doc),
py_or_module,
module.map(PyNativeType::as_borrowed).as_deref(),
)
.map(Bound::into_gil_ref)
}
Expand All @@ -93,7 +97,7 @@ impl PyCFunction {
doc: &'static str,
module: Option<&Bound<'py, PyModule>>,
) -> PyResult<Bound<'py, Self>> {
Self::internal_new_bound(
Self::internal_new(
py,
&PyMethodDef::noargs(name, pymethods::PyCFunction(fun), doc),
module,
Expand Down Expand Up @@ -180,35 +184,6 @@ impl PyCFunction {

#[doc(hidden)]
pub fn internal_new<'py>(
method_def: &PyMethodDef,
py_or_module: PyFunctionArguments<'py>,
) -> PyResult<Bound<'py, Self>> {
let (py, module) = py_or_module.into_py_and_maybe_module();
let (mod_ptr, module_name): (_, Option<Py<PyString>>) = if let Some(m) = module {
let mod_ptr = m.as_ptr();
(mod_ptr, Some(m.name()?.into_py(py)))
} else {
(std::ptr::null_mut(), None)
};
let (def, destructor) = method_def.as_method_def()?;

// FIXME: stop leaking the def and destructor
let def = Box::into_raw(Box::new(def));
std::mem::forget(destructor);

let module_name_ptr = module_name
.as_ref()
.map_or(std::ptr::null_mut(), Py::as_ptr);

unsafe {
ffi::PyCFunction_NewEx(def, mod_ptr, module_name_ptr)
.assume_owned_or_err(py)
.downcast_into_unchecked()
}
}

#[doc(hidden)]
pub(crate) fn internal_new_bound<'py>(
py: Python<'py>,
method_def: &PyMethodDef,
module: Option<&Bound<'py, PyModule>>,
Expand Down

0 comments on commit 6f03a54

Please sign in to comment.