Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate &PyModule as #[pymodule] argument type #3936

Merged
merged 6 commits into from
Mar 8, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
deprecate &PyModule as #[pymodule] argument type
Icxolu committed Mar 6, 2024
commit 1cfba4d4cc88bc861de53f0c3c645f5c9eb7198d
25 changes: 23 additions & 2 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ use crate::{
pyfunction::{impl_wrap_pyfunction, PyFunctionOptions},
};
use proc_macro2::TokenStream;
use quote::quote;
use quote::{quote, quote_spanned};
use syn::{
ext::IdentExt,
parse::{Parse, ParseStream},
@@ -295,8 +295,29 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
}
module_args.push(quote!(::std::convert::Into::into(BoundRef(module))));

let syn::ItemFn {
attrs, sig, block, ..
} = &function;

let extractors = sig.inputs.iter().filter_map(|param| {
if let syn::FnArg::Typed(pat_type) = param {
if let syn::Pat::Ident(pat_ident) = &*pat_type.pat {
let ident = &pat_ident.ident;
return Some(quote_spanned! { pat_type.span() => {
let (_, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to assign the argument back out:

Suggested change
let (_, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident);
let (#ident, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident);

(I think this would matter for edge cases where the module is using Bound<'_, PyModule> or Py<PyModule> for some reason, and inspect_type would transfer ownership rather than copy.)

let _ = e.extract_gil_ref();
}});
}
}
None
});

Ok(quote! {
#function
#(#attrs)*
#vis #sig {
#(#extractors)*
#block
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than splitting up the function it might be possible to insert these into the front of function.block.stmts.

#vis mod #ident {
#initialization
}
39 changes: 39 additions & 0 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
@@ -580,3 +580,42 @@ pub unsafe fn tp_new_impl<T: PyClass>(
.create_class_object_of_type(py, target_type)
.map(Bound::into_ptr)
}

pub fn inspect_type<T>(t: T) -> (T, Extractor<T>) {
(t, Extractor::new())
}

pub struct Extractor<T>(NotAGilRef<T>);
pub struct NotAGilRef<T>(std::marker::PhantomData<T>);

pub trait IsGilRef {}

impl<T: crate::PyNativeType> IsGilRef for &'_ T {}

impl<T> Extractor<T> {
pub fn new() -> Self {
Extractor(NotAGilRef(std::marker::PhantomData))
}
}

impl<T: IsGilRef> Extractor<T> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, T>` instead for this function argument"
)
)]
pub fn extract_gil_ref(&self) {}
}

impl<T> NotAGilRef<T> {
pub fn extract_gil_ref(&self) {}
}

impl<T> std::ops::Deref for Extractor<T> {
type Target = NotAGilRef<T>;
fn deref(&self) -> &Self::Target {
&self.0
}
}