Forward cfgs on pyclass fields to the method defs #2796
Conversation
There was a problem hiding this comment.
Seems like an uncontroversial good idea to me! We already forward cfgs in other places.
Needs a CHANGELOG entry - changed, perhaps?
As for tests, I'm not sure there's much else which could be added? The hygiene test will already fail if the cfg are not forwarded correctly. I suppose a unit test which checks the generated proc-macro output would be most precise test, though we don't do that at all yet :)
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
I suspect it probably works to use an Option instead of having the intermediate TokenStream buffer:
| 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"))); | |
| } |
There was a problem hiding this comment.
I don't think I can collect into a tokenstream :(
newsfragments/2796.md
Outdated
| @@ -0,0 +1 @@ | |||
| Mixing cfgs and pyo3 attributes on struct fields will now work | |||
There was a problem hiding this comment.
Needs to be 2796.changed.md (category is important so that towncrier knows where this goes on the changelog).
|
bors r+ |
|
Build succeeded: |
With this and the cfg_attr PR, I don't need cfg_eval at all anymore :)