Curious if Debug can removed?

[JPHutchins]

I have a protocol that uses big endian for a few of the u16 bytes. So, it works perfectly with read_be(), nice! But I have to layout the deserialization kinda funky like to match the BE:

use bitfield_struct::bitfield;
use binrw::{BinRead, BinReaderExt};

#[bitfield(u64)]
#[derive(BinRead)]
struct Header {
    command_id: u8,
    sequence: u8,
    group_id: u16,
    length: u16,
    flags: u8,
    #[bits[3]]
    op: u8,
    #[bits[2]]
    version: u8,
    #[bits[3]]
    _reserved: u8,
}

fn main() {
    const DATA: [u8; 8] = [0x01 | (2 << 3), 3, 0, 4, 0, 5, 6, 7];

    let mut reader = std::io::Cursor::new(&DATA);
    let header: Header = reader.read_be().unwrap();
    println!("{:?}", header);
}

This will run with output:

Header { command_id: 7, sequence: 6, group_id: 5, length: 4, flags: 3, op: 1, version: 2 }

Great! Except that the debug ordering is also "the wrong order", and I didn't even specify that I wanted to derive Debug!

I'd love to override the Debug trait, but this seems to not be a thing in Rust?

impl std::fmt::Debug for Header {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
        write!(f, "Header {{ op: {}, version: {}, flags: {}, length: {}, group_id: {}, sequence: {}, command_id: {} }}",
            self.op(),
            self.version(),
            self.flags(),
            self.length(),
            self.group_id(),
            self.sequence(),
            self.command_id()
        )
    }
}

Gives diagnostic

error[E0119]: conflicting implementations of trait `Debug` for type `Header`
  --> src/main.rs:22:1
   |
22 | #[bitfield(u64)]
   | ^^^^^^^^^^^^^^^^ conflicting implementation for `Header`
...
39 | impl std::fmt::Debug for Header {
   | ------------------------------- first implementation here
   |
   = note: this error originates in the attribute macro `bitfield` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0119`.

Perhaps the Debug trait can be removed from bitfield so that people can define their own Debug if needed? Maybe there is a better way, I am very new to Rust!

Also, I think it's been addressed that the struct layout occurs in LE or BE order as a design decision. I can understand that. But it will cause protocols like this to be "backwards" of the spec since these sorts of protocols are not usually "written backwards" just because they use BE for some of their integers!

Something like this would match how Python and C would do it, for example:

#[bitfield(u64)]
#[derive(BinRead)]
struct Header {
    #[bits[3]]
    op: u8,
    #[bits[2]]
    version: u8,
    #[bits[3]]
    _reserved: u8,
    flags: u8,
    #[endian[big]]
    length: u16,
    #[endian[big]]
    group_id: u16,
    sequence: u8,
    command_id: u8,
}

And then the reader would use read_le, assuming they understand what they are doing! Yet I can see how ordering the struct for endianness is more correct - here I bring it up only as a concern for readability and program correctness.

Thanks for this great work!

JP

[JPHutchins]

Another thought - am I bringing this on myself by using u64?

[JPHutchins]

Agh! Debug is resolved easily with #[bitfield[u64, debug = false]]

use bitfield_struct::bitfield;
use binrw::{BinRead, BinReaderExt};

#[bitfield[u64, debug = false]]
#[derive(BinRead)]
struct Header {
    command_id: u8,
    sequence: u8,
    group_id: u16,
    length: u16,
    flags: u8,
    #[bits[3]]
    op: u8,
    #[bits[2]]
    version: u8,
    #[bits[3]]
    _reserved: u8,
}


impl std::fmt::Debug for Header {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
        write!(f, "Header {{ op: {}, version: {}, flags: {}, length: {}, group_id: {}, sequence: {}, command_id: {} }}",
            self.op(),
            self.version(),
            self.flags(),
            self.length(),
            self.group_id(),
            self.sequence(),
            self.command_id()
        )
    }
}

fn main() {
    const DATA: [u8; 8] = [0x01 | (2 << 3), 3, 0, 4, 0, 5, 6, 7];

    let mut reader = std::io::Cursor::new(&DATA);
    let header: Header = reader.read_be().unwrap();
    println!("{:?}", header);
}

Header { op: 1, version: 2, flags: 3, length: 4, group_id: 5, sequence: 6, command_id: 7 }

🚀

[JPHutchins]

The funny thing about the way people throw BE and LE around, is that the commonly only think about the byte order of INDIVIDUAL integers, not the byte order of the entire structure. I believe this implementation is correct in considering it to be the byte order of the overall structure.

[csnover]

Sorry for the delay. Just to be clear, binrw doesn’t add a Debug implementation. This was caused by the use of the unrelated bitfield_struct crate. Glad you are enjoying binrw!