deku icon indicating copy to clipboard operation
deku copied to clipboard

`#[deku(as = "")]`/`#[deku(with = "")]` attributes

Open coolreader18 opened this issue 3 years ago • 6 comments

Sort-of related to #174, but more general.

I'm looking into converting RustPython's bytecode format to use deku over bincode (https://github.com/RustPython/RustPython/issues/2362#issuecomment-857747216), and I'm running into a few issues cause I want to keep the DekuRead/Write structs roughly as they are, i.e. ergonomic to use from Rust. For example, not adding separate length fields for deserializing vectors, and also I'd like to use LEB128 to represent most integers as our existing bincode impl does. I understand why those aren't defaults in deku, but it'd be a lot easier to customize deku to meet our needs if there was something like serde's with attribute, or even serde_with's as attribute. Would you accept a PR that adds Deku{Read/Write}As traits and an as attribute to the derive macros so that individual field [de]serializing can be customized? I might just base it off serde_with's design, since I think that seems to work well.

coolreader18 avatar Jun 12 '21 01:06 coolreader18

I'd like to use LEB128 to represent most integers as our existing bincode impl does

Same when writing the luajit bytecode parser. I'd appreciate if we have the as/with mechanism in deku.

constfold avatar Jun 12 '21 03:06 constfold

I'm doing something very similar to what deku does in wasmbin - self-generating parser/serializer for Wasm - where I'm using custom derive implementation. Example: https://github.com/GoogleChromeLabs/wasmbin/blob/3508d13398c59a337d42eaa8a49f9f02434fee9d/src/sections.rs#L197-L237

I was thinking of what would it take to try and reimplement it on top of deku instead, and I think lack of as/with or some other mechanism for defining own implementations for built-in types would be the biggest hurdle.

I think I'd like something even more general though - ideally I'd want to be able to define custom parsers for built-in types (e.g. LEB128 for integers, custom parser for floats, etc.) somewhere in a single place, and then somehow tell Deku to use those implementations for parsing, rather than putting as/with on each single field. I don't have a good idea in mind for how this would work though... Maybe some sort of context object to pass around?

UPD: I see DekuRead and DekuWrite accept Ctx as 2nd type param, perhaps I can build off that...

RReverser avatar Jun 19 '21 23:06 RReverser

I think I'd like something even more general though - ideally I'd want to be able to define custom parsers for built-in types (e.g. LEB128 for integers, custom parser for floats, etc.) somewhere in a single place, and then somehow tell Deku to use those implementations for parsing, rather than putting as/with on each single field. I don't have a good idea in mind for how this would work though... Maybe some sort of context object to pass around?

Something like this, perhaps? The suggestion there was to build one's own proc macro lib that'd do it's thing...

EDIT: Can you show an example please of what you have in mind when you say you can get by with ctx for the above requirement?

v1gnesh avatar Jun 20 '21 03:06 v1gnesh

EDIT: Can you show an example please of what you have in mind when you say you can get by with ctx for the above requirement?

My idea was to use a custom marker to implement custom parsing for built-in types. Thanks to DekuRead / DekuWrite accepting Ctx, this is allowed under Rust trait implementation rules:

struct MyCtx;

impl DekuRead<'_, MyCtx> for u8 {
    fn read(
        input: &'a deku::bitvec::BitSlice<deku::bitvec::Msb0, u8>,
        ctx: MyCtx,
    ) -> Result<(&'a deku::bitvec::BitSlice<deku::bitvec::Msb0, u8>, Self), DekuError>
    where
        Self: Sized {
        todo!()
    }
}

impl DekuRead<'_, MyCtx> for u16 { ... }

impl DekuRead<'_, MyCtx> for u32 { ... }

Then, I can derive DekuRead on all my structures using this context like

#[derive(DekuRead)]
#[deku(ctx = "_: MyCtx")]
struct MyStruct {
    a: u8,
    b: u16,
    y: u32,
}

This ensures that my structures get impl DekuRead<'_, MyCtx> instead of regular impl DekuRead<'_, ()>.

At this point the only missing bit is making sure that the same context is passed to all fields, so that entire type tree uses same context type consistently. Unfortunately, at this step I've realised that currently the only way to do so is to specify it on every field:

#[derive(DekuRead)]
#[deku(ctx = "ctx: MyCtx")]
struct BBB {
    #[deku(ctx = "ctx")]
    a: u8,
    #[deku(ctx = "ctx")]
    b: u16,
    #[deku(ctx = "ctx")]
    y: u32,
}

So, while this helps with boilerplate for types, we're back to square one in terms of specifying something for all fields and probably need a custom macro like suggested in #217...

It's not very complicated to write one, but I wonder if it would make sense to add ctx_nested to Deku itself.

RReverser avatar Jun 20 '21 16:06 RReverser

@RReverser @v1gnesh these are interesting suggestions, I never really thought about an override for built-in types. I like the solution above but it is verbose and kinda a hack way around it. I think we can do better. I'll give it some thought and experimentation this week.

sharksforarms avatar Jun 21 '21 01:06 sharksforarms

That's great to hear, thank you! Please conisder scenarios from both #217 and #213, i.e.,

  • .conf file to put project-wide settings
  • struct/enum-wide setting, but filterable (to specific types or field names perhaps)

v1gnesh avatar Jun 21 '21 02:06 v1gnesh