sharksforarms / deku

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

Cannot specify struct endianness when a nested struct field is present #340

Closed NTmatter closed 10 months ago

NTmatter commented 1 year ago

Specifying endianness with #[deku(endian = "big")] will fail to compile when a structure contains something other than a primitive or primitive array.

This can be worked around by specifying endianness on the primitive fields, though it feels rather noisy and unergonomic to do so.

use deku::prelude::*;

// No problem for primitive array with Big-Endian
#[derive(Debug, DekuRead, DekuWrite)]
#[deku(endian = "big")]
struct DekuFine {
    a: u32,
    b: u32,
    c: [u32; 8],
}

// Used as part of other structs
#[derive(Debug, DekuRead, DekuWrite)]
#[deku(endian = "big")]
struct Contained {
    a: u32,
    b: u32,
}

// Works fine if Endian is specified on individual fields, other than Contained.
#[derive(Debug, DekuRead, DekuWrite)]
struct PerFieldEndian {
    #[deku(endian = "big")]
    a: u32,
    #[deku(endian = "big")]
    b: u32,
    c: [Contained; 8],
}

// This fails with "expected `()`, found `Endian`"
#[derive(Debug, DekuRead, DekuWrite)]
#[deku(endian = "big")]
struct DekuFail {
    a: u32,
    b: u32,
    foo: [Contained; 8],
}
wcampbell0x2a commented 1 year ago

Sorry I didn't give this a good look-over when I gave it a bug label.

There isn't really good compiler errors for this, but Contained needs to understand that it's endian will always come from a ctx from the endian attribute use. These docs might give you some extra context for this. Thus, the following works:

use deku::prelude::*;
use deku::ctx::Endian;

#[derive(Debug, DekuRead, DekuWrite)]
#[deku(endian = "endian", ctx = "endian: Endian")]
struct Contained {
    a: u32,
    b: u32,
}

#[derive(Debug, DekuRead, DekuWrite)]
#[deku(endian = "big")]
struct DekuFail {
    a: u32,
    b: u32,
    foo: [Contained; 8],
}
wcampbell0x2a commented 10 months ago

Closing, let me know if you still have issues.