google / zerocopy

https://discord.gg/MAvWH2R6zk
Apache License 2.0
1.61k stars 105 forks source link

Bitfield Integration in Structs #1497

Open shilonic opened 4 months ago

shilonic commented 4 months ago

Edits by maintainers:

https://github.com/bitflags/bitflags/pull/326#discussion_r1179770888 suggests that there's no way to support zerocopy, but I think we can work around that.

cc @KodrAus @qwandor

IIUC, the problem is that:

However, I think we can work around this by instead modifying bitflags so that certain internals of its macros are gated by #[cfg(feature = "zerocopy_0_N")]. That way, the code emitted into the caller's crate will simply have #[derive(...)] attributes which are already correctly chosen.

The naive solution requires the caller to also depend on zerocopy so that derive-emitted code can refer to zerocopy. A more ergonomic solution depends on supporting a #[zerocopy(crate = ...)] attribute (#11) so that only bitflags needs to take a zerocopy dependency (which it then re-exports).


Original text by @shilonic:

I'm currently working on a project that requires integrating bitfields into structs while maintaining zero-copy functionality. I believe adding native support for bitfield fields would enhance the usability and flexibility of the zero-copy crate. I have tried integrating the bitfield crate and modular-bitfield, but using these options requires two levels of validation:

Questions: What is the best way to integrate bitfields into structs using this crate? Are there existing patterns or recommendations for handling bitfields efficiently in a zero-copy context?

joshlf commented 3 months ago

We don't have any special support for bitfields, nor any plans to add that support.

However, as part of the TryFromBytes trait (#5), we plan to eventually support "custom validators". When calling a conversion method like try_ref_from_bytes, zerocopy first validates that the bytes are a valid instance of the Rust type (ie, that constructing the type does not constitute undefined behavior). If this validation passes, then the user's custom validator is called, and it is given the opportunity to accept or reject the instance. If the custom validator returns an error, try_ref_from_bytes also returns an error.

Would that give you the support you need?

ianthetechie commented 1 month ago

I have a similar request. I think the approach would work at least in theory for everything I've run into. If zercopy could explicitly support bitfields, that'd be nice, but understood that this isn't your focus.

I am in fact already using bitfields extensively and it works well for the most part, but have a few edge cases. I'm using the bitfield_struct crate, and deriving TryFromBytes fails when used on an enum with a numeric representation.

Example enum:

#[derive(TryFromBytes, Debug)]
#[repr(u8)]
pub enum IntBackedEnum {
    FirstVariant = 0,
    SecondVariant = 0,
    // Some other variants, with gaps across the u8 space...
}

Trivial bitfield struct:

#[derive(TryFromBytes)]
#[bitfield(u8)]
struct MyBitfield {
    // We're bit packing and only care about 6 bits!
    #[bits(6)]
    enum_value: IntBackedEnum
    #[bits(2)]
    other_field: u8, // You get the idea...
}

Could be that I'm holding it wrong, but I get the following errors in this situation:

no variant or associated item named `from_bits` found for enum `IntBackedEnum` in the current scope
no variant or associated item named `into_bits` found for enum `IntBackedEnum` in the current scope
attempted to take value of method `other_field` on type `MyBitfield`
joshlf commented 1 month ago

@ianthetechie I believe you've run into #388. Try putting #[bitfield(u8)] above #[derive(TryFromBytes)] and see if it works?

ianthetechie commented 1 month ago

Thanks for the quick feedback @joshlf! I attempted that as well, but still end up with (extremely similar; only varying in count?) errors 😅 I'll continue the discussion and post an MRE over in #388.

qwandor commented 1 month ago

bitflags now supports custom derives via https://github.com/bitflags/bitflags/issues/348 which provides a reasonable workaround to this with just a bit of extra boilerplate, so I don't need anything else for zerocopy for that.