Support zerocopy derives #326
Conversation
| #[cfg(feature = "zerocopy_0_6")] | ||
| use ::core::marker::Sized; | ||
|
|
||
| #[cfg_attr(feature = "zerocopy_0_6", derive($crate::__private::zerocopy::AsBytes, $crate::__private::zerocopy::FromBytes))] |
There was a problem hiding this comment.
Unfortunately this won't actually work. This code is expanded in an end-user's library where there's no knowledge of whether or not bitflags has this feature enabled. For other libraries, we instead expand to a macro that calls back into bitflags and generates appropriate impl blocks.
Since this library requires using their custom derive I think we'll need a different strategy that accepts the struct definition as input and annotates it with its attributes. We'll need a second macro that also includes those necessary use statements.
If zerocopy 0.7 that's currently in-progress has a nicer experience for implementing its traits then I think it would be good to wait for it.
There was a problem hiding this comment.
Hey, came across this while discussing google/zerocopy#287. Zerocopy 0.7 has been released now. Is the experience you're referring to the need to emit an impl for the function only_derive_is_allowed_to_implement_this_trait or something else? We've recently done some work to make the docs more precise with regards to what needs to hold in order for a trait impl to be sound, so that might help you. Let me know if there's anything else we could add that would make this easier for you!
| @@ -49,7 +49,7 @@ jobs: | |||
| run: cargo install cargo-hack | |||
|
|
|||
| - name: Powerset | |||
| run: cargo hack test --feature-powerset --lib --optional-deps "std serde" --depth 3 --skip rustc-dep-of-std | |||
| run: cargo hack test --feature-powerset --lib --optional-deps "std serde zerocopy_0_6" --depth 3 --skip rustc-dep-of-std | |||
There was a problem hiding this comment.
After rebasing on main this shouldn't be necessary anymore. We automatically test all of these features.
| @@ -23,6 +23,7 @@ exclude = ["tests", ".github"] | |||
| serde = { version = "1.0", optional = true, default-features = false } | |||
| core = { version = "1.0.0", optional = true, package = "rustc-std-workspace-core" } | |||
| compiler_builtins = { version = "0.1.2", optional = true } | |||
| zerocopy = { version = "0.6.1", optional = true, default-features = false } | |||
There was a problem hiding this comment.
I think we should do something like:
zerocopy_0_6 = { version = "0.6.1", optional = true, default-features = false, package = "zerocopy" }here and remove the explicit feature. Otherwise users can depend on zerocopy directly, which will become confusing when a new version is released.
|
@qwandor It looks like this is going to be a bit trickier than I first thought. I'm thinking of adding a If you're happy, I'll work on that and roll |
Fixes #325.