Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tuple and unit struct support for pyclass macro #1504

Merged
merged 3 commits into from Mar 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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)
Expand Down
48 changes: 30 additions & 18 deletions pyo3-macros-backend/src/pyclass.rs
Expand Up @@ -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)
Expand Down Expand Up @@ -497,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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious what the reason for disallowing get/set is? I was thinking that it should be possible, but a name would be required.

e.g. this seems a plausible combination to me:

#[pyclass]
struct PyFoo(
    #[pyo3(get, set, name = "inner", from_py_with = "pytimestamp_to_int")] i32,
);

I'm happy for us to create an issue to leave this for a follow-up PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was disallowed just to avoid proc-maco panic when user try to use get/set on it. We can certainly add support for it, I think we can do the follow-up after #1507 merged.

}
})
.collect::<Vec<syn::Result<TokenStream>>>()
Expand Down
27 changes: 27 additions & 0 deletions tests/test_class_basics.rs
Expand Up @@ -18,6 +18,20 @@ fn empty_class() {
py_assert!(py, typeobj, "typeobj.__name__ == 'EmptyClass'");
}

#[pyclass]
struct UnitClass;

#[test]
fn unit_class() {
Python::with_gil(|py| {
let typeobj = py.get_type::<UnitClass>();
// By default, don't allow creating instances from python.
assert!(typeobj.call((), None).is_err());
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


py_assert!(py, typeobj, "typeobj.__name__ == 'UnitClass'");
});
}

/// Line1
///Line2
/// Line3
Expand Down Expand Up @@ -289,3 +303,16 @@ fn test_pymethods_from_py_with() {
);
})
}

#[pyclass]
struct TupleClass(i32);

#[test]
fn test_tuple_struct_class() {
Python::with_gil(|py| {
let typeobj = py.get_type::<TupleClass>();
assert!(typeobj.call((), None).is_err());

py_assert!(py, typeobj, "typeobj.__name__ == 'TupleClass'");
});
}
45 changes: 45 additions & 0 deletions tests/test_class_new.rs
Expand Up @@ -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() {
Python::with_gil(|py| {
let typeobj = py.get_type::<UnitClassWithNew>();
assert!(typeobj
.call((), None)
.unwrap()
.cast_as::<PyCell<UnitClassWithNew>>()
.is_ok());
});
}

#[pyclass]
struct TupleClassWithNew(i32);

#[pymethods]
impl TupleClassWithNew {
#[new]
fn new(arg: i32) -> Self {
Self(arg)
}
}

#[test]
fn tuple_class_with_new() {
Python::with_gil(|py| {
let typeobj = py.get_type::<TupleClassWithNew>();
let wrp = typeobj.call((42,), None).unwrap();
let obj = wrp.cast_as::<PyCell<TupleClassWithNew>>().unwrap();
let obj_ref = obj.borrow();
assert_eq!(obj_ref.0, 42);
});
}

#[pyclass]
#[derive(Debug)]
struct NewWithOneArg {
Expand Down
28 changes: 28 additions & 0 deletions tests/test_getter_setter.rs
Expand Up @@ -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_assert!(py, inst, "inst.num == 10");
py_run!(py, inst, "inst.num = 20");
py_assert!(py, inst, "inst.num == 20");
}
3 changes: 3 additions & 0 deletions tests/ui/invalid_property_args.rs
Expand Up @@ -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() {}
6 changes: 6 additions & 0 deletions tests/ui/invalid_property_args.stderr
Expand Up @@ -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);
| ^^^