From 773a371ba572ab3b1d40398736717bc67da4bcd0 Mon Sep 17 00:00:00 2001 From: messense Date: Thu, 18 Mar 2021 00:04:25 +0800 Subject: [PATCH 1/3] Add tuple and unit struct support for pyclass macro --- CHANGELOG.md | 1 + pyo3-macros-backend/src/pyclass.rs | 23 ++++++++++----- tests/test_class_basics.rs | 27 ++++++++++++++++++ tests/test_class_new.rs | 45 ++++++++++++++++++++++++++++++ tests/test_getter_setter.rs | 28 +++++++++++++++++++ 5 files changed, 117 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1fd8677cc5..cd0e02ca293 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add FFI definition `PyCFunction_CheckExact` for Python 3.9 and later. [#1425](https://github.com/PyO3/pyo3/pull/1425) - Add FFI definition `Py_IS_TYPE`. [#1429](https://github.com/PyO3/pyo3/pull/1429) - Add FFI definition `_Py_InitializeMain`. [#1473](https://github.com/PyO3/pyo3/pull/1473) +- Add tuple and unit struct support for `#[pyclass]` macro. [#1504](https://github.com/PyO3/pyo3/pull/1504) ### Changed - Change `PyTimeAcces::get_fold()` to return a `bool` instead of a `u8`. [#1397](https://github.com/PyO3/pyo3/pull/1397) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index e1812419c8d..2f2a3f5d398 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -179,15 +179,24 @@ pub fn build_py_class( class.generics.span() => "#[pyclass] cannot have generic parameters" ); - if let syn::Fields::Named(fields) = &mut class.fields { - for field in fields.named.iter_mut() { - let field_descs = parse_descriptors(field)?; - if !field_descs.is_empty() { - descriptors.push((field.clone(), field_descs)); + match &mut class.fields { + syn::Fields::Named(fields) => { + for field in fields.named.iter_mut() { + let field_descs = parse_descriptors(field)?; + if !field_descs.is_empty() { + descriptors.push((field.clone(), field_descs)); + } } } - } else { - bail_spanned!(class.fields.span() => "#[pyclass] can only be used with C-style structs"); + syn::Fields::Unnamed(fields) => { + for field in fields.unnamed.iter_mut() { + let field_descs = parse_descriptors(field)?; + if !field_descs.is_empty() { + descriptors.push((field.clone(), field_descs)); + } + } + } + syn::Fields::Unit => { /* No fields for unit struct */ } } impl_class(&class.ident, &attr, doc, descriptors, methods_type) diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index d84b09deb33..fb8b74d55ff 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -18,6 +18,20 @@ fn empty_class() { py_assert!(py, typeobj, "typeobj.__name__ == 'EmptyClass'"); } +#[pyclass] +struct UnitClass; + +#[test] +fn unit_class() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + // By default, don't allow creating instances from python. + assert!(typeobj.call((), None).is_err()); + + py_assert!(py, typeobj, "typeobj.__name__ == 'UnitClass'"); +} + /// Line1 ///Line2 /// Line3 @@ -289,3 +303,16 @@ fn test_pymethods_from_py_with() { ); }) } + +#[pyclass] +struct TupleClass(i32); + +#[test] +fn test_tuple_struct_class() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + assert!(typeobj.call((), None).is_err()); + + py_assert!(py, typeobj, "typeobj.__name__ == 'TupleClass'"); +} diff --git a/tests/test_class_new.rs b/tests/test_class_new.rs index b98f29b7999..3b8c2c6cdba 100644 --- a/tests/test_class_new.rs +++ b/tests/test_class_new.rs @@ -24,6 +24,51 @@ fn empty_class_with_new() { .is_ok()); } +#[pyclass] +struct UnitClassWithNew; + +#[pymethods] +impl UnitClassWithNew { + #[new] + fn new() -> Self { + Self + } +} + +#[test] +fn unit_class_with_new() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + assert!(typeobj + .call((), None) + .unwrap() + .cast_as::>() + .is_ok()); +} + +#[pyclass] +struct TupleClassWithNew(i32); + +#[pymethods] +impl TupleClassWithNew { + #[new] + fn new(arg: i32) -> Self { + Self(arg) + } +} + +#[test] +fn tuple_class_with_new() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + let wrp = typeobj.call((42,), None).unwrap(); + let obj = wrp.cast_as::>().unwrap(); + let obj_ref = obj.borrow(); + assert_eq!(obj_ref.0, 42); +} + #[pyclass] #[derive(Debug)] struct NewWithOneArg { diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index 4fe15ba7cc9..943f616b0e3 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -130,3 +130,31 @@ fn ref_getter_setter() { py_run!(py, inst, "assert inst.num == 10"); py_run!(py, inst, "inst.num = 20; assert inst.num == 20"); } + +#[pyclass] +struct TupleClassGetterSetter(i32); + +#[pymethods] +impl TupleClassGetterSetter { + #[getter(num)] + fn get_num(&self) -> i32 { + self.0 + } + + #[setter(num)] + fn set_num(&mut self, value: i32) { + self.0 = value; + } +} + +#[test] +fn tuple_struct_getter_setter() { + let gil = Python::acquire_gil(); + let py = gil.python(); + + let inst = Py::new(py, TupleClassGetterSetter(10)).unwrap(); + + py_run!(py, inst, "assert inst.num == 10"); + py_run!(py, inst, "inst.num = 20"); + py_run!(py, inst, "assert inst.num == 20"); +} From 2cec240b0ebd1dc9082a4ae7fca46c85682ac0c0 Mon Sep 17 00:00:00 2001 From: messense Date: Thu, 18 Mar 2021 18:27:18 +0800 Subject: [PATCH 2/3] Ban pyo3(get, set) on tuple struct field --- pyo3-macros-backend/src/pyclass.rs | 25 ++++++++++++++----------- tests/ui/invalid_property_args.rs | 3 +++ tests/ui/invalid_property_args.stderr | 6 ++++++ 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 2f2a3f5d398..f570850dae7 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -506,18 +506,21 @@ fn impl_descriptors( .flat_map(|(field, fns)| { fns.iter() .map(|desc| { - let name = field.ident.as_ref().unwrap().unraw(); - let doc = utils::get_doc(&field.attrs, None, true) - .unwrap_or_else(|_| syn::LitStr::new(&name.to_string(), name.span())); - let property_type = PropertyType::Descriptor(&field); - match desc { - FnType::Getter(self_ty) => { - impl_py_getter_def(cls, property_type, self_ty, &name, &doc) - } - FnType::Setter(self_ty) => { - impl_py_setter_def(cls, property_type, self_ty, &name, &doc) + if let Some(name) = field.ident.as_ref().map(|ident| ident.unraw()) { + let doc = utils::get_doc(&field.attrs, None, true) + .unwrap_or_else(|_| syn::LitStr::new(&name.to_string(), name.span())); + let property_type = PropertyType::Descriptor(&field); + match desc { + FnType::Getter(self_ty) => { + impl_py_getter_def(cls, property_type, self_ty, &name, &doc) + } + FnType::Setter(self_ty) => { + impl_py_setter_def(cls, property_type, self_ty, &name, &doc) + } + _ => unreachable!(), } - _ => unreachable!(), + } else { + bail_spanned!(field.span() => "get/set are not supported on tuple struct field"); } }) .collect::>>() diff --git a/tests/ui/invalid_property_args.rs b/tests/ui/invalid_property_args.rs index 5982add7e22..20dc1218da0 100644 --- a/tests/ui/invalid_property_args.rs +++ b/tests/ui/invalid_property_args.rs @@ -24,4 +24,7 @@ impl ClassWithSetter { fn setter_with_too_many_args(&mut self, py: Python, foo: u32, bar: u32) {} } +#[pyclass] +struct TupleGetterSetter(#[pyo3(get, set)] i32); + fn main() {} diff --git a/tests/ui/invalid_property_args.stderr b/tests/ui/invalid_property_args.stderr index c4d953254a3..8e61fd289e7 100644 --- a/tests/ui/invalid_property_args.stderr +++ b/tests/ui/invalid_property_args.stderr @@ -15,3 +15,9 @@ error: setter function can have at most two arguments ([pyo3::Python,] and value | 24 | fn setter_with_too_many_args(&mut self, py: Python, foo: u32, bar: u32) {} | ^^^ + +error: get/set are not supported on tuple struct field + --> $DIR/invalid_property_args.rs:28:44 + | +28 | struct TupleGetterSetter(#[pyo3(get, set)] i32); + | ^^^ From 1c572942149a579b35f8c511fea73b726ac16603 Mon Sep 17 00:00:00 2001 From: messense Date: Thu, 18 Mar 2021 21:09:46 +0800 Subject: [PATCH 3/3] Use Python::with_gil and py_assert Co-authored-by: Yuji Kanagawa --- tests/test_class_basics.rs | 22 +++++++++++----------- tests/test_class_new.rs | 30 +++++++++++++++--------------- tests/test_getter_setter.rs | 4 ++-- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index fb8b74d55ff..64ee36b90de 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -23,13 +23,13 @@ struct UnitClass; #[test] fn unit_class() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let typeobj = py.get_type::(); - // By default, don't allow creating instances from python. - assert!(typeobj.call((), None).is_err()); + Python::with_gil(|py| { + let typeobj = py.get_type::(); + // By default, don't allow creating instances from python. + assert!(typeobj.call((), None).is_err()); - py_assert!(py, typeobj, "typeobj.__name__ == 'UnitClass'"); + py_assert!(py, typeobj, "typeobj.__name__ == 'UnitClass'"); + }); } /// Line1 @@ -309,10 +309,10 @@ struct TupleClass(i32); #[test] fn test_tuple_struct_class() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let typeobj = py.get_type::(); - assert!(typeobj.call((), None).is_err()); + Python::with_gil(|py| { + let typeobj = py.get_type::(); + assert!(typeobj.call((), None).is_err()); - py_assert!(py, typeobj, "typeobj.__name__ == 'TupleClass'"); + py_assert!(py, typeobj, "typeobj.__name__ == 'TupleClass'"); + }); } diff --git a/tests/test_class_new.rs b/tests/test_class_new.rs index 3b8c2c6cdba..65e22bdd661 100644 --- a/tests/test_class_new.rs +++ b/tests/test_class_new.rs @@ -37,14 +37,14 @@ impl UnitClassWithNew { #[test] fn unit_class_with_new() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let typeobj = py.get_type::(); - assert!(typeobj - .call((), None) - .unwrap() - .cast_as::>() - .is_ok()); + Python::with_gil(|py| { + let typeobj = py.get_type::(); + assert!(typeobj + .call((), None) + .unwrap() + .cast_as::>() + .is_ok()); + }); } #[pyclass] @@ -60,13 +60,13 @@ impl TupleClassWithNew { #[test] fn tuple_class_with_new() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let typeobj = py.get_type::(); - let wrp = typeobj.call((42,), None).unwrap(); - let obj = wrp.cast_as::>().unwrap(); - let obj_ref = obj.borrow(); - assert_eq!(obj_ref.0, 42); + Python::with_gil(|py| { + let typeobj = py.get_type::(); + let wrp = typeobj.call((42,), None).unwrap(); + let obj = wrp.cast_as::>().unwrap(); + let obj_ref = obj.borrow(); + assert_eq!(obj_ref.0, 42); + }); } #[pyclass] diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index 943f616b0e3..53e99803bb7 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -154,7 +154,7 @@ fn tuple_struct_getter_setter() { let inst = Py::new(py, TupleClassGetterSetter(10)).unwrap(); - py_run!(py, inst, "assert inst.num == 10"); + py_assert!(py, inst, "inst.num == 10"); py_run!(py, inst, "inst.num = 20"); - py_run!(py, inst, "assert inst.num == 20"); + py_assert!(py, inst, "inst.num == 20"); }