luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

frompyobject: fix `from_py_with` ignored for transparent structs

This commit is contained in:
David Hewitt 2022-06-08 06:41:31 +01:00
parent da5b9814cc
commit 7c56a03d64
5 changed files with 162 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix FFI definitions `_PyDateTime_BaseTime` and `_PyDateTime_BaseDateTime` incorrectly having `fold` member. [#2432](https://github.com/PyO3/pyo3/pull/2432) - Fix FFI definitions `_PyDateTime_BaseTime` and `_PyDateTime_BaseDateTime` incorrectly having `fold` member. [#2432](https://github.com/PyO3/pyo3/pull/2432)
- Fix FFI definitions `PyTypeObject`. `PyHeapTypeObject`, and `PyCFunctionObject` having incorrect members on PyPy 3.9. [#2428](https://github.com/PyO3/pyo3/pull/2428) - Fix FFI definitions `PyTypeObject`. `PyHeapTypeObject`, and `PyCFunctionObject` having incorrect members on PyPy 3.9. [#2428](https://github.com/PyO3/pyo3/pull/2428)
- Fix FFI definition `PyGetSetDef` to have `*const c_char` for `doc` member (not `*mut c_char`). [#2439](https://github.com/PyO3/pyo3/pull/2439) - Fix FFI definition `PyGetSetDef` to have `*const c_char` for `doc` member (not `*mut c_char`). [#2439](https://github.com/PyO3/pyo3/pull/2439)
- Fix `#[pyo3(from_py_with = "...")]` being ignored for 1-element tuple structs and transparent structs. [#2440](https://github.com/PyO3/pyo3/pull/2440)
## [0.16.5] - 2022-05-15 ## [0.16.5] - 2022-05-15

192
pyo3-macros-backend/src/frompyobject.rs
View File

@ -14,7 +14,6 @@ use syn::{
}; };
/// Describes derivation input of an enum. /// Describes derivation input of an enum.
#[derive(Debug)]
struct Enum<'a> { struct Enum<'a> {
enum_ident: &'a Ident, enum_ident: &'a Ident,
variants: Vec<Container<'a>>, variants: Vec<Container<'a>>,
@ -86,34 +85,42 @@ impl<'a> Enum<'a> {
} }
} }
struct NamedStructField<'a> {
ident: &'a syn::Ident,
getter: Option<FieldGetter>,
from_py_with: Option<FromPyWithAttribute>,
}
struct TupleStructField {
from_py_with: Option<FromPyWithAttribute>,
}
/// Container Style /// Container Style
/// ///
/// Covers Structs, Tuplestructs and corresponding Newtypes. /// Covers Structs, Tuplestructs and corresponding Newtypes.
#[derive(Debug)]
enum ContainerType<'a> { enum ContainerType<'a> {
/// Struct Container, e.g. `struct Foo { a: String }` /// Struct Container, e.g. `struct Foo { a: String }`
/// ///
/// Variant contains the list of field identifiers and the corresponding extraction call. /// Variant contains the list of field identifiers and the corresponding extraction call.
Struct(Vec<(&'a Ident, FieldPyO3Attributes)>), Struct(Vec<NamedStructField<'a>>),
/// Newtype struct container, e.g. `#[transparent] struct Foo { a: String }` /// Newtype struct container, e.g. `#[transparent] struct Foo { a: String }`
/// ///
/// The field specified by the identifier is extracted directly from the object. /// The field specified by the identifier is extracted directly from the object.
StructNewtype(&'a Ident), StructNewtype(&'a syn::Ident, Option<FromPyWithAttribute>),
/// Tuple struct, e.g. `struct Foo(String)`. /// Tuple struct, e.g. `struct Foo(String)`.
/// ///
/// Variant contains a list of conversion methods for each of the fields that are directly /// Variant contains a list of conversion methods for each of the fields that are directly
/// extracted from the tuple. /// extracted from the tuple.
Tuple(Vec<FieldPyO3Attributes>), Tuple(Vec<TupleStructField>),
/// Tuple newtype, e.g. `#[transparent] struct Foo(String)` /// Tuple newtype, e.g. `#[transparent] struct Foo(String)`
/// ///
/// The wrapped field is directly extracted from the object. /// The wrapped field is directly extracted from the object.
TupleNewtype, TupleNewtype(Option<FromPyWithAttribute>),
} }
/// Data container /// Data container
/// ///
/// Either describes a struct or an enum variant. /// Either describes a struct or an enum variant.
#[derive(Debug)]
struct Container<'a> { struct Container<'a> {
path: syn::Path, path: syn::Path,
ty: ContainerType<'a>, ty: ContainerType<'a>,
@ -125,55 +132,72 @@ impl<'a> Container<'a> {
/// ///
/// Fails if the variant has no fields or incompatible attributes. /// Fails if the variant has no fields or incompatible attributes.
fn new(fields: &'a Fields, path: syn::Path, options: ContainerOptions) -> Result<Self> { fn new(fields: &'a Fields, path: syn::Path, options: ContainerOptions) -> Result<Self> {
ensure_spanned!( let style = match fields {
!fields.is_empty(), Fields::Unnamed(unnamed) if !unnamed.unnamed.is_empty() => {
fields.span() => "cannot derive FromPyObject for empty structs and variants" let mut tuple_fields = unnamed
); .unnamed
if options.transparent { .iter()
ensure_spanned!( .map(|field| {
fields.len() == 1, let attrs = FieldPyO3Attributes::from_attrs(&field.attrs)?;
fields.span() => "transparent structs and variants can only have 1 field" ensure_spanned!(
); attrs.getter.is_none(),
} field.span() => "`getter` is not permitted on tuple struct elements."
let style = match (fields, options.transparent) { );
(Fields::Unnamed(_), true) => ContainerType::TupleNewtype, Ok(TupleStructField {
(Fields::Unnamed(unnamed), false) => match unnamed.unnamed.len() { from_py_with: attrs.from_py_with,
1 => ContainerType::TupleNewtype, })
_ => { })
let fields = unnamed .collect::<Result<Vec<_>>>()?;
.unnamed
.iter()
.map(|field| FieldPyO3Attributes::from_attrs(&field.attrs))
.collect::<Result<Vec<_>>>()?;
ContainerType::Tuple(fields) if tuple_fields.len() == 1 {
// Always treat a 1-length tuple struct as "transparent", even without the
// explicit annotation.
let field = tuple_fields.pop().unwrap();
ContainerType::TupleNewtype(field.from_py_with)
} else if options.transparent {
bail_spanned!(
fields.span() => "transparent structs and variants can only have 1 field"
);
} else {
ContainerType::Tuple(tuple_fields)
} }
}, }
(Fields::Named(named), true) => { Fields::Named(named) if !named.named.is_empty() => {
let field = named let mut struct_fields = named
.named .named
.iter() .iter()
.next() .map(|field| {
.expect("Check for len 1 is done above"); let ident = field
let ident = field .ident
.ident .as_ref()
.as_ref() .expect("Named fields should have identifiers");
.expect("Named fields should have identifiers"); let attrs = FieldPyO3Attributes::from_attrs(&field.attrs)?;
ContainerType::StructNewtype(ident)
} Ok(NamedStructField {
(Fields::Named(named), false) => { ident,
let mut fields = Vec::new(); getter: attrs.getter,
for field in named.named.iter() { from_py_with: attrs.from_py_with,
let ident = field })
.ident })
.as_ref() .collect::<Result<Vec<_>>>()?;
.expect("Named fields should have identifiers"); if options.transparent {
let attrs = FieldPyO3Attributes::from_attrs(&field.attrs)?; ensure_spanned!(
fields.push((ident, attrs)) struct_fields.len() == 1,
fields.span() => "transparent structs and variants can only have 1 field"
);
let field = struct_fields.pop().unwrap();
ensure_spanned!(
field.getter.is_none(),
field.ident.span() => "`transparent` structs may not have a `getter` for the inner field"
);
ContainerType::StructNewtype(field.ident, field.from_py_with)
} else {
ContainerType::Struct(struct_fields)
} }
ContainerType::Struct(fields)
} }
(Fields::Unit, _) => unreachable!(), // covered by length check above _ => bail_spanned!(
fields.span() => "cannot derive FromPyObject for empty structs and variants"
),
}; };
let err_name = options.annotation.map_or_else( let err_name = options.annotation.map_or_else(
|| path.segments.last().unwrap().ident.to_string(), || path.segments.last().unwrap().ident.to_string(),
@ -202,39 +226,63 @@ impl<'a> Container<'a> {
/// Build derivation body for a struct. /// Build derivation body for a struct.
fn build(&self) -> TokenStream { fn build(&self) -> TokenStream {
match &self.ty { match &self.ty {
ContainerType::StructNewtype(ident) => self.build_newtype_struct(Some(ident)), ContainerType::StructNewtype(ident, from_py_with) => {
ContainerType::TupleNewtype => self.build_newtype_struct(None), self.build_newtype_struct(Some(ident), from_py_with)
}
ContainerType::TupleNewtype(from_py_with) => {
self.build_newtype_struct(None, from_py_with)
}
ContainerType::Tuple(tups) => self.build_tuple_struct(tups), ContainerType::Tuple(tups) => self.build_tuple_struct(tups),
ContainerType::Struct(tups) => self.build_struct(tups), ContainerType::Struct(tups) => self.build_struct(tups),
} }
} }
fn build_newtype_struct(&self, field_ident: Option<&Ident>) -> TokenStream { fn build_newtype_struct(
&self,
field_ident: Option<&Ident>,
from_py_with: &Option<FromPyWithAttribute>,
) -> TokenStream {
let self_ty = &self.path; let self_ty = &self.path;
let struct_name = self.name(); let struct_name = self.name();
if let Some(ident) = field_ident { if let Some(ident) = field_ident {
let field_name = ident.to_string(); let field_name = ident.to_string();
quote!( match from_py_with {
::std::result::Result::Ok(#self_ty{ None => quote! {
#ident: _pyo3::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)? Ok(#self_ty {
}) #ident: _pyo3::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
) })
},
Some(FromPyWithAttribute {
value: expr_path, ..
}) => quote! {
Ok(#self_ty {
#ident: _pyo3::impl_::frompyobject::extract_struct_field_with(#expr_path, obj, #struct_name, #field_name)?
})
},
}
} else { } else {
quote!( match from_py_with {
_pyo3::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty) None => quote!(
) _pyo3::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
),
Some(FromPyWithAttribute {
value: expr_path, ..
}) => quote! (
_pyo3::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path, obj, #struct_name, 0).map(#self_ty)
),
}
} }
} }
fn build_tuple_struct(&self, tups: &[FieldPyO3Attributes]) -> TokenStream { fn build_tuple_struct(&self, struct_fields: &[TupleStructField]) -> TokenStream {
let self_ty = &self.path; let self_ty = &self.path;
let struct_name = &self.name(); let struct_name = &self.name();
let field_idents: Vec<_> = (0..tups.len()) let field_idents: Vec<_> = (0..struct_fields.len())
.into_iter() .into_iter()
.map(|i| format_ident!("arg{}", i)) .map(|i| format_ident!("arg{}", i))
.collect(); .collect();
let fields = tups.iter().zip(&field_idents).enumerate().map(|(index, (attrs, ident))| { let fields = struct_fields.iter().zip(&field_idents).enumerate().map(|(index, (field, ident))| {
match &attrs.from_py_with { match &field.from_py_with {
None => quote!( None => quote!(
_pyo3::impl_::frompyobject::extract_tuple_struct_field(#ident, #struct_name, #index)? _pyo3::impl_::frompyobject::extract_tuple_struct_field(#ident, #struct_name, #index)?
), ),
@ -253,13 +301,14 @@ impl<'a> Container<'a> {
) )
} }
fn build_struct(&self, tups: &[(&Ident, FieldPyO3Attributes)]) -> TokenStream { fn build_struct(&self, struct_fields: &[NamedStructField<'_>]) -> TokenStream {
let self_ty = &self.path; let self_ty = &self.path;
let struct_name = &self.name(); let struct_name = &self.name();
let mut fields: Punctuated<TokenStream, syn::Token![,]> = Punctuated::new(); let mut fields: Punctuated<TokenStream, syn::Token![,]> = Punctuated::new();
for (ident, attrs) in tups { for field in struct_fields {
let ident = &field.ident;
let field_name = ident.to_string(); let field_name = ident.to_string();
let getter = match &attrs.getter { let getter = match field.getter.as_ref().unwrap_or(&FieldGetter::GetAttr(None)) {
FieldGetter::GetAttr(Some(name)) => { FieldGetter::GetAttr(Some(name)) => {
quote!(getattr(_pyo3::intern!(obj.py(), #name))) quote!(getattr(_pyo3::intern!(obj.py(), #name)))
} }
@ -269,7 +318,7 @@ impl<'a> Container<'a> {
FieldGetter::GetItem(Some(key)) => quote!(get_item(#key)), FieldGetter::GetItem(Some(key)) => quote!(get_item(#key)),
FieldGetter::GetItem(None) => quote!(get_item(#field_name)), FieldGetter::GetItem(None) => quote!(get_item(#field_name)),
}; };
let extractor = match &attrs.from_py_with { let extractor = match &field.from_py_with {
None => { None => {
quote!(_pyo3::impl_::frompyobject::extract_struct_field(obj.#getter?, #struct_name, #field_name)?) quote!(_pyo3::impl_::frompyobject::extract_struct_field(obj.#getter?, #struct_name, #field_name)?)
} }
@ -297,7 +346,6 @@ struct ContainerOptions {
} }
/// Attributes for deriving FromPyObject scoped on containers. /// Attributes for deriving FromPyObject scoped on containers.
#[derive(Debug)]
enum ContainerPyO3Attribute { enum ContainerPyO3Attribute {
/// Treat the Container as a Wrapper, directly extract its fields from the input object. /// Treat the Container as a Wrapper, directly extract its fields from the input object.
Transparent(attributes::kw::transparent), Transparent(attributes::kw::transparent),
@ -365,7 +413,7 @@ impl ContainerOptions {
/// Attributes for deriving FromPyObject scoped on fields. /// Attributes for deriving FromPyObject scoped on fields.
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
struct FieldPyO3Attributes { struct FieldPyO3Attributes {
getter: FieldGetter, getter: Option<FieldGetter>,
from_py_with: Option<FromPyWithAttribute>, from_py_with: Option<FromPyWithAttribute>,
} }
@ -457,7 +505,7 @@ impl FieldPyO3Attributes {
} }
Ok(FieldPyO3Attributes { Ok(FieldPyO3Attributes {
getter: getter.unwrap_or(FieldGetter::GetAttr(None)), getter,
from_py_with, from_py_with,
}) })
} }

21
tests/test_frompyobject.rs
View File

@ -2,7 +2,7 @@
use pyo3::exceptions::PyValueError; use pyo3::exceptions::PyValueError;
use pyo3::prelude::*; use pyo3::prelude::*;
use pyo3::types::{PyDict, PyString, PyTuple}; use pyo3::types::{PyDict, PyList, PyString, PyTuple};
#[macro_use] #[macro_use]
mod common; mod common;
@ -546,8 +546,25 @@ fn test_from_py_with_enum() {
.expect("failed to create tuple"); .expect("failed to create tuple");
let zap = ZapEnum::extract(py_zap).unwrap(); let zap = ZapEnum::extract(py_zap).unwrap();
let expected_zap = ZapEnum::Zap(String::from("whatever"), 3usize); let expected_zap = ZapEnum::Zip(2);
assert_eq!(zap, expected_zap); assert_eq!(zap, expected_zap);
}); });
} }
#[derive(Debug, FromPyObject, PartialEq)]
#[pyo3(transparent)]
pub struct TransparentFromPyWith {
#[pyo3(from_py_with = "PyAny::len")]
len: usize,
}
#[test]
fn test_transparent_from_py_with() {
Python::with_gil(|py| {
let result = TransparentFromPyWith::extract(PyList::new(py, &[1, 2, 3])).unwrap();
let expected = TransparentFromPyWith { len: 3 };
assert_eq!(result, expected);
});
}

10
tests/ui/invalid_frompy_derive.rs
View File

@ -165,4 +165,14 @@ struct InvalidFromPyWithLiteral {
field: String, field: String,
} }
#[derive(FromPyObject)]
struct InvalidTupleGetter(#[pyo3(item("foo"))] String);
#[derive(FromPyObject)]
#[pyo3(transparent)]
struct InvalidTransparentWithGetter {
#[pyo3(item("foo"))]
field: String,
}
fn main() {} fn main() {}

12
tests/ui/invalid_frompy_derive.stderr
View File

@ -181,3 +181,15 @@ error: expected string literal
| |
164 | #[pyo3(from_py_with = func)] 164 | #[pyo3(from_py_with = func)]
| ^^^^ | ^^^^
error: `getter` is not permitted on tuple struct elements.
--> tests/ui/invalid_frompy_derive.rs:169:27
|
169 | struct InvalidTupleGetter(#[pyo3(item("foo"))] String);
| ^
error: `transparent` structs may not have a `getter` for the inner field
--> tests/ui/invalid_frompy_derive.rs:175:5
|
175 | field: String,
| ^^^^^