From 843afb0766f795411869fb183d067827c332cf2a Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Thu, 9 Jan 2025 17:57:04 +0100 Subject: [PATCH 1/2] introduce `into_py_with`/`into_py_with_ref` for `#[derive(IntoPyObject, IntoPyObjectRef)]` --- guide/src/conversions/traits.md | 28 +++++++ newsfragments/4850.added.md | 1 + pyo3-macros-backend/src/attributes.rs | 4 + pyo3-macros-backend/src/intopyobject.rs | 102 ++++++++++++++++++++---- tests/test_compile_error.rs | 2 + tests/test_intopyobject.rs | 66 +++++++++++++-- tests/ui/invalid_intopy_derive.rs | 50 ++++++++++++ tests/ui/invalid_intopy_derive.stderr | 48 +++++++++++ tests/ui/invalid_intopy_with.rs | 23 ++++++ tests/ui/invalid_intopy_with.stderr | 23 ++++++ 10 files changed, 325 insertions(+), 22 deletions(-) create mode 100644 newsfragments/4850.added.md create mode 100644 tests/ui/invalid_intopy_with.rs create mode 100644 tests/ui/invalid_intopy_with.stderr diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md index 1aa445cce41..c1028f5110e 100644 --- a/guide/src/conversions/traits.md +++ b/guide/src/conversions/traits.md @@ -606,6 +606,34 @@ enum Enum<'a, 'py, K: Hash + Eq, V> { // enums are supported and convert using t Additionally `IntoPyObject` can be derived for a reference to a struct or enum using the `IntoPyObjectRef` derive macro. All the same rules from above apply as well. +##### `#[derive(IntoPyObject)]`/`#[derive(IntoPyObjectRef)]` Field Attributes +- `pyo3(into_py_with = ...)`/`pyo3(into_py_with_ref = ...)` + - apply a custom function to convert the field from Rust into Python. + - the argument must be the function indentifier + - the function signature must be `fn(T, Python<'_>) -> PyResult>`/`fn<'py>(&T, Python<'py>) -> PyResult>` where `T` is the Rust type of the argument. + + ```rust + # use pyo3::prelude::*; + # use pyo3::IntoPyObjectExt; + struct NotIntoPy(usize); + + #[derive(IntoPyObject, IntoPyObjectRef)] + struct MyStruct { + #[pyo3(into_py_with = convert, into_py_with_ref = convert_ref)] + not_into_py: NotIntoPy, + } + + /// Convert `NotIntoPy` into Python by value + fn convert(NotIntoPy(i): NotIntoPy, py: Python<'_>) -> PyResult> { + i.into_bound_py_any(py) + } + + /// Convert `NotIntoPy` into Python by reference + fn convert_ref<'py>(&NotIntoPy(i): &NotIntoPy, py: Python<'py>) -> PyResult> { + i.into_bound_py_any(py) + } + ``` + #### manual implementation If the derive macro is not suitable for your use case, `IntoPyObject` can be implemented manually as diff --git a/newsfragments/4850.added.md b/newsfragments/4850.added.md new file mode 100644 index 00000000000..acdd7c2e48a --- /dev/null +++ b/newsfragments/4850.added.md @@ -0,0 +1 @@ +introduce `into_py_with`/`into_py_with_ref` for `#[derive(IntoPyObject, IntoPyObjectRef)]` \ No newline at end of file diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index bd5da377121..589c853922b 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -25,6 +25,8 @@ pub mod kw { syn::custom_keyword!(get); syn::custom_keyword!(get_all); syn::custom_keyword!(hash); + syn::custom_keyword!(into_py_with); + syn::custom_keyword!(into_py_with_ref); syn::custom_keyword!(item); syn::custom_keyword!(from_item_all); syn::custom_keyword!(mapping); @@ -350,6 +352,8 @@ impl ToTokens for OptionalKeywordAttribute { } pub type FromPyWithAttribute = KeywordAttribute>; +pub type IntoPyWithAttribute = KeywordAttribute; +pub type IntoPyWithRefAttribute = KeywordAttribute; pub type DefaultAttribute = OptionalKeywordAttribute; diff --git a/pyo3-macros-backend/src/intopyobject.rs b/pyo3-macros-backend/src/intopyobject.rs index a60a5486cb8..8fbc6cd4a65 100644 --- a/pyo3-macros-backend/src/intopyobject.rs +++ b/pyo3-macros-backend/src/intopyobject.rs @@ -1,4 +1,6 @@ -use crate::attributes::{self, get_pyo3_options, CrateAttribute}; +use crate::attributes::{ + self, get_pyo3_options, CrateAttribute, IntoPyWithAttribute, IntoPyWithRefAttribute, +}; use crate::utils::Ctx; use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote, quote_spanned}; @@ -89,6 +91,8 @@ impl ItemOption { enum FieldAttribute { Item(ItemOption), + IntoPyWith(IntoPyWithAttribute), + IntoPyWithRef(IntoPyWithRefAttribute), } impl Parse for FieldAttribute { @@ -118,6 +122,10 @@ impl Parse for FieldAttribute { span: attr.span, })) } + } else if lookahead.peek(attributes::kw::into_py_with) { + input.parse().map(FieldAttribute::IntoPyWith) + } else if lookahead.peek(attributes::kw::into_py_with_ref) { + input.parse().map(FieldAttribute::IntoPyWithRef) } else { Err(lookahead.error()) } @@ -127,6 +135,8 @@ impl Parse for FieldAttribute { #[derive(Clone, Debug, Default)] struct FieldAttributes { item: Option, + into_py_with: Option, + into_py_with_ref: Option, } impl FieldAttributes { @@ -159,6 +169,8 @@ impl FieldAttributes { match option { FieldAttribute::Item(item) => set_option!(item), + FieldAttribute::IntoPyWith(into_py_with) => set_option!(into_py_with), + FieldAttribute::IntoPyWithRef(into_py_with_ref) => set_option!(into_py_with_ref), } Ok(()) } @@ -182,10 +194,14 @@ struct NamedStructField<'a> { ident: &'a syn::Ident, field: &'a syn::Field, item: Option, + into_py_with: Option, + into_py_with_ref: Option, } struct TupleStructField<'a> { field: &'a syn::Field, + into_py_with: Option, + into_py_with_ref: Option, } /// Container Style @@ -214,14 +230,14 @@ enum ContainerType<'a> { /// Data container /// /// Either describes a struct or an enum variant. -struct Container<'a> { +struct Container<'a, const REF: bool> { path: syn::Path, receiver: Option, ty: ContainerType<'a>, } /// Construct a container based on fields, identifier and attributes. -impl<'a> Container<'a> { +impl<'a, const REF: bool> Container<'a, REF> { /// /// Fails if the variant has no fields or incompatible attributes. fn new( @@ -241,13 +257,29 @@ impl<'a> Container<'a> { attrs.item.is_none(), attrs.item.unwrap().span() => "`item` is not permitted on tuple struct elements." ); - Ok(TupleStructField { field }) + Ok(TupleStructField { + field, + into_py_with: attrs.into_py_with, + into_py_with_ref: attrs.into_py_with_ref + }) }) .collect::>>()?; if tuple_fields.len() == 1 { // Always treat a 1-length tuple struct as "transparent", even without the // explicit annotation. - let TupleStructField { field } = tuple_fields.pop().unwrap(); + let TupleStructField { + field, + into_py_with, + into_py_with_ref, + } = tuple_fields.pop().unwrap(); + ensure_spanned!( + into_py_with.is_none(), + into_py_with.span() => "`into_py_with` is not permitted on `transparent` structs" + ); + ensure_spanned!( + into_py_with_ref.is_none(), + into_py_with_ref.span() => "`into_py_with_ref` is not permitted on `transparent` structs" + ); ContainerType::TupleNewtype(field) } else if options.transparent.is_some() { bail_spanned!( @@ -270,6 +302,14 @@ impl<'a> Container<'a> { attrs.item.is_none(), attrs.item.unwrap().span() => "`transparent` structs may not have `item` for the inner field" ); + ensure_spanned!( + attrs.into_py_with.is_none(), + attrs.into_py_with.span() => "`into_py_with` is not permitted on `transparent` structs or variants" + ); + ensure_spanned!( + attrs.into_py_with_ref.is_none(), + attrs.into_py_with_ref.span() => "`into_py_with_ref` is not permitted on `transparent` structs or variants" + ); ContainerType::StructNewtype(field) } else { let struct_fields = named @@ -287,6 +327,8 @@ impl<'a> Container<'a> { ident, field, item: attrs.item, + into_py_with: attrs.into_py_with, + into_py_with_ref: attrs.into_py_with_ref, }) }) .collect::>>()?; @@ -389,8 +431,21 @@ impl<'a> Container<'a> { .map(|item| item.value()) .unwrap_or_else(|| f.ident.unraw().to_string()); let value = Ident::new(&format!("arg{i}"), f.field.ty.span()); - quote! { - #pyo3_path::types::PyDictMethods::set_item(&dict, #key, #value)?; + let expr_path = if REF { + f.into_py_with_ref.as_ref().map(|i|&i.value) + } else { + f.into_py_with.as_ref().map(|i|&i.value) + }; + + if let Some(expr_path) = expr_path { + quote! { + let into_py_with: fn(_, #pyo3_path::Python<'_>) -> #pyo3_path::PyResult<#pyo3_path::Bound<'_, #pyo3_path::PyAny>> = #expr_path; + #pyo3_path::types::PyDictMethods::set_item(&dict, #key, into_py_with(#value, py)?)?; + } + } else { + quote! { + #pyo3_path::types::PyDictMethods::set_item(&dict, #key, #value)?; + } } }) .collect::(); @@ -426,11 +481,26 @@ impl<'a> Container<'a> { .iter() .enumerate() .map(|(i, f)| { + let ty = &f.field.ty; let value = Ident::new(&format!("arg{i}"), f.field.ty.span()); - quote_spanned! { f.field.ty.span() => - #pyo3_path::conversion::IntoPyObject::into_pyobject(#value, py) - .map(#pyo3_path::BoundObject::into_any) - .map(#pyo3_path::BoundObject::into_bound)?, + let expr_path = if REF { + f.into_py_with_ref.as_ref().map(|i|&i.value) + } else { + f.into_py_with.as_ref().map(|i|&i.value) + }; + if let Some(expr_path) = expr_path { + quote_spanned! { ty.span() => + { + let into_py_with: fn(_, #pyo3_path::Python<'_>) -> #pyo3_path::PyResult<#pyo3_path::Bound<'_, #pyo3_path::PyAny>> = #expr_path; + into_py_with(#value, py)? + }, + } + } else { + quote_spanned! { ty.span() => + #pyo3_path::conversion::IntoPyObject::into_pyobject(#value, py) + .map(#pyo3_path::BoundObject::into_any) + .map(#pyo3_path::BoundObject::into_bound)?, + } } }) .collect::(); @@ -450,11 +520,11 @@ impl<'a> Container<'a> { } /// Describes derivation input of an enum. -struct Enum<'a> { - variants: Vec>, +struct Enum<'a, const REF: bool> { + variants: Vec>, } -impl<'a> Enum<'a> { +impl<'a, const REF: bool> Enum<'a, REF> { /// Construct a new enum representation. /// /// `data_enum` is the `syn` representation of the input enum, `ident` is the @@ -563,12 +633,12 @@ pub fn build_derive_into_pyobject(tokens: &DeriveInput) -> Resu if options.transparent.is_some() { bail_spanned!(tokens.span() => "`transparent` is not supported at top level for enums"); } - let en = Enum::new(en, &tokens.ident)?; + let en = Enum::::new(en, &tokens.ident)?; en.build(ctx) } syn::Data::Struct(st) => { let ident = &tokens.ident; - let st = Container::new( + let st = Container::::new( Some(Ident::new("self", Span::call_site())), &st.fields, parse_quote!(#ident), diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 10b692a604c..a60d11698f6 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -26,6 +26,8 @@ fn test_compile_errors() { t.compile_fail("tests/ui/pyclass_send.rs"); t.compile_fail("tests/ui/invalid_argument_attributes.rs"); t.compile_fail("tests/ui/invalid_intopy_derive.rs"); + #[cfg(not(windows))] + t.compile_fail("tests/ui/invalid_intopy_with.rs"); t.compile_fail("tests/ui/invalid_frompy_derive.rs"); t.compile_fail("tests/ui/static_ref.rs"); t.compile_fail("tests/ui/wrong_aspyref_lifetimes.rs"); diff --git a/tests/test_intopyobject.rs b/tests/test_intopyobject.rs index 971663b05d7..4335c55ecd7 100644 --- a/tests/test_intopyobject.rs +++ b/tests/test_intopyobject.rs @@ -1,7 +1,7 @@ #![cfg(feature = "macros")] -use pyo3::types::{PyDict, PyString}; -use pyo3::{prelude::*, IntoPyObject}; +use pyo3::types::{PyDict, PyList, PyString}; +use pyo3::{prelude::*, py_run, IntoPyObject, IntoPyObjectExt}; use std::collections::HashMap; use std::hash::Hash; @@ -150,9 +150,20 @@ fn test_transparent_tuple_struct() { }); } +fn phantom_into_py( + _: std::marker::PhantomData, + py: Python<'_>, +) -> PyResult> { + std::any::type_name::().into_bound_py_any(py) +} + #[derive(Debug, IntoPyObject)] pub enum Foo<'py> { - TupleVar(usize, String), + TupleVar( + usize, + String, + #[pyo3(into_py_with = phantom_into_py::<()>)] std::marker::PhantomData<()>, + ), StructVar { test: Bound<'py, PyString>, }, @@ -167,10 +178,12 @@ pub enum Foo<'py> { #[test] fn test_enum() { Python::with_gil(|py| { - let foo = Foo::TupleVar(1, "test".into()).into_pyobject(py).unwrap(); + let foo = Foo::TupleVar(1, "test".into(), std::marker::PhantomData) + .into_pyobject(py) + .unwrap(); assert_eq!( - foo.extract::<(usize, String)>().unwrap(), - (1, String::from("test")) + foo.extract::<(usize, String, String)>().unwrap(), + (1, String::from("test"), String::from("()")) ); let foo = Foo::StructVar { @@ -199,3 +212,44 @@ fn test_enum() { assert!(foo.is_none()); }); } + +#[derive(Debug, IntoPyObject, IntoPyObjectRef)] +pub struct Zap { + #[pyo3(item)] + name: String, + + #[pyo3(into_py_with = zap_into_py, into_py_with_ref = zap_into_py_ref, item("my_object"))] + some_object_length: usize, +} + +fn zap_into_py(len: usize, py: Python<'_>) -> PyResult> { + Ok(PyList::new(py, 1..len + 1)?.into_any()) +} + +fn zap_into_py_ref<'py>(&len: &usize, py: Python<'py>) -> PyResult> { + Ok(PyList::new(py, 1..len + 1)?.into_any()) +} + +#[test] +fn test_into_py_with() { + Python::with_gil(|py| { + let zap = Zap { + name: "whatever".into(), + some_object_length: 3, + }; + + let py_zap_ref = (&zap).into_pyobject(py).unwrap(); + let py_zap = zap.into_pyobject(py).unwrap(); + + py_run!( + py, + py_zap_ref, + "assert py_zap_ref == {'name': 'whatever', 'my_object': [1, 2, 3]},f'{py_zap_ref}'" + ); + py_run!( + py, + py_zap, + "assert py_zap == {'name': 'whatever', 'my_object': [1, 2, 3]},f'{py_zap}'" + ); + }); +} diff --git a/tests/ui/invalid_intopy_derive.rs b/tests/ui/invalid_intopy_derive.rs index c65d44ff1bc..3bf1de4a050 100644 --- a/tests/ui/invalid_intopy_derive.rs +++ b/tests/ui/invalid_intopy_derive.rs @@ -106,4 +106,54 @@ struct StructTransparentItem { foo: String, } +#[derive(IntoPyObject)] +#[pyo3(transparent)] +struct StructTransparentIntoPyWith { + #[pyo3(into_py_with = into)] + foo: String, +} + +#[derive(IntoPyObjectRef)] +#[pyo3(transparent)] +struct StructTransparentIntoPyWithRef { + #[pyo3(into_py_with = into_ref)] + foo: String, +} + +#[derive(IntoPyObject)] +#[pyo3(transparent)] +struct TupleTransparentIntoPyWith(#[pyo3(into_py_with = into)] String); + +#[derive(IntoPyObjectRef)] +#[pyo3(transparent)] +struct TupleTransparentIntoPyWithRef(#[pyo3(into_py_with_ref = into_ref)] String); + +#[derive(IntoPyObject)] +enum EnumTupleIntoPyWith { + TransparentTuple(#[pyo3(into_py_with = into)] usize), +} + +#[derive(IntoPyObject)] +enum EnumStructIntoPyWith { + #[pyo3(transparent)] + TransparentStruct { + #[pyo3(into_py_with = into)] + a: usize, + }, +} + +#[derive(IntoPyObjectRef)] +enum EnumTupleIntoPyWithRef { + TransparentTuple(#[pyo3(into_py_with_ref = into_ref)] usize), +} + +#[derive(IntoPyObjectRef)] +enum EnumStructIntoPyWithRef { + #[pyo3(transparent)] + TransparentStruct { + #[pyo3(into_py_with_ref = into_ref)] + a: usize, + }, +} + fn main() {} diff --git a/tests/ui/invalid_intopy_derive.stderr b/tests/ui/invalid_intopy_derive.stderr index cf125d9c073..5d9baec096a 100644 --- a/tests/ui/invalid_intopy_derive.stderr +++ b/tests/ui/invalid_intopy_derive.stderr @@ -125,3 +125,51 @@ error: `transparent` structs may not have `item` for the inner field | 105 | #[pyo3(item)] | ^^^^ + +error: `into_py_with` is not permitted on `transparent` structs or variants + --> tests/ui/invalid_intopy_derive.rs:112:12 + | +112 | #[pyo3(into_py_with = into)] + | ^^^^^^^^^^^^ + +error: `into_py_with` is not permitted on `transparent` structs or variants + --> tests/ui/invalid_intopy_derive.rs:119:12 + | +119 | #[pyo3(into_py_with = into_ref)] + | ^^^^^^^^^^^^ + +error: `into_py_with` is not permitted on `transparent` structs + --> tests/ui/invalid_intopy_derive.rs:125:42 + | +125 | struct TupleTransparentIntoPyWith(#[pyo3(into_py_with = into)] String); + | ^^^^^^^^^^^^ + +error: `into_py_with_ref` is not permitted on `transparent` structs + --> tests/ui/invalid_intopy_derive.rs:129:45 + | +129 | struct TupleTransparentIntoPyWithRef(#[pyo3(into_py_with_ref = into_ref)] String); + | ^^^^^^^^^^^^^^^^ + +error: `into_py_with` is not permitted on `transparent` structs + --> tests/ui/invalid_intopy_derive.rs:133:29 + | +133 | TransparentTuple(#[pyo3(into_py_with = into)] usize), + | ^^^^^^^^^^^^ + +error: `into_py_with` is not permitted on `transparent` structs or variants + --> tests/ui/invalid_intopy_derive.rs:140:16 + | +140 | #[pyo3(into_py_with = into)] + | ^^^^^^^^^^^^ + +error: `into_py_with_ref` is not permitted on `transparent` structs + --> tests/ui/invalid_intopy_derive.rs:147:29 + | +147 | TransparentTuple(#[pyo3(into_py_with_ref = into_ref)] usize), + | ^^^^^^^^^^^^^^^^ + +error: `into_py_with_ref` is not permitted on `transparent` structs or variants + --> tests/ui/invalid_intopy_derive.rs:154:16 + | +154 | #[pyo3(into_py_with_ref = into_ref)] + | ^^^^^^^^^^^^^^^^ diff --git a/tests/ui/invalid_intopy_with.rs b/tests/ui/invalid_intopy_with.rs new file mode 100644 index 00000000000..bf34701190d --- /dev/null +++ b/tests/ui/invalid_intopy_with.rs @@ -0,0 +1,23 @@ +use pyo3::{IntoPyObject, IntoPyObjectRef}; + +#[derive(IntoPyObject)] +struct InvalidIntoPyWithFn { + #[pyo3(into_py_with = into)] + inner: String, +} + +#[derive(IntoPyObjectRef)] +struct InvalidIntoPyWithRefFn { + #[pyo3(into_py_with_ref = into_ref)] + inner: String, +} + +fn into(_a: String) -> pyo3::Py { + todo!() +} + +fn into_ref(_a: String, _py: pyo3::Python<'_>) -> pyo3::Bound<'_, pyo3::PyAny> { + todo!() +} + +fn main() {} diff --git a/tests/ui/invalid_intopy_with.stderr b/tests/ui/invalid_intopy_with.stderr new file mode 100644 index 00000000000..a45f8860f8b --- /dev/null +++ b/tests/ui/invalid_intopy_with.stderr @@ -0,0 +1,23 @@ +error[E0308]: mismatched types + --> tests/ui/invalid_intopy_with.rs:5:27 + | +3 | #[derive(IntoPyObject)] + | ------------ expected due to this +4 | struct InvalidIntoPyWithFn { +5 | #[pyo3(into_py_with = into)] + | ^^^^ incorrect number of function parameters + | + = note: expected fn pointer `for<'a> fn(_, Python<'a>) -> Result, PyErr>` + found fn item `fn(String) -> Py {into}` + +error[E0308]: mismatched types + --> tests/ui/invalid_intopy_with.rs:11:31 + | +9 | #[derive(IntoPyObjectRef)] + | --------------- expected due to this +10 | struct InvalidIntoPyWithRefFn { +11 | #[pyo3(into_py_with_ref = into_ref)] + | ^^^^^^^^ expected fn pointer, found fn item + | + = note: expected fn pointer `for<'a> fn(_, Python<'a>) -> Result, PyErr>` + found fn item `for<'a> fn(String, Python<'a>) -> pyo3::Bound<'a, PyAny> {into_ref}` From a288c60ade3d2f9b86a261d99950f383d586af7e Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:53:06 +0100 Subject: [PATCH 2/2] unify to `into_py_with` by taking a `Cow<'_, T>` --- guide/src/conversions/traits.md | 19 ++++----- pyo3-macros-backend/src/attributes.rs | 2 - pyo3-macros-backend/src/intopyobject.rs | 55 ++++++++----------------- tests/test_intopyobject.rs | 23 +++++------ tests/ui/invalid_intopy_derive.rs | 18 -------- tests/ui/invalid_intopy_derive.stderr | 26 ++---------- tests/ui/invalid_intopy_with.rs | 14 +------ tests/ui/invalid_intopy_with.stderr | 28 ++++++------- 8 files changed, 57 insertions(+), 128 deletions(-) mode change 100644 => 100755 guide/src/conversions/traits.md mode change 100644 => 100755 pyo3-macros-backend/src/attributes.rs mode change 100644 => 100755 pyo3-macros-backend/src/intopyobject.rs mode change 100644 => 100755 tests/test_intopyobject.rs mode change 100644 => 100755 tests/ui/invalid_intopy_derive.rs mode change 100644 => 100755 tests/ui/invalid_intopy_with.rs mode change 100644 => 100755 tests/ui/invalid_intopy_with.stderr diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md old mode 100644 new mode 100755 index c1028f5110e..6923624f4f6 --- a/guide/src/conversions/traits.md +++ b/guide/src/conversions/traits.md @@ -607,30 +607,27 @@ Additionally `IntoPyObject` can be derived for a reference to a struct or enum u `IntoPyObjectRef` derive macro. All the same rules from above apply as well. ##### `#[derive(IntoPyObject)]`/`#[derive(IntoPyObjectRef)]` Field Attributes -- `pyo3(into_py_with = ...)`/`pyo3(into_py_with_ref = ...)` +- `pyo3(into_py_with = ...)` - apply a custom function to convert the field from Rust into Python. - the argument must be the function indentifier - - the function signature must be `fn(T, Python<'_>) -> PyResult>`/`fn<'py>(&T, Python<'py>) -> PyResult>` where `T` is the Rust type of the argument. + - the function signature must be `fn(Cow<'_, T>, Python<'py>) -> PyResult>` where `T` is the Rust type of the argument. ```rust # use pyo3::prelude::*; # use pyo3::IntoPyObjectExt; + # use std::borrow::Cow; + #[derive(Clone)] struct NotIntoPy(usize); #[derive(IntoPyObject, IntoPyObjectRef)] struct MyStruct { - #[pyo3(into_py_with = convert, into_py_with_ref = convert_ref)] + #[pyo3(into_py_with = convert)] not_into_py: NotIntoPy, } - /// Convert `NotIntoPy` into Python by value - fn convert(NotIntoPy(i): NotIntoPy, py: Python<'_>) -> PyResult> { - i.into_bound_py_any(py) - } - - /// Convert `NotIntoPy` into Python by reference - fn convert_ref<'py>(&NotIntoPy(i): &NotIntoPy, py: Python<'py>) -> PyResult> { - i.into_bound_py_any(py) + /// Convert `NotIntoPy` into Python + fn convert<'py>(not_into_py: Cow<'_, NotIntoPy>, py: Python<'py>) -> PyResult> { + not_into_py.0.into_bound_py_any(py) } ``` diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs old mode 100644 new mode 100755 index 589c853922b..c0591d74868 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -26,7 +26,6 @@ pub mod kw { syn::custom_keyword!(get_all); syn::custom_keyword!(hash); syn::custom_keyword!(into_py_with); - syn::custom_keyword!(into_py_with_ref); syn::custom_keyword!(item); syn::custom_keyword!(from_item_all); syn::custom_keyword!(mapping); @@ -353,7 +352,6 @@ impl ToTokens for OptionalKeywordAttribute { pub type FromPyWithAttribute = KeywordAttribute>; pub type IntoPyWithAttribute = KeywordAttribute; -pub type IntoPyWithRefAttribute = KeywordAttribute; pub type DefaultAttribute = OptionalKeywordAttribute; diff --git a/pyo3-macros-backend/src/intopyobject.rs b/pyo3-macros-backend/src/intopyobject.rs old mode 100644 new mode 100755 index 8fbc6cd4a65..787d1dd6259 --- a/pyo3-macros-backend/src/intopyobject.rs +++ b/pyo3-macros-backend/src/intopyobject.rs @@ -1,6 +1,4 @@ -use crate::attributes::{ - self, get_pyo3_options, CrateAttribute, IntoPyWithAttribute, IntoPyWithRefAttribute, -}; +use crate::attributes::{self, get_pyo3_options, CrateAttribute, IntoPyWithAttribute}; use crate::utils::Ctx; use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote, quote_spanned}; @@ -92,7 +90,6 @@ impl ItemOption { enum FieldAttribute { Item(ItemOption), IntoPyWith(IntoPyWithAttribute), - IntoPyWithRef(IntoPyWithRefAttribute), } impl Parse for FieldAttribute { @@ -124,8 +121,6 @@ impl Parse for FieldAttribute { } } else if lookahead.peek(attributes::kw::into_py_with) { input.parse().map(FieldAttribute::IntoPyWith) - } else if lookahead.peek(attributes::kw::into_py_with_ref) { - input.parse().map(FieldAttribute::IntoPyWithRef) } else { Err(lookahead.error()) } @@ -136,7 +131,6 @@ impl Parse for FieldAttribute { struct FieldAttributes { item: Option, into_py_with: Option, - into_py_with_ref: Option, } impl FieldAttributes { @@ -170,7 +164,6 @@ impl FieldAttributes { match option { FieldAttribute::Item(item) => set_option!(item), FieldAttribute::IntoPyWith(into_py_with) => set_option!(into_py_with), - FieldAttribute::IntoPyWithRef(into_py_with_ref) => set_option!(into_py_with_ref), } Ok(()) } @@ -195,13 +188,11 @@ struct NamedStructField<'a> { field: &'a syn::Field, item: Option, into_py_with: Option, - into_py_with_ref: Option, } struct TupleStructField<'a> { field: &'a syn::Field, into_py_with: Option, - into_py_with_ref: Option, } /// Container Style @@ -260,7 +251,6 @@ impl<'a, const REF: bool> Container<'a, REF> { Ok(TupleStructField { field, into_py_with: attrs.into_py_with, - into_py_with_ref: attrs.into_py_with_ref }) }) .collect::>>()?; @@ -270,16 +260,11 @@ impl<'a, const REF: bool> Container<'a, REF> { let TupleStructField { field, into_py_with, - into_py_with_ref, } = tuple_fields.pop().unwrap(); ensure_spanned!( into_py_with.is_none(), into_py_with.span() => "`into_py_with` is not permitted on `transparent` structs" ); - ensure_spanned!( - into_py_with_ref.is_none(), - into_py_with_ref.span() => "`into_py_with_ref` is not permitted on `transparent` structs" - ); ContainerType::TupleNewtype(field) } else if options.transparent.is_some() { bail_spanned!( @@ -306,10 +291,6 @@ impl<'a, const REF: bool> Container<'a, REF> { attrs.into_py_with.is_none(), attrs.into_py_with.span() => "`into_py_with` is not permitted on `transparent` structs or variants" ); - ensure_spanned!( - attrs.into_py_with_ref.is_none(), - attrs.into_py_with_ref.span() => "`into_py_with_ref` is not permitted on `transparent` structs or variants" - ); ContainerType::StructNewtype(field) } else { let struct_fields = named @@ -328,7 +309,6 @@ impl<'a, const REF: bool> Container<'a, REF> { field, item: attrs.item, into_py_with: attrs.into_py_with, - into_py_with_ref: attrs.into_py_with_ref, }) }) .collect::>>()?; @@ -431,16 +411,16 @@ impl<'a, const REF: bool> Container<'a, REF> { .map(|item| item.value()) .unwrap_or_else(|| f.ident.unraw().to_string()); let value = Ident::new(&format!("arg{i}"), f.field.ty.span()); - let expr_path = if REF { - f.into_py_with_ref.as_ref().map(|i|&i.value) - } else { - f.into_py_with.as_ref().map(|i|&i.value) - }; - if let Some(expr_path) = expr_path { + if let Some(expr_path) = f.into_py_with.as_ref().map(|i|&i.value) { + let cow = if REF { + quote!(::std::borrow::Cow::Borrowed(#value)) + } else { + quote!(::std::borrow::Cow::Owned(#value)) + }; quote! { - let into_py_with: fn(_, #pyo3_path::Python<'_>) -> #pyo3_path::PyResult<#pyo3_path::Bound<'_, #pyo3_path::PyAny>> = #expr_path; - #pyo3_path::types::PyDictMethods::set_item(&dict, #key, into_py_with(#value, py)?)?; + let into_py_with: fn(::std::borrow::Cow<'_, _>, #pyo3_path::Python<'py>) -> #pyo3_path::PyResult<#pyo3_path::Bound<'py, #pyo3_path::PyAny>> = #expr_path; + #pyo3_path::types::PyDictMethods::set_item(&dict, #key, into_py_with(#cow, py)?)?; } } else { quote! { @@ -483,16 +463,17 @@ impl<'a, const REF: bool> Container<'a, REF> { .map(|(i, f)| { let ty = &f.field.ty; let value = Ident::new(&format!("arg{i}"), f.field.ty.span()); - let expr_path = if REF { - f.into_py_with_ref.as_ref().map(|i|&i.value) - } else { - f.into_py_with.as_ref().map(|i|&i.value) - }; - if let Some(expr_path) = expr_path { + + if let Some(expr_path) = f.into_py_with.as_ref().map(|i|&i.value) { + let cow = if REF { + quote!(::std::borrow::Cow::Borrowed(#value)) + } else { + quote!(::std::borrow::Cow::Owned(#value)) + }; quote_spanned! { ty.span() => { - let into_py_with: fn(_, #pyo3_path::Python<'_>) -> #pyo3_path::PyResult<#pyo3_path::Bound<'_, #pyo3_path::PyAny>> = #expr_path; - into_py_with(#value, py)? + let into_py_with: fn(::std::borrow::Cow<'_, _>, #pyo3_path::Python<'py>) -> #pyo3_path::PyResult<#pyo3_path::Bound<'py, #pyo3_path::PyAny>> = #expr_path; + into_py_with(#cow, py)? }, } } else { diff --git a/tests/test_intopyobject.rs b/tests/test_intopyobject.rs old mode 100644 new mode 100755 index 4335c55ecd7..8e758c636cd --- a/tests/test_intopyobject.rs +++ b/tests/test_intopyobject.rs @@ -150,14 +150,14 @@ fn test_transparent_tuple_struct() { }); } -fn phantom_into_py( - _: std::marker::PhantomData, - py: Python<'_>, -) -> PyResult> { +fn phantom_into_py<'py, T>( + _: std::borrow::Cow<'_, std::marker::PhantomData>, + py: Python<'py>, +) -> PyResult> { std::any::type_name::().into_bound_py_any(py) } -#[derive(Debug, IntoPyObject)] +#[derive(Debug, IntoPyObject, IntoPyObjectRef)] pub enum Foo<'py> { TupleVar( usize, @@ -218,16 +218,15 @@ pub struct Zap { #[pyo3(item)] name: String, - #[pyo3(into_py_with = zap_into_py, into_py_with_ref = zap_into_py_ref, item("my_object"))] + #[pyo3(into_py_with = zap_into_py, item("my_object"))] some_object_length: usize, } -fn zap_into_py(len: usize, py: Python<'_>) -> PyResult> { - Ok(PyList::new(py, 1..len + 1)?.into_any()) -} - -fn zap_into_py_ref<'py>(&len: &usize, py: Python<'py>) -> PyResult> { - Ok(PyList::new(py, 1..len + 1)?.into_any()) +fn zap_into_py<'py>( + len: std::borrow::Cow<'_, usize>, + py: Python<'py>, +) -> PyResult> { + Ok(PyList::new(py, 1..*len + 1)?.into_any()) } #[test] diff --git a/tests/ui/invalid_intopy_derive.rs b/tests/ui/invalid_intopy_derive.rs old mode 100644 new mode 100755 index 3bf1de4a050..b609a740e56 --- a/tests/ui/invalid_intopy_derive.rs +++ b/tests/ui/invalid_intopy_derive.rs @@ -124,10 +124,6 @@ struct StructTransparentIntoPyWithRef { #[pyo3(transparent)] struct TupleTransparentIntoPyWith(#[pyo3(into_py_with = into)] String); -#[derive(IntoPyObjectRef)] -#[pyo3(transparent)] -struct TupleTransparentIntoPyWithRef(#[pyo3(into_py_with_ref = into_ref)] String); - #[derive(IntoPyObject)] enum EnumTupleIntoPyWith { TransparentTuple(#[pyo3(into_py_with = into)] usize), @@ -142,18 +138,4 @@ enum EnumStructIntoPyWith { }, } -#[derive(IntoPyObjectRef)] -enum EnumTupleIntoPyWithRef { - TransparentTuple(#[pyo3(into_py_with_ref = into_ref)] usize), -} - -#[derive(IntoPyObjectRef)] -enum EnumStructIntoPyWithRef { - #[pyo3(transparent)] - TransparentStruct { - #[pyo3(into_py_with_ref = into_ref)] - a: usize, - }, -} - fn main() {} diff --git a/tests/ui/invalid_intopy_derive.stderr b/tests/ui/invalid_intopy_derive.stderr index 5d9baec096a..4c04f8002a0 100644 --- a/tests/ui/invalid_intopy_derive.stderr +++ b/tests/ui/invalid_intopy_derive.stderr @@ -144,32 +144,14 @@ error: `into_py_with` is not permitted on `transparent` structs 125 | struct TupleTransparentIntoPyWith(#[pyo3(into_py_with = into)] String); | ^^^^^^^^^^^^ -error: `into_py_with_ref` is not permitted on `transparent` structs - --> tests/ui/invalid_intopy_derive.rs:129:45 - | -129 | struct TupleTransparentIntoPyWithRef(#[pyo3(into_py_with_ref = into_ref)] String); - | ^^^^^^^^^^^^^^^^ - error: `into_py_with` is not permitted on `transparent` structs - --> tests/ui/invalid_intopy_derive.rs:133:29 + --> tests/ui/invalid_intopy_derive.rs:129:29 | -133 | TransparentTuple(#[pyo3(into_py_with = into)] usize), +129 | TransparentTuple(#[pyo3(into_py_with = into)] usize), | ^^^^^^^^^^^^ error: `into_py_with` is not permitted on `transparent` structs or variants - --> tests/ui/invalid_intopy_derive.rs:140:16 + --> tests/ui/invalid_intopy_derive.rs:136:16 | -140 | #[pyo3(into_py_with = into)] +136 | #[pyo3(into_py_with = into)] | ^^^^^^^^^^^^ - -error: `into_py_with_ref` is not permitted on `transparent` structs - --> tests/ui/invalid_intopy_derive.rs:147:29 - | -147 | TransparentTuple(#[pyo3(into_py_with_ref = into_ref)] usize), - | ^^^^^^^^^^^^^^^^ - -error: `into_py_with_ref` is not permitted on `transparent` structs or variants - --> tests/ui/invalid_intopy_derive.rs:154:16 - | -154 | #[pyo3(into_py_with_ref = into_ref)] - | ^^^^^^^^^^^^^^^^ diff --git a/tests/ui/invalid_intopy_with.rs b/tests/ui/invalid_intopy_with.rs old mode 100644 new mode 100755 index bf34701190d..7cc910f57d8 --- a/tests/ui/invalid_intopy_with.rs +++ b/tests/ui/invalid_intopy_with.rs @@ -1,22 +1,12 @@ use pyo3::{IntoPyObject, IntoPyObjectRef}; -#[derive(IntoPyObject)] +#[derive(IntoPyObject, IntoPyObjectRef)] struct InvalidIntoPyWithFn { #[pyo3(into_py_with = into)] inner: String, } -#[derive(IntoPyObjectRef)] -struct InvalidIntoPyWithRefFn { - #[pyo3(into_py_with_ref = into_ref)] - inner: String, -} - -fn into(_a: String) -> pyo3::Py { - todo!() -} - -fn into_ref(_a: String, _py: pyo3::Python<'_>) -> pyo3::Bound<'_, pyo3::PyAny> { +fn into(_a: String, _py: pyo3::Python<'_>) -> pyo3::PyResult> { todo!() } diff --git a/tests/ui/invalid_intopy_with.stderr b/tests/ui/invalid_intopy_with.stderr old mode 100644 new mode 100755 index a45f8860f8b..bfa3e6ec274 --- a/tests/ui/invalid_intopy_with.stderr +++ b/tests/ui/invalid_intopy_with.stderr @@ -1,23 +1,23 @@ error[E0308]: mismatched types --> tests/ui/invalid_intopy_with.rs:5:27 | -3 | #[derive(IntoPyObject)] +3 | #[derive(IntoPyObject, IntoPyObjectRef)] | ------------ expected due to this 4 | struct InvalidIntoPyWithFn { 5 | #[pyo3(into_py_with = into)] - | ^^^^ incorrect number of function parameters + | ^^^^ expected fn pointer, found fn item | - = note: expected fn pointer `for<'a> fn(_, Python<'a>) -> Result, PyErr>` - found fn item `fn(String) -> Py {into}` + = note: expected fn pointer `for<'a> fn(Cow<'a, _>, Python<'py>) -> Result, PyErr>` + found fn item `for<'a> fn(String, Python<'a>) -> Result, PyErr> {into}` error[E0308]: mismatched types - --> tests/ui/invalid_intopy_with.rs:11:31 - | -9 | #[derive(IntoPyObjectRef)] - | --------------- expected due to this -10 | struct InvalidIntoPyWithRefFn { -11 | #[pyo3(into_py_with_ref = into_ref)] - | ^^^^^^^^ expected fn pointer, found fn item - | - = note: expected fn pointer `for<'a> fn(_, Python<'a>) -> Result, PyErr>` - found fn item `for<'a> fn(String, Python<'a>) -> pyo3::Bound<'a, PyAny> {into_ref}` + --> tests/ui/invalid_intopy_with.rs:5:27 + | +3 | #[derive(IntoPyObject, IntoPyObjectRef)] + | --------------- expected due to this +4 | struct InvalidIntoPyWithFn { +5 | #[pyo3(into_py_with = into)] + | ^^^^ expected fn pointer, found fn item + | + = note: expected fn pointer `for<'a> fn(Cow<'a, _>, Python<'py>) -> Result, PyErr>` + found fn item `for<'a> fn(String, Python<'a>) -> Result, PyErr> {into}`