Skip to content

Allow other attributes in #[pin_data] structs#122

Open
nertpinx wants to merge 1 commit into
Rust-for-Linux:mainfrom
nertpinx:extras
Open

Allow other attributes in #[pin_data] structs#122
nertpinx wants to merge 1 commit into
Rust-for-Linux:mainfrom
nertpinx:extras

Conversation

@nertpinx
Copy link
Copy Markdown

When using a macro with custom attributes in a #[pin_data] struct it can mess up the generated __Unpin struct.

This commit moves the decision-making of which attributes to keep into a new, separate function, so that it is unified in all places. Only #[cfg] and #[doc] attributes are kept since all other can cause issues.

With this commit a struct can use both #[pin] and other custom attributes, allowing combining multiple attribute macros without clashing.

To keep this functionality this commit also adds a test with a custom attribute that fails without this fix.

This is a rebased version of the old #94 which was originally based on the now-removed dev/syn2 branch.

Comment thread internal/src/pin_data.rs Outdated
Comment thread tests/extra_attrs.rs Outdated
use pin_init::*;

#[pin_data]
#[derive(Dummy)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, I'd rather we use something that already exists. How about perhaps test compatibility with serde? @BennoLossin thoughts on introducing serde as test-only dep?

When using a macro with custom attributes in a `#[pin_data]` struct it
can mess up the generated code. The generated code needs nothing more than
the `#[cfg]` attribute, thus strip away all other attributes.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
[ Rebased and updated to only include `#[cfg]` instead of both `#[cfg]` and
  `#[doc]`; doc is not needed for the generated hidden items. - Gary ]
Co-developed-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Gary Guo <gary@garyguo.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants