sharksforarms / deku

Declarative binary reading and writing: bit-level, symmetric, serialization/deserialization
Apache License 2.0
1.11k stars 54 forks source link

Can't use newtype-structs together with deku-attributes #311

Closed pflakus closed 1 year ago

pflakus commented 1 year ago

I was trying to implement newtype structs for some of my frame's fields, to be able to add custom methods to those fields themselves. At the same time, I also needed to specify the amount of bits the field uses in a frame.

However this combination (newtype struct + some deku attribute applied to the respective field) seems to cause something in the DekuRead- and DekuWrite-invocations to fail.
When the field type is replaced by the underlying type (here: u8) the errors go away.

Here's a minimal example illustrating the problem:

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct Flags(u8);

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
pub struct Frame {
    #[deku(bits = 6)]
    flags: Flags,
    //flags: u8,  // will compile, if `flags` is of the type `u8` instead
}

Attempting to build with that newtype struct causes the following errors to be thrown:

❯ cargo build   
   Compiling deku_test v0.1.0 (/home/pflakus/deku_test)
error[E0308]: mismatched types
   --> src/main.rs:6:28
    |
6   | #[derive(Debug, PartialEq, DekuRead, DekuWrite)]
    |                            ^^^^^^^^
    |                            |
    |                            expected `()`, found struct `deku::ctx::BitSize`
    |                            arguments to this function are incorrect
    |
note: associated function defined here
   --> /home/pflakus/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/deku-0.15.1/src/lib.rs:295:8
    |
295 |     fn read(
    |        ^^^^
    = note: this error originates in the derive macro `DekuRead` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> src/main.rs:6:38
    |
6   | #[derive(Debug, PartialEq, DekuRead, DekuWrite)]
    |                                      ^^^^^^^^^
    |                                      |
    |                                      expected `()`, found struct `deku::ctx::BitSize`
    |                                      arguments to this function are incorrect
    |
note: associated function defined here
   --> /home/pflakus/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/deku-0.15.1/src/lib.rs:321:8
    |
321 |     fn write(
    |        ^^^^^
    = note: this error originates in the derive macro `DekuWrite` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0308`.
error: could not compile `deku_test` due to 2 previous errors
wcampbell0x2a commented 1 year ago

The following should work:

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct Flags(#[deku(bits = 6)] u8);

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
pub struct Frame {
    flags: Flags,
}

Wish deku had better errors to the user, if possible in this case!

pflakus commented 1 year ago

Ah, this makes sense in hindsight, and works on my end like it should.
But seconded, a better error message would have been helpful. ;)

Anyway, thank you, @wcampbell0x2a, for pointing out the immediate solution!

sharksforarms commented 1 year ago

That's correct, yes. Clear errors is not something I've found a solution for yet, any ideas are welcome.

As an alternative, I pushed a branch as an example of what we could do to allow such a construct: sharksforarms/bits-as-tokenstream

Which would allow something like this to compile, passing the value from the parent to the child

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
#[deku(ctx = "parent_bit_size: deku::ctx::BitSize")]
struct Flags( #[deku(bits = "*parent_bit_size")] u8);

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
pub struct Frame {
    #[deku(bits = 6)]
    flags: Flags,
}

so your previous example would fail to compile with this error instead:

error[E0308]: mismatched types
   --> examples/example.rs:23:32
    |
23  |     #[derive(PartialEq, Debug, DekuRead, DekuWrite)]
    |                                ^^^^^^^^
    |                                |
    |                                expected `()`, found struct `deku::ctx::BitSize`
    |                                arguments to this function are incorrect

Still not super clear... requires knowledge of deku's context (ctx) feature/attribute.

Any thoughts @wcampbell0x2a @pflakus ?

pflakus commented 1 year ago

Tbh, I don't feel like that kind of workaround is really necessary. For my sake at least.
I'd be fine to keep it the old way, if the documentation makes it clear that field-attributes should only be applied to primitive types (or whatever the underlying requirement is) and maybe an examples/-script that hints at @wcampbell0x2a's solution to deal with my specific use case.

I don't feel up to the challenge of finding a way to handle the scenario in the library itself, to yield a cleaner error.
So until somebody does, this seems like the best solution to me.

sharksforarms commented 1 year ago

Closing for now.