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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.

Copy link

@austin362667 austin362667 Jun 4, 2024

Choose a reason for hiding this comment

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

Hi @davidhewitt and @messense This's nice! And as a local wrapper of external structure, I think we still can't get them work with nested getter and setter, which is get_all and set_all?

#287

}
})
.collect::<Vec<syn::Result<TokenStream>>>()
Expand Down
27 changes: 27 additions & 0 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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);
| ^^^