Add optional binrw/BinRead implementation

caitp commented

2023-06-28 09:58:43 -05:00

(Migrated from github.com)

The goal here is ultimately to teach serde to deserialize raw bitfields from a binary file format, being mindful of the expected endian-ness of the structs. This doesn't get it there from the start, afaik -- but it does seem like a usable starting point.

I don't write a ton of Rust code, so I won't be surprised if this isn't the best way to get this done. Please let me know if there's a better way to do this.

The goal here is ultimately to teach serde to deserialize raw bitfields from a binary file format, being mindful of the expected endian-ness of the structs. This doesn't get it there from the start, afaik -- but it does seem like a usable starting point. I don't write a ton of Rust code, so I won't be surprised if this isn't the best way to get this done. Please let me know if there's a better way to do this.

KodrAus commented

2023-06-28 17:48:41 -05:00

(Migrated from github.com)

Hi @caitp 👋

Thanks for working on this!

Since binrw is unstable, we unfortunately can't add direct support for it in bitflags, but you can still define bitflags types that you can apply its attributes to. We have an example using the zerocopy library here. I think the principle should be the same for binrw.

Hi @caitp :wave: Thanks for working on this! Since `binrw` is unstable, we unfortunately can't add direct support for it in `bitflags`, but you can still define `bitflags` types that you can apply its attributes to. We have an example using the `zerocopy` library [here](https://github.com/bitflags/bitflags/blob/main/examples/custom_derive.rs). I think the principle should be the same for `binrw`.

DCNick3 commented

2023-07-12 04:51:39 -05:00

(Migrated from github.com)

This is an unfortunate degradation of ergonomics compared to 1.x release, where the derive(BinRead) worked without the impl mode

This is an unfortunate degradation of ergonomics compared to 1.x release, where the `derive(BinRead)` worked without the `impl` mode

KodrAus commented

2023-07-14 00:35:06 -05:00

(Migrated from github.com)

@DCNick3 Yes it is a difference with 1.x, but trade-offs have to be made. The 1.x behavior of simply forwarding to the underlying integer type is only useful for cases like this where you're treating them as transparent.

@DCNick3 Yes it is a difference with `1.x`, but trade-offs have to be made. The `1.x` behavior of simply forwarding to the underlying integer type is only useful for cases like this where you're treating them as transparent.

DCNick3 commented

2023-07-14 02:03:24 -05:00

(Migrated from github.com)

Is it possible to have the textual Debug and Display implementations when using the impl mode?

Is it possible to have the textual `Debug` and `Display` implementations when using the `impl` mode?

KodrAus commented

2023-07-14 03:43:03 -05:00

(Migrated from github.com)

You can yes, you just have to implement traits for formatting and parsing based on the functions in the bitflags::parser module. We've got an example that shows it.

You can yes, you just have to implement traits for formatting and parsing based on the functions in the `bitflags::parser` module. We've got [an example that shows it](https://github.com/bitflags/bitflags/blob/main/examples/fmt.rs#L18-L28).

KodrAus commented

2024-03-19 22:38:05 -05:00

(Migrated from github.com)

It's been a while here so I'll go ahead and close this one to keep our PR list current. I'd be happy to take this with unstable support based on the approach we have for zerocopy in the future though.

Thanks again for the PR!

It's been a while here so I'll go ahead and close this one to keep our PR list current. I'd be happy to take this with unstable support based on the approach we have for `zerocopy` in the future though. Thanks again for the PR!

😕 1

DCNick3 commented

2024-03-20 03:06:05 -05:00

(Migrated from github.com)

Until the Debug impl can be derived without boilerplate (tracked in #395), I'll have to stick to bitflags 1.x.

Until the `Debug` impl can be derived without boilerplate (tracked in #395), I'll have to stick to bitflags 1.x.

yeongjet commented

2025-01-20 09:50:09 -06:00

(Migrated from github.com)

Need this feature

DCNick3 commented

2025-01-20 16:26:24 -06:00

(Migrated from github.com)

For my use-case I've been doing this:

bitflags! {
    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
    struct CompressionFlags: u16 {
        const USE_INLINE_ALPHA = 0b00000001;
        const USE_DICT_ENCODING = 0b00000010;
    }
}

impl BinRead for CompressionFlags {
    type Args<'a> = ();

    fn read_options<R: io::Read + io::Seek>(
        reader: &mut R,
        endian: Endian,
        _: (),
    ) -> BinResult<Self> {
        let flags = u16::read_options(reader, endian, ())?;
        CompressionFlags::from_bits(flags).ok_or_else(|| {
            binrw::Error::Io(io::Error::new(
                io::ErrorKind::InvalidData,
                format!("Invalid compression flags: {}", flags),
            ))
        })
    }
}
impl BinWrite for CompressionFlags {
    type Args<'a> = ();

    fn write_options<W: io::Write + io::Seek>(
        &self,
        writer: &mut W,
        endian: Endian,
        _: (),
    ) -> BinResult<()> {
        self.bits().write_options(writer, endian, ())
    }
}

It's a mouthful, but that's that's (probably) the best you can do with the 2.0 design. You can probably wrap it into a decl macro if you need to repeat it a bunch.

Unlike the (naive) 1.0 derive, it will error out on incorrect flags, which is a bonus I guess.

For my use-case I've been doing this: ```rust bitflags! { #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] struct CompressionFlags: u16 { const USE_INLINE_ALPHA = 0b00000001; const USE_DICT_ENCODING = 0b00000010; } } impl BinRead for CompressionFlags { type Args<'a> = (); fn read_options<R: io::Read + io::Seek>( reader: &mut R, endian: Endian, _: (), ) -> BinResult<Self> { let flags = u16::read_options(reader, endian, ())?; CompressionFlags::from_bits(flags).ok_or_else(|| { binrw::Error::Io(io::Error::new( io::ErrorKind::InvalidData, format!("Invalid compression flags: {}", flags), )) }) } } impl BinWrite for CompressionFlags { type Args<'a> = (); fn write_options<W: io::Write + io::Seek>( &self, writer: &mut W, endian: Endian, _: (), ) -> BinResult<()> { self.bits().write_options(writer, endian, ()) } } ``` It's a mouthful, but that's that's (probably) the best you can do with the 2.0 design. You can probably wrap it into a decl macro if you need to repeat it a bunch. Unlike the (naive) 1.0 derive, it will error out on incorrect flags, which is a bonus I guess.

DCNick3 commented

2025-01-20 16:29:23 -06:00

(Migrated from github.com)

Btw, @KodrAus, how do you feel about a binrw integration feature that would be explicitly exempt from semver guarantees (to reflect binrw's unstable nature)? I think it would still be better than nothing for binrw users, even with potential breakages

Btw, @KodrAus, how do you feel about a `binrw` integration feature that would be explicitly exempt from semver guarantees (to reflect `binrw`'s unstable nature)? I think it would still be better than nothing for `binrw` users, even with potential breakages

KodrAus commented

2025-01-20 16:58:13 -06:00

(Migrated from github.com)

@DCNick3 I'm not really keen to add semver-exempt features directly to bitflags. I am keen though to write a proc-macro library like bitflags_derive, that could include this sort of thing in it to reduce boilerplate of deriving traits from other libraries in a flags-aware way.

@DCNick3 I'm not really keen to add semver-exempt features directly to `bitflags`. I am keen though to write a proc-macro library like `bitflags_derive`, that could include this sort of thing in it to reduce boilerplate of deriving traits from other libraries in a flags-aware way.

👀 1

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

Add optional binrw/BinRead implementation #370

Pull request closed