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.

Sign up for GitHub

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

Forward cfgs on pyclass fields to the method defs #2796

Merged
merged 4 commits into from
Dec 29, 2022
Merged

Forward cfgs on pyclass fields to the method defs #2796

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
44310ec
Forward cfgs on pyclass fields to the method defs
Dec 6, 2022
5198722
Add test
Dec 28, 2022
8f51142
Add changelog
Dec 28, 2022
530c5b3
Fix changelog
Dec 29, 2022
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
18 changes: 18 additions & 0 deletions pyo3-macros-backend/src/pymethod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,15 @@ pub fn impl_py_setter_def(
}
};

let mut cfg_attrs = TokenStream::new();
if let PropertyType::Descriptor { field, .. } = &property_type {
for attr in field.attrs.iter().filter(|attr| attr.path.is_ident("cfg")) {
attr.to_tokens(&mut cfg_attrs);
}
}

let associated_method = quote! {
#cfg_attrs
unsafe fn #wrapper_ident(
_py: _pyo3::Python<'_>,
_slf: *mut _pyo3::ffi::PyObject,
Expand All @@ -538,6 +546,7 @@ pub fn impl_py_setter_def(
};

let method_def = quote! {
#cfg_attrs
_pyo3::class::PyMethodDefType::Setter({
#deprecations
_pyo3::class::PySetterDef::new(
Expand Down Expand Up @@ -653,7 +662,15 @@ pub fn impl_py_getter_def(
}
};

let mut cfg_attrs = TokenStream::new();
if let PropertyType::Descriptor { field, .. } = &property_type {
for attr in field.attrs.iter().filter(|attr| attr.path.is_ident("cfg")) {
attr.to_tokens(&mut cfg_attrs);
}
}
Comment on lines +665 to +670
Copy link
Member

Choose a reason for hiding this comment

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

I suspect it probably works to use an Option instead of having the intermediate TokenStream buffer:

Suggested change
let mut cfg_attrs = TokenStream::new();
if let PropertyType::Descriptor { field, .. } = &property_type {
for attr in field.attrs.iter().filter(|attr| attr.path.is_ident("cfg")) {
attr.to_tokens(&mut cfg_attrs);
}
}
let mut cfg_attrs = None;
if let PropertyType::Descriptor { field, .. } = &property_type {
cfg_attrs = Some(field.attrs.iter().filter(|attr| attr.path.is_ident("cfg")));
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think I can collect into a tokenstream :(


let associated_method = quote! {
#cfg_attrs
unsafe fn #wrapper_ident(
_py: _pyo3::Python<'_>,
_slf: *mut _pyo3::ffi::PyObject
Expand All @@ -665,6 +682,7 @@ pub fn impl_py_getter_def(
};

let method_def = quote! {
#cfg_attrs
_pyo3::class::PyMethodDefType::Getter({
#deprecations
_pyo3::class::PyGetterDef::new(
Expand Down
25 changes: 25 additions & 0 deletions src/test_hygiene/pyclass.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,28 @@ pub struct Bar {
pub enum Enum {
Var0,
}

#[crate::pyclass]
#[pyo3(crate = "crate")]
pub struct Foo3 {
#[pyo3(get, set)]
#[cfg(FALSE)]
field: i32,

#[pyo3(get, set)]
#[cfg(not(FALSE))]
field: u32,
}

#[crate::pyclass]
#[pyo3(crate = "crate")]
pub struct Foo4 {
#[pyo3(get, set)]
#[cfg(FALSE)]
#[cfg(not(FALSE))]
field: i32,

#[pyo3(get, set)]
#[cfg(any(not(FALSE)))]
field: u32,
}