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 custom derives callback #2059

Merged
merged 2 commits into from Jul 16, 2021
Merged

Conversation

ericseppanen
Copy link
Contributor

This callback allows us to specify arbitrary derive attributes for each named struct. This is useful for adding things that can't easily be implemented separately, such as serde::Deserialize or zerocopy::FromBytes.

This addresses the easiest cases of #1089.

There aren't any tests yet-- I'm not sure what the best way would be to test this, since it can't be reduced to a command line.

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good but needs a test. You can add one to e.g. bindgen-integration perhaps?

src/callbacks.rs Outdated Show resolved Hide resolved
This callback allows us to specify arbitrary derive attributes for each
named struct. This is useful for adding things that can't easily be
implemented separately, such as `serde::Deserialize` or
`zerocopy::FromBytes`.
This test derives PartialEq for the Test struct, and then attempts to
use that by calling assert_ne! on two Test instances. If the derive
callback doesn't work, no PartialEq will be present and the test will
fail to compile.
@ericseppanen
Copy link
Contributor Author

I added a test to bindgen-integration, and verified that the test fails if the add_derives callback isn't working.

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Thank you!

@ericseppanen
Copy link
Contributor Author

Is there anything I can do to help drive this change forward?

@emilio
Copy link
Contributor

emilio commented Jul 16, 2021

No, sorry, I've been just on vacation most of June, will merge+release asap :)

@emilio emilio merged commit f65f230 into rust-lang:master Jul 16, 2021
@ericseppanen
Copy link
Contributor Author

I'm happy to hear that. If there is any follow-up work needed, I'm happy to help contribute.

@a-cristi
Copy link

Is there a way of adding a custom derive when using the CLI tool?

@ericseppanen
Copy link
Contributor Author

@a-cristi Unfortunately, no. I couldn't imagine how the CLI would work, and I wasn't sure whether it would be useful, so I didn't even try.

My opinion doesn't matter much (I'm not a regular contributor to this crate), so feel free to open a new issue to discuss the benefits of adding CLI support.

@a-cristi
Copy link

@a-cristi Unfortunately, no. I couldn't imagine how the CLI would work, and I wasn't sure whether it would be useful, so I didn't even try.

My opinion doesn't matter much (I'm not a regular contributor to this crate), so feel free to open a new issue to discuss the benefits of adding CLI support.

I was thinking that it could work like --with-derive-default for example, by simply deriving something for all types. Something like --with-custom-derives=Foo,Bar will just end up adding a #[derive(Foo, Bar)] for every type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants