internal: add tuple struct support in pin_data and pin_init! (V2) - #155
internal: add tuple struct support in pin_data and pin_init! (V2)#155mqqz wants to merge 4 commits into
pin_data and pin_init! (V2)#155Conversation
c9dab99 to
1f2c279
Compare
1f2c279 to
2d2042e
Compare
5a4e625 to
65d2dc5
Compare
|
I pushed new changes that addressed your comments and rebased. |
|
Can you rebase on #161? |
9a5c70e to
a61d14e
Compare
|
Could you handle cfgs in a way similar to #165? Unlike |
|
I'll rethink this PR as a whole and double check (the last few fixes were rushed because they were slaving me away at work) |
|
Thanks for working on this! |
a61d14e to
ec74456
Compare
|
oh its my absolute pleasure to work on this. I took my time to fix botched rebase and clean up commits. Also, I think CI is broken now. |
ec74456 to
3ec0b2f
Compare
Create a new `util.rs` to host utility code that are generic and can be shared by multiple macros. Signed-off-by: Gary Guo <gary@garyguo.net>
`#[pin_data]` rejects tuple structs because it assumes every field has a name, which it uses for the projection field, the `__Unpin` field and the pin-data accessor. Identify fields by `syn::Member` instead, so that tuple fields are referred to by their index in generated field accesses. The names that generated items still need are derived from the index as `_0`, `_1`, etc. The projection of a tuple struct is a tuple struct itself, so projected fields are accessed with the same `.0`, `.1` syntax as on the input type rather than through synthesised names. Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc> [ Moved utility code to util.rs as extension trait - Gary ] Signed-off-by: Gary Guo <gary@garyguo.net>
Extend the initializer syntax so that a field can be named by an index,
addressing tuple struct fields the same way a struct expression does:
pin_init!(Foo { 0: value, 1 <- initializer })
Tuple fields are not exposed by a `let` binding to the fields after them,
since they have no name to bind; `_0` would shadow a user variable.
Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
[ Fixed incorrect index calculation and cleaned up the code - Gary ]
Signed-off-by: Gary Guo <gary@garyguo.net>
A tuple struct whose fields are all set to a value reads better written
like a call to its constructor than with the indices spelled out:
pin_init!(Foo(value, value))
Parse the two forms into separate types and rewrite the constructor
arguments into the indexed fields they are shorthand for, so that only the
parser has to know about the second form.
The arguments have no names, so they cannot use `<-`. Parse it anyway and
reject it afterwards, which reports the position of every offending `<-`
rather than stopping at the first one.
`cfg` needs different treatment for tuple constructor syntax. As non-derive
proc macros are invoked before cfg is resolved, the macro cannot know
whether a field survives, and dropping a tuple field renumbers every field
after it. That cannot be expressed by attaching a `cfg` attribute to the
initializer of a single field. Thus, resolve tuple field cfgs up front
instead, by generating two cfg-gated invocations of the macro with one
field resolved in each. This is the approach of commit 3445a65
("internal: rework how `#[pin_data]` handles cfg"), and it is linear time
because only one of the two branches is ever expanded. Struct expression
syntax do not renumber, so using tuple structs with struct syntax can keep
using the existing attribute-based handling.
Suggested-by: Gary Guo <gary@garyguo.net>
Link: Rust-for-Linux#165
Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Co-developed-by: Gary Guo <gary@garyguo.net> # cfg expansion
[ Use generics instead of separate types for normalization - Gary ]
Signed-off-by: Gary Guo <gary@garyguo.net>
| // Removing a tuple field shifts every field after it down by one. | ||
| for field in init.fields.iter_mut() { | ||
| if let Some(Member::Unnamed(index)) = field.kind.member_mut() { | ||
| if index.index > removed_index { | ||
| index.index -= 1; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Unfortunately this logic is completely broken, because index in struct expression are never renumbered.
3ec0b2f to
5d04c89
Compare
|
I've updated the PR with tuple index renumbering fixed (and some misc code style improvements too). Can you check if you're happy with the outcome? |
Replaces #113.
This adds tuple struct support to
#[pin_data],init!, andpin_init!.The projected form of a tuple struct is also a tuple struct, so projected fields are accessed with native tuple syntax (
.0,.1, ...), which keeps the generated API closer toordinary Rust and avoids exposing synthetic field names (also makes it easier fields are
cfg'd out).#[pin_data]pin_init!(Foo { 0 <- ..., 1: ... })pin_init!(Foo(...))(but no<-e.g.Foo(a, <- b, c)is rejected)support for tuple structs when fields or constructor arguments are removed by(Error out unless#[cfg]#[cfg]is on the final tuple field)Notes
._0like last time.cfg handling does not try to evaluate user#[cfg]conditions in the proc macroinstead, the macro generates the necessary cfg-dependent layouts and lets rustc select the active branch#[cfg]is supported only on the final tuple field / constructor argument, when it does not change numbering.Tests
Added coverage for:
#[pin]interaction with!Unpinfield typesfeature-dependent cfg field layoutsComparison with the old PR
This replaces the earlier attempt in #113.
Compared with that PR, this version is intentionally narrower and easier to review:
cfghandling wasredesigned to correctly handlesimplified to accept only on last field/arg. or error outcfg-stripped tuple fields and constructor argumentsCloses: #85