From 6e1834cf1de77cebf0c6dd56f02f66d05011298c Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Tue, 19 Mar 2024 23:11:00 +0100 Subject: [PATCH] deprecate `from_py_with` in `#[derive(FromPyObject)]` (Enum, Struct) --- pyo3-macros-backend/src/frompyobject.rs | 108 ++++++++++++++++++------ tests/ui/deprecations.rs | 39 +++++++++ tests/ui/deprecations.stderr | 36 ++++++-- 3 files changed, 152 insertions(+), 31 deletions(-) diff --git a/pyo3-macros-backend/src/frompyobject.rs b/pyo3-macros-backend/src/frompyobject.rs index cfbcd734928..7f26e5b14fc 100644 --- a/pyo3-macros-backend/src/frompyobject.rs +++ b/pyo3-macros-backend/src/frompyobject.rs @@ -44,13 +44,16 @@ impl<'a> Enum<'a> { } /// Build derivation body for enums. - fn build(&self, ctx: &Ctx) -> TokenStream { + fn build(&self, ctx: &Ctx) -> (TokenStream, 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, _) = var.build(ctx); + let (struct_derive, dep) = var.build(ctx); + deprecations.extend(dep); let ext = quote!({ let maybe_ret = || -> #pyo3_path::PyResult { #struct_derive @@ -67,19 +70,22 @@ 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, ) } } @@ -246,8 +252,8 @@ impl<'a> Container<'a> { ContainerType::TupleNewtype(from_py_with) => { self.build_newtype_struct(None, from_py_with, ctx) } - ContainerType::Tuple(tups) => (self.build_tuple_struct(tups, ctx), TokenStream::new()), - ContainerType::Struct(tups) => (self.build_struct(tups, ctx), TokenStream::new()), + ContainerType::Tuple(tups) => self.build_tuple_struct(tups, ctx), + ContainerType::Struct(tups) => self.build_struct(tups, ctx), } } @@ -318,7 +324,11 @@ impl<'a> Container<'a> { } } - fn build_tuple_struct(&self, struct_fields: &[TupleStructField], ctx: &Ctx) -> TokenStream { + fn build_tuple_struct( + &self, + struct_fields: &[TupleStructField], + ctx: &Ctx, + ) -> (TokenStream, TokenStream) { let Ctx { pyo3_path } = ctx; let self_ty = &self.path; let struct_name = &self.name(); @@ -337,15 +347,41 @@ impl<'a> Container<'a> { ), } }); - 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), - } + + 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::(); + + ( + 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, ) } - fn build_struct(&self, struct_fields: &[NamedStructField<'_>], ctx: &Ctx) -> TokenStream { + fn build_struct( + &self, + struct_fields: &[NamedStructField<'_>], + ctx: &Ctx, + ) -> (TokenStream, TokenStream) { let Ctx { pyo3_path } = ctx; let self_ty = &self.path; let struct_name = &self.name(); @@ -383,7 +419,29 @@ impl<'a> Container<'a> { fields.push(quote!(#ident: #extractor)); } - quote!(::std::result::Result::Ok(#self_ty{#fields})) + + 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::(); + + ( + quote!(::std::result::Result::Ok(#self_ty{#fields})), + deprecations, + ) } } @@ -622,7 +680,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result { at top level for enums"); } let en = Enum::new(en, &tokens.ident)?; - (en.build(ctx), TokenStream::new()) + en.build(ctx) } syn::Data::Struct(st) => { if let Some(lit_str) = &options.annotation { diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs index 1dcc673a33a..cbaba2fa4ff 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -89,6 +89,45 @@ fn pyfunction_from_py_with( ) { } +#[derive(Debug, FromPyObject)] +pub struct Zap { + #[pyo3(item)] + name: String, + + #[pyo3(from_py_with = "PyAny::len", item("my_object"))] + some_object_length: usize, + + #[pyo3(from_py_with = "extract_bound")] + some_number: i32, +} + +#[derive(Debug, FromPyObject)] +pub struct ZapTuple( + String, + #[pyo3(from_py_with = "PyAny::len")] usize, + #[pyo3(from_py_with = "extract_bound")] i32, +); + +#[derive(Debug, FromPyObject, PartialEq, Eq)] +pub enum ZapEnum { + Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), + Zap(String, #[pyo3(from_py_with = "extract_bound")] i32), +} + +#[derive(Debug, FromPyObject, PartialEq, Eq)] +#[pyo3(transparent)] +pub struct TransparentFromPyWithGilRef { + #[pyo3(from_py_with = "extract_gil_ref")] + len: i32, +} + +#[derive(Debug, FromPyObject, PartialEq, Eq)] +#[pyo3(transparent)] +pub struct TransparentFromPyWithBound { + #[pyo3(from_py_with = "extract_bound")] + len: i32, +} + fn test_wrap_pyfunction(py: Python<'_>, m: &Bound<'_, PyModule>) { // should lint let _ = wrap_pyfunction!(double, py); diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 8bd1450874f..d5da8572d02 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -52,10 +52,34 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_ 87 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, | ^^^^^^^^^^^^^^^^^ -error: use of deprecated method `pyo3::deprecations::GilRefs::>::is_python`: use `wrap_pyfunction_bound!` instead - --> tests/ui/deprecations.rs:94:13 - | -94 | let _ = wrap_pyfunction!(double, py); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor + --> tests/ui/deprecations.rs:97:27 | - = note: this error originates in the macro `wrap_pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) +97 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))] + | ^^^^^^^^^^^^ + +error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor + --> tests/ui/deprecations.rs:107:27 + | +107 | #[pyo3(from_py_with = "PyAny::len")] usize, + | ^^^^^^^^^^^^ + +error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor + --> tests/ui/deprecations.rs:113:31 + | +113 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), + | ^^^^^^^^^^^^^^^^^ + +error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor + --> tests/ui/deprecations.rs:120:27 + | +120 | #[pyo3(from_py_with = "extract_gil_ref")] + | ^^^^^^^^^^^^^^^^^ + +error: use of deprecated method `pyo3::deprecations::GilRefs::>::is_python`: use `wrap_pyfunction_bound!` instead + --> tests/ui/deprecations.rs:133:13 + | +133 | let _ = wrap_pyfunction!(double, py); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `wrap_pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)