Skip to content

Commit

Permalink
remove the gil-ref deprecations infrastructure (#4320)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Icxolu
Icxolu authored Jul 8, 2024
1 parent d5c886f commit 3c155d9
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 650 deletions.
170 changes: 43 additions & 127 deletions pyo3-macros-backend/src/frompyobject.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::attributes::{self, get_pyo3_options, CrateAttribute, FromPyWithAttribute};
use crate::utils::Ctx;
use proc_macro2::TokenStream;
use quote::{format_ident, quote, quote_spanned};
use quote::{format_ident, quote};
use syn::{
parenthesized,
parse::{Parse, ParseStream},
Expand Down Expand Up @@ -44,16 +44,14 @@ impl<'a> Enum<'a> {
}

/// Build derivation body for enums.
fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) {
fn build(&self, ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let mut var_extracts = Vec::new();
let mut variant_names = Vec::new();
let mut error_names = Vec::new();

let mut deprecations = TokenStream::new();
for var in &self.variants {
let (struct_derive, dep) = var.build(ctx);
deprecations.extend(dep);
let struct_derive = var.build(ctx);
let ext = quote!({
let maybe_ret = || -> #pyo3_path::PyResult<Self> {
#struct_derive
Expand All @@ -70,22 +68,19 @@ impl<'a> Enum<'a> {
error_names.push(&var.err_name);
}
let ty_name = self.enum_ident.to_string();
(
quote!(
let errors = [
#(#var_extracts),*
];
::std::result::Result::Err(
#pyo3_path::impl_::frompyobject::failed_to_extract_enum(
obj.py(),
#ty_name,
&[#(#variant_names),*],
&[#(#error_names),*],
&errors
)
quote!(
let errors = [
#(#var_extracts),*
];
::std::result::Result::Err(
#pyo3_path::impl_::frompyobject::failed_to_extract_enum(
obj.py(),
#ty_name,
&[#(#variant_names),*],
&[#(#error_names),*],
&errors
)
),
deprecations,
)
)
}
}
Expand Down Expand Up @@ -244,7 +239,7 @@ impl<'a> Container<'a> {
}

/// Build derivation body for a struct.
fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) {
fn build(&self, ctx: &Ctx) -> TokenStream {
match &self.ty {
ContainerType::StructNewtype(ident, from_py_with) => {
self.build_newtype_struct(Some(ident), from_py_with, ctx)
Expand All @@ -262,73 +257,42 @@ impl<'a> Container<'a> {
field_ident: Option<&Ident>,
from_py_with: &Option<FromPyWithAttribute>,
ctx: &Ctx,
) -> (TokenStream, TokenStream) {
) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let self_ty = &self.path;
let struct_name = self.name();
if let Some(ident) = field_ident {
let field_name = ident.to_string();
match from_py_with {
None => (
quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
})
},
TokenStream::new(),
),
None => quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
})
},
Some(FromPyWithAttribute {
value: expr_path, ..
}) => (
quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
})
},
quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
},
),
}) => quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
})
},
}
} else {
match from_py_with {
None => (
quote!(
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
),
TokenStream::new(),
),
None => quote! {
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
},

Some(FromPyWithAttribute {
value: expr_path, ..
}) => (
quote! (
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
),
quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
},
),
}) => quote! {
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
},
}
}
}

fn build_tuple_struct(
&self,
struct_fields: &[TupleStructField],
ctx: &Ctx,
) -> (TokenStream, TokenStream) {
fn build_tuple_struct(&self, struct_fields: &[TupleStructField], ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let self_ty = &self.path;
let struct_name = &self.name();
Expand All @@ -348,40 +312,15 @@ impl<'a> Container<'a> {
}
});

let deprecations = struct_fields
.iter()
.filter_map(|field| {
let FromPyWithAttribute {
value: expr_path, ..
} = field.from_py_with.as_ref()?;
Some(quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
})
})
.collect::<TokenStream>();

(
quote!(
match #pyo3_path::types::PyAnyMethods::extract(obj) {
::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
::std::result::Result::Err(err) => ::std::result::Result::Err(err),
}
),
deprecations,
quote!(
match #pyo3_path::types::PyAnyMethods::extract(obj) {
::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
::std::result::Result::Err(err) => ::std::result::Result::Err(err),
}
)
}

fn build_struct(
&self,
struct_fields: &[NamedStructField<'_>],
ctx: &Ctx,
) -> (TokenStream, TokenStream) {
fn build_struct(&self, struct_fields: &[NamedStructField<'_>], ctx: &Ctx) -> TokenStream {
let Ctx { pyo3_path, .. } = ctx;
let self_ty = &self.path;
let struct_name = &self.name();
Expand Down Expand Up @@ -420,28 +359,7 @@ impl<'a> Container<'a> {
fields.push(quote!(#ident: #extractor));
}

let deprecations = struct_fields
.iter()
.filter_map(|field| {
let FromPyWithAttribute {
value: expr_path, ..
} = field.from_py_with.as_ref()?;
Some(quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
})
})
.collect::<TokenStream>();

(
quote!(::std::result::Result::Ok(#self_ty{#fields})),
deprecations,
)
quote!(::std::result::Result::Ok(#self_ty{#fields}))
}
}

Expand Down Expand Up @@ -673,7 +591,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
let ctx = &Ctx::new(&options.krate, None);
let Ctx { pyo3_path, .. } = &ctx;

let (derives, from_py_with_deprecations) = match &tokens.data {
let derives = match &tokens.data {
syn::Data::Enum(en) => {
if options.transparent || options.annotation.is_some() {
bail_spanned!(tokens.span() => "`transparent` or `annotation` is not supported \
Expand Down Expand Up @@ -703,7 +621,5 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
#derives
}
}

#from_py_with_deprecations
))
}
14 changes: 2 additions & 12 deletions pyo3-macros-backend/src/method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,11 +683,10 @@ impl<'a> FnSpec<'a> {
}
_ => {
if let Some(self_arg) = self_arg() {
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),
#self_arg
#(#args),*
)
}
Expand All @@ -714,11 +713,10 @@ impl<'a> FnSpec<'a> {
}
call
} else if let Some(self_arg) = self_arg() {
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),
#self_arg
#(#args),*
)
}
Expand Down Expand Up @@ -762,7 +760,6 @@ impl<'a> FnSpec<'a> {
})
.collect();
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 @@ -774,7 +771,6 @@ impl<'a> FnSpec<'a> {
let function = #rust_name; // Shadow the function name to avoid #3017
#init_holders
let result = #call;
#check_gil_refs
result
}
}
Expand All @@ -784,7 +780,6 @@ impl<'a> FnSpec<'a> {
let (arg_convert, args) = impl_arg_params(self, cls, true, &mut holders, ctx);
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 @@ -800,7 +795,6 @@ impl<'a> FnSpec<'a> {
#arg_convert
#init_holders
let result = #call;
#check_gil_refs
result
}
}
Expand All @@ -810,7 +804,6 @@ impl<'a> FnSpec<'a> {
let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx);
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 @@ -825,7 +818,6 @@ impl<'a> FnSpec<'a> {
#arg_convert
#init_holders
let result = #call;
#check_gil_refs
result
}
}
Expand All @@ -838,7 +830,6 @@ impl<'a> FnSpec<'a> {
.self_arg(cls, ExtractErrorMode::Raise, &mut holders, ctx);
let call = quote_spanned! {*output_span=> #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 @@ -854,7 +845,6 @@ impl<'a> FnSpec<'a> {
#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
27 changes: 1 addition & 26 deletions pyo3-macros-backend/src/module.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::ffi::CString;
use syn::{
ext::IdentExt,
parse::{Parse, ParseStream},
parse_quote, parse_quote_spanned,
parse_quote,
punctuated::Punctuated,
spanned::Spanned,
token::Comma,
Expand Down Expand Up @@ -377,7 +377,6 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
let options = PyModuleOptions::from_attrs(&mut function.attrs)?;
process_functions_in_module(&options, &mut function)?;
let ctx = &Ctx::new(&options.krate, None);
let stmts = std::mem::take(&mut function.block.stmts);
let Ctx { pyo3_path, .. } = ctx;
let ident = &function.sig.ident;
let name = options.name.unwrap_or_else(|| ident.unraw());
Expand All @@ -394,30 +393,6 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
module_args
.push(quote!(::std::convert::Into::into(#pyo3_path::impl_::pymethods::BoundRef(module))));

let extractors = function
.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: &syn::Ident = &pat_ident.ident;
return Some([
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(); },
]);
}
}
None
})
.flatten();

function.block.stmts = extractors.chain(stmts).collect();
function
.attrs
.push(parse_quote!(#[allow(clippy::used_underscore_binding)]));

Ok(quote! {
#function
#[doc(hidden)]
Expand Down
Loading

0 comments on commit 3c155d9

Please sign in to comment.