Skip to content

Commit

Permalink
deprecate GIL Refs in function arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidhewitt
davidhewitt committed Mar 19, 2024
1 parent caf80ec commit 7c8bf45
Show file tree
Hide file tree
Showing 9 changed files with 206 additions and 151 deletions.
1 change: 0 additions & 1 deletion newsfragments/3963.added.md
View file

This file was deleted.

118 changes: 53 additions & 65 deletions pyo3-macros-backend/src/method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::utils::Ctx;
use crate::{
attributes::{TextSignatureAttribute, TextSignatureAttributeValue},
deprecations::{Deprecation, Deprecations},
params::impl_arg_params,
params::{impl_arg_params, Holders},
pyfunction::{
FunctionSignature, PyFunctionArgPyO3Attributes, PyFunctionOptions, SignatureAttribute,
},
Expand Down Expand Up @@ -108,7 +108,7 @@ impl FnType {
&self,
cls: Option<&syn::Type>,
error_mode: ExtractErrorMode,
holders: &mut Vec<TokenStream>,
holders: &mut Holders,
ctx: &Ctx,
) -> TokenStream {
let Ctx { pyo3_path } = ctx;
Expand Down Expand Up @@ -186,7 +186,7 @@ impl SelfType {
&self,
cls: &syn::Type,
error_mode: ExtractErrorMode,
holders: &mut Vec<TokenStream>,
holders: &mut Holders,
ctx: &Ctx,
) -> TokenStream {
// Due to use of quote_spanned in this function, need to bind these idents to the
Expand All @@ -201,17 +201,12 @@ impl SelfType {
} else {
syn::Ident::new("extract_pyclass_ref", *span)
};
let holder = syn::Ident::new(&format!("holder_{}", holders.len()), *span);
let holder = holders.push_holder(*span);
let pyo3_path = pyo3_path.to_tokens_spanned(*span);
holders.push(quote_spanned! { *span =>
#[allow(clippy::let_unit_value)]
let mut #holder = #pyo3_path::impl_::extract_argument::FunctionArgumentHolder::INIT;
let mut #slf = #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf);
});
error_mode.handle_error(
quote_spanned! { *span =>
#pyo3_path::impl_::extract_argument::#method::<#cls>(
&#slf,
#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf).0,
&mut #holder,
)
},
Expand Down Expand Up @@ -519,10 +514,8 @@ impl<'a> FnSpec<'a> {
);
}

let rust_call = |args: Vec<TokenStream>,
self_e: &syn::Ident,
holders: &mut Vec<TokenStream>| {
let self_arg = self.tp.self_arg(cls, ExtractErrorMode::Raise, holders, ctx);
let rust_call = |args: Vec<TokenStream>, holders: &mut Holders| {
let mut self_arg = || self.tp.self_arg(cls, ExtractErrorMode::Raise, holders, ctx);

let call = if self.asyncness.is_some() {
let throw_callback = if cancel_handle.is_some() {
Expand All @@ -537,35 +530,30 @@ impl<'a> FnSpec<'a> {
};
let future = match self.tp {
FnType::Fn(SelfType::Receiver { mutable: false, .. }) => {
holders.pop().unwrap(); // does not actually use holder created by `self_arg`

quote! {{
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
let __guard = #pyo3_path::impl_::coroutine::RefGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
async move { function(&__guard, #(#args),*).await }
}}
}
FnType::Fn(SelfType::Receiver { mutable: true, .. }) => {
holders.pop().unwrap(); // does not actually use holder created by `self_arg`

quote! {{
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
let mut __guard = #pyo3_path::impl_::coroutine::RefMutGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
async move { function(&mut __guard, #(#args),*).await }
}}
}
_ => {
let self_arg = self_arg();
if self_arg.is_empty() {
quote! {{
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
function(#(#args),*)
}}
quote! { function(#(#args),*) }
} else {
quote! { function({
let (self_arg, e) = #pyo3_path::impl_::deprecations::inspect_type(#self_arg);
#self_e = e;
self_arg
}, #(#args),*) }
let self_checker = holders.push_gil_refs_checker(self_arg.span());
quote! {
function(
// NB #self_arg includes a comma, so none inserted here
#pyo3_path::impl_::deprecations::inspect_type(#self_arg &#self_checker),
#(#args),*
)
}
}
}
};
Expand All @@ -586,24 +574,22 @@ impl<'a> FnSpec<'a> {
}};
}
call
} else if self_arg.is_empty() {
quote! {{
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
function(#(#args),*)
}}
} else {
quote! {
function({
let (self_arg, e) = #pyo3_path::impl_::deprecations::inspect_type(#self_arg);
#self_e = e;
self_arg
}, #(#args),*)
let self_arg = self_arg();
if self_arg.is_empty() {
quote! { function(#(#args),*) }
} else {
let self_checker = holders.push_gil_refs_checker(self_arg.span());
quote! {
function(
// NB #self_arg includes a comma, so none inserted here
#pyo3_path::impl_::deprecations::inspect_type(#self_arg &#self_checker),
#(#args),*
)
}
}
};
(
quotes::map_result_into_ptr(quotes::ok_wrap(call, ctx), ctx),
self_arg.span(),
)
quotes::map_result_into_ptr(quotes::ok_wrap(call, ctx), ctx)
};

let func_name = &self.name;
Expand All @@ -613,10 +599,9 @@ impl<'a> FnSpec<'a> {
quote!(#func_name)
};

let self_e = syn::Ident::new("self_e", Span::call_site());
Ok(match self.convention {
CallingConvention::Noargs => {
let mut holders = Vec::new();
let mut holders = Holders::new();
let args = self
.signature
.arguments
Expand All @@ -631,8 +616,9 @@ impl<'a> FnSpec<'a> {
}
})
.collect();
let (call, self_arg_span) = rust_call(args, &self_e, &mut holders);
let function_arg = quote_spanned! { self_arg_span => #self_e.function_arg(); };
let call = rust_call(args, &mut holders);
let check_gil_refs = holders.check_gil_refs();
let init_holders = holders.init_holders(ctx);

quote! {
unsafe fn #ident<'py>(
Expand All @@ -641,19 +627,19 @@ impl<'a> FnSpec<'a> {
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
let _slf_ref = &_slf;
let function = #rust_name; // Shadow the function name to avoid #3017
let #self_e;
#( #holders )*
#init_holders
let result = #call;
#function_arg
#check_gil_refs
result
}
}
}
CallingConvention::Fastcall => {
let mut holders = Vec::new();
let mut holders = Holders::new();
let (arg_convert, args) = impl_arg_params(self, cls, true, &mut holders, ctx)?;
let (call, self_arg_span) = rust_call(args, &self_e, &mut holders);
let function_arg = quote_spanned! { self_arg_span => #self_e.function_arg(); };
let call = rust_call(args, &mut holders);
let init_holders = holders.init_holders(ctx);
let check_gil_refs = holders.check_gil_refs();

quote! {
unsafe fn #ident<'py>(
Expand All @@ -665,20 +651,20 @@ impl<'a> FnSpec<'a> {
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
let _slf_ref = &_slf;
let function = #rust_name; // Shadow the function name to avoid #3017
let #self_e;
#arg_convert
#( #holders )*
#init_holders
let result = #call;
#function_arg
#check_gil_refs
result
}
}
}
CallingConvention::Varargs => {
let mut holders = Vec::new();
let mut holders = Holders::new();
let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx)?;
let (call, self_arg_span) = rust_call(args, &self_e, &mut holders);
let function_arg = quote_spanned! { self_arg_span => #self_e.function_arg(); };
let call = rust_call(args, &mut holders);
let init_holders = holders.init_holders(ctx);
let check_gil_refs = holders.check_gil_refs();

quote! {
unsafe fn #ident<'py>(
Expand All @@ -689,22 +675,23 @@ impl<'a> FnSpec<'a> {
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
let _slf_ref = &_slf;
let function = #rust_name; // Shadow the function name to avoid #3017
let #self_e;
#arg_convert
#( #holders )*
#init_holders
let result = #call;
#function_arg
#check_gil_refs
result
}
}
}
CallingConvention::TpNew => {
let mut holders = Vec::new();
let mut holders = Holders::new();
let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx)?;
let self_arg = self
.tp
.self_arg(cls, ExtractErrorMode::Raise, &mut holders, ctx);
let call = quote! { #rust_name(#self_arg #(#args),*) };
let init_holders = holders.init_holders(ctx);
let check_gil_refs = holders.check_gil_refs();
quote! {
unsafe fn #ident(
py: #pyo3_path::Python<'_>,
Expand All @@ -716,9 +703,10 @@ impl<'a> FnSpec<'a> {
let _slf_ref = &_slf;
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#( #holders )*
#init_holders
let result = #call;
let initializer: #pyo3_path::PyClassInitializer::<#cls> = result.convert(py)?;
#check_gil_refs
#pyo3_path::impl_::pymethods::tp_new_impl(py, initializer, _slf)
}
}
Expand Down
7 changes: 4 additions & 3 deletions pyo3-macros-backend/src/module.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,11 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
.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;
let ident: &syn::Ident = &pat_ident.ident;
return Some([
parse_quote! { let (#ident, e) = #pyo3_path::impl_::deprecations::inspect_type(#ident); },
parse_quote_spanned! { pat_type.span() => e.function_arg(); },
parse_quote!{ let check_gil_refs = #pyo3_path::impl_::deprecations::GilRefs::new(); },
parse_quote! { let #ident = #pyo3_path::impl_::deprecations::inspect_type(#ident, &check_gil_refs); },
parse_quote_spanned! { pat_type.span() => check_gil_refs.function_arg(); },
]);
}
}
Expand Down
Loading

0 comments on commit 7c8bf45

Please sign in to comment.