diff --git a/pyo3-macros-backend/src/frompyobject.rs b/pyo3-macros-backend/src/frompyobject.rs index 68ef72ea157..7f26e5b14fc 100644 --- a/pyo3-macros-backend/src/frompyobject.rs +++ b/pyo3-macros-backend/src/frompyobject.rs @@ -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}; +use quote::{format_ident, quote, quote_spanned}; use syn::{ parenthesized, parse::{Parse, ParseStream}, @@ -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, ) } } @@ -238,7 +244,7 @@ impl<'a> Container<'a> { } /// Build derivation body for a struct. - fn build(&self, ctx: &Ctx) -> TokenStream { + fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) { match &self.ty { ContainerType::StructNewtype(ident, from_py_with) => { self.build_newtype_struct(Some(ident), from_py_with, ctx) @@ -256,41 +262,73 @@ impl<'a> Container<'a> { field_ident: Option<&Ident>, from_py_with: &Option, 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)? - }) - }, + None => ( + quote! { + Ok(#self_ty { + #ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)? + }) + }, + TokenStream::new(), + ), 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! { + 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(); + } + }; + }, + ), } } else { match from_py_with { - None => quote!( - #pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty) + None => ( + quote!( + #pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty) + ), + TokenStream::new(), ), 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! ( + #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(); + } + }; + }, ), } } } - 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(); @@ -309,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(); @@ -355,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, + ) } } @@ -587,7 +673,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result { let ctx = &Ctx::new(&options.krate); let Ctx { pyo3_path } = &ctx; - let derives = match &tokens.data { + let (derives, from_py_with_deprecations) = match &tokens.data { syn::Data::Enum(en) => { if options.transparent || options.annotation.is_some() { bail_spanned!(tokens.span() => "`transparent` or `annotation` is not supported \ @@ -617,5 +703,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result { #derives } } + + #from_py_with_deprecations )) } diff --git a/src/instance.rs b/src/instance.rs index 024a9fe5b51..5f054d0d2d4 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -465,6 +465,13 @@ impl<'py, T> Bound<'py, T> { unsafe { Py::from_non_null(non_null) } } + /// Removes the connection for this `Bound` from the GIL, allowing + /// it to cross thread boundaries, without transferring ownership. + #[inline] + pub fn as_unbound(&self) -> &Py { + &self.1 + } + /// Casts this `Bound` as the corresponding "GIL Ref" type. /// /// This is a helper to be used for migration from the deprecated "GIL Refs" API. @@ -521,11 +528,11 @@ impl<'py, T> Borrowed<'_, 'py, T> { /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| -> PyResult<()> { /// let tuple = PyTuple::new_bound(py, [1, 2, 3]); - /// + /// /// // borrows from `tuple`, so can only be /// // used while `tuple` stays alive /// let borrowed = tuple.get_borrowed_item(0)?; - /// + /// /// // creates a new owned reference, which /// // can be used indendently of `tuple` /// let bound = borrowed.to_owned(); @@ -1960,8 +1967,8 @@ impl PyObject { mod tests { use super::{Bound, Py, PyObject}; use crate::types::any::PyAnyMethods; - use crate::types::PyCapsule; use crate::types::{dict::IntoPyDict, PyDict, PyString}; + use crate::types::{PyCapsule, PyStringMethods}; use crate::{ffi, Borrowed, PyAny, PyNativeType, PyResult, Python, ToPyObject}; #[test] @@ -2161,6 +2168,20 @@ a = A() }); } + #[test] + fn test_bound_py_conversions() { + Python::with_gil(|py| { + let obj: Bound<'_, PyString> = PyString::new_bound(py, "hello world"); + let obj_unbound: &Py = obj.as_unbound(); + let _: &Bound<'_, PyString> = obj_unbound.bind(py); + + let obj_unbound: Py = obj.unbind(); + let obj: Bound<'_, PyString> = obj_unbound.into_bound(py); + + assert_eq!(obj.to_cow().unwrap(), "hello world"); + }); + } + #[test] fn bound_from_borrowed_ptr_constructors() { // More detailed tests of the underlying semantics in pycell.rs diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs index 191d6b2b07b..f623215f531 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -95,6 +95,45 @@ fn pyfunction_from_py_with( #[pyfunction] fn pyfunction_gil_ref(_any: &PyAny) {} +#[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 1f48a58aea6..b133b21486c 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -64,10 +64,34 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg` 96 | fn pyfunction_gil_ref(_any: &PyAny) {} | ^ +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:103:27 + | +103 | #[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:113:27 + | +113 | #[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:119:31 + | +119 | 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:126:27 + | +126 | #[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:100:13 + --> tests/ui/deprecations.rs:139:13 | -100 | let _ = wrap_pyfunction!(double, py); +139 | 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)