Provide a way to derive a sensible Debug impl

[jam1garner]

Currently, the behavior results in the following:

            QuickDir {
                bytes: [
                    111,
                    242,
                    94,
                    29,
                    5,
                    111,
                    0,
                    0,
                    44,
                    10,
                    0,
                    0,
                ],
            },

Since the attribute macro runs before the derive happens, it should be possible to check for #[derive(Debug)], remove Debug from the list of traits to be derived, and manually derive Debug ourselves. This would allow for Debug to look the way it would if you didn't use #[bitfield].

I'm willing to do an implementation, just want to make sure this fits in with the design goals before starting work on it.

[Robbepop]

I haven't thought about this yet, but it is a very good idea!

However, before you continue implementing this feature let us please discuss upfront how we want to design it. There are some minor pitfalls with bitfields potentially containing invalid bits and bytes that we have to display accordingly.

Can you give me an example output for the following bitfield?

use modular_bitfield::prelude::*;

#[derive(BitfieldSpecifier)]
#[bits = 3]
pub enum Weekday { Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday }

#[bitfield(specifier = true)]
pub struct Date {
    #[bits = 3]
    weekday: Weekday,
    week: u8,
}

#[bitfield]
pub struct FromTo {
    #[bits = 11]
    from: Date,
    #[bits = 11]
    to: Date,
    distance: B10,
}

Note that Weekday is only having 7 and not 8 (power-of-two) variants. So one particular bitpattern is invalid with 3 bits. Also Date has 11 bits, leaving 5 bits invalid if set to 1.

[jam1garner]

I'm a bit confused... aren't, under your current model, invalid bit-patterns more or less undefined behavior? As far as I can tell from the docs (or at least the autogenerated ones) the only way to create an invalid bitpattern is by calling an unsafe function? (that being from_bytes_unchecked).

I'm further confused by the fact the snippet you sent doesn't seem to compile? Although the reason (not having the bit size divisible by 8) seems intentional going off of the following:

Note that Weekday is only having 7 and not 8 (power-of-two) variants. So one particular bitpattern is invalid with 3 bits. Also Date has 11 bits, leaving 5 bits invalid if set to 1

I will, however, try to answer the question to the best of my ability.

My proposed way of going about this is rather simple, the codegen for what your above snippet would look something like the following:

impl core::fmt::Debug for Date {
    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
        f.debug_struct("Date")
            .field("weekday", &self.weekday())
            .field("week", &self.week())
            .finish()
    }
}

impl core::fmt::Debug for FromTo {
    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
        f.debug_struct("Date")
            .field("from", &self.from())
            .field("to", &self.to())
            .field("distance", &self.distance())
            .finish()
    }
}

I don't think the Debug impl is really the place for handling invalid state? It seems to all just fall together by using the user-accessible methods for building the debug struct. I'm sure your concern is valid, but I'd appreciate a bit of elaboration about where the issue with the above approach arises.

[Robbepop]

I'm a bit confused... aren't, under your current model, invalid bit-patterns more or less undefined behavior?

Nope, we just changed the semantics and now rely on getters to protect against these accesses safely. This is also the reason why we will change fn from_bytes to be a safe function in version 0.9.0 of the crate.

I'm further confused by the fact the snippet you sent doesn't seem to compile?

Sorry! Should have explained that it will only compile on master branch atm since it uses features that have not yet been released on crates.io.

For a Debug implementation I would assume as user to see through the state of the printed type. This means that I can safely observe which fields contain invalid bit patterns. With your above presented implementation of Debug for Date and FromTo we would instead panic in case they contain invalid bit patterns but those are actually expected to occur so we really really should never panic upon them with Debug.

A simple fix is to instead expand to these Debug implementations:

impl core::fmt::Debug for Date {
    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
        f.debug_struct("Date")
            .field("weekday", &self.weekday_or_err())
            .field("week", &self.week_or_err())
            .finish()
    }
}

impl core::fmt::Debug for FromTo {
    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
        f.debug_struct("Date")
            .field("from", &self.from_or_err())
            .field("to", &self.to_or_err())
            .field("distance", &self.distance_or_err())
            .finish()
    }
}

However, the ugly part about this particular implementation is that we'd print Ok(..) even though some fields can never contain invalid bit patterns. An ideal solution is to only print as field_or_err if the field can actually contain invalid bit patterns. A compile-time check for this has not yet been implemented and I am unsure if it is possible but it might be.

I would be fine with a non-ideal implementation for a start and eventually improve upon it in a follow-up PR if possible.

[jam1garner]

ohhhhh, thank you that 100% clears it up. I think it should also be possible to avoid the Ok wrapping and do something like...

.field(
    "from",
    self.from_or_err()
        .map(|field| &field as &dyn core::fmt::Debug)
        .unwrap_or(|err| &err as &dyn core::fmt::Debug)
)

(the field method already uses &dyn so this isn't really adding any overhead)

I'll get working on an implementation!

[Robbepop]

Great idea!

I have made a small playground to toy around with some code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1a23c53767f7efa20d605c5349f611ef

Prints:

ok_date = Date { weekday: Thursday, week: 34 }
err_date = Date { weekday: InvalidBitPattern { invalid_bits: 7 }, week: 34 }