-
-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC/feat: Add ctx_nested
attribute to top level
#229
base: master
Are you sure you want to change the base?
Conversation
This allows clients to pass context arguments down to all fields
ctx_nested
attribute to top levelctx_nested
attribute to top level
@RReverser @v1gnesh @constfold @wcampbell0x2a We can continue the discussion here. Would a |
bits.as_ref(), | ||
bytes.as_ref(), | ||
ctx.as_ref(), | ||
ctx_nested.as_ref(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be first?
impl DekuWrite<Endian, Size, MyCtx> for u32 {
vs
impl DekuWrite<MyCtx, Endian, Size> for u32 {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like MyCtx
being first, since it's "non-default" deku usage and something you want to see first in impl error.
Apologies in advance if what I'm about to say makes little sense. I'm a pretty basic programmer... So if I have a #[deku(endian="little")]
pub struct H {
a: u16,
#[deku(map="morph", bytes_read="4")]
b: String,
#[deku(map="morph", bytes_read="4")]
c: String,
#[deku(map="morph", bytes_read="4")]
d: String,
#[deku(map="morph", bytes_read="64")]
e: String,
} I can now do this as: struct MyCtx {
boits: [4, 4, 4, 64]
funcs: ["morph", "morph", "morph", "morph"]
}
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
#[deku(ctx = "ctx: MyCtx", ctx_nested = "ctx")]
#[deku(endian="little")]
pub struct H {
a: u16,
b: String,
c: String,
d: String,
e: String,
} and impl<'a> DekuRead<'a, MyCtx> for String {
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,
{
// Custom String reader
// interate/zip through ctx.boits and ctx.funcs and apply that to the `input`
// ... what would the above impl look like?
}
} |
Yeah looks exactly like what I had in mind in #225 (comment):
Seems sufficient for defining custom type structure. I wonder though, how does it interact with other ctx? Is it possible to still add extra |
I don't like this idea at my first feeling. Changing the beheviour of build-in types is somewhat strange for me. But it is indeed a solution. If I must choose one between this and the
Maybe? Maybe |
This PR woudln't address this usecase. The
This will not interfere with existing use deku::{ctx::Endian, prelude::*};
#[derive(Copy, Clone)]
struct MyCtx {}
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
#[deku(
ctx = "endian: Endian, ctx: MyCtx",
ctx_nested = "ctx",
endian = "endian", // endian works with the context system
)]
pub struct Child {
data: u32,
}
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
#[deku(ctx = "ctx: MyCtx", ctx_nested = "ctx")]
pub struct Parent {
#[deku(endian = "big")]
child: Child,
}
// ERROR, Endian is foreign
impl<'a> DekuRead<'a, (Endian, MyCtx)> for u32 {
fn read(
input: &'a deku::bitvec::BitSlice<deku::bitvec::Msb0, u8>,
(endian, ctx): (Endian, MyCtx),
) -> Result<(&'a deku::bitvec::BitSlice<deku::bitvec::Msb0, u8>, Self), DekuError>
where
Self: Sized,
{
// Custom u32 reader
todo!()
}
}
/// ...
error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
--> examples/overide_defaults.rs:24:1
|
24 | impl<'a> DekuRead<'a, (Endian, MyCtx)> for u32 {
| ^^^^^^^^^-----------------------------^^^^^---
| | | |
| | | `u32` is not defined in the current crate
| | this is not defined in the current crate because tuples are always foreign
| impl doesn't use only types from inside the current crate
|
= note: define and implement a trait or new type instead But this is is ok because it doesn't affect the trait impl: use deku::{ctx::Endian, prelude::*};
#[derive(Copy, Clone)]
struct MyCtx {}
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
#[deku(
ctx = "field: u8, ctx: MyCtx",
ctx_nested = "ctx",
)]
pub struct Child {
data: u32,
}
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
#[deku(ctx = "ctx: MyCtx", ctx_nested = "ctx")]
pub struct Parent {
field: u8,
#[deku(ctx = "*field")]
child: Child,
}
impl<'a> DekuRead<'a, MyCtx> for u32 {
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,
{
// Custom u32 reader
todo!()
}
} |
@RReverser Is there a reason why you wouldn't want to abstract this into a separate type? struct Leb128 {
// ...
}
impl DekuRead for Leb128 { ... }
impl DekuWrite for Leb128 { ... }
#[derive(DekuRead, DekuWrite)]
struct ParseMe {
field1: Leb128,
// ...
} |
Leb128 can be used for variety of types (u8, u16, u32, usize, i8, ...) and while they all share same encoding method, they still need to be reflected as distinct types in the final struct. Similar happens for f32 vs f64, where encoding in both cases is just "a set of bytes in little-endian order" but now both types need to be replaced with wrappers too. For me, the whole point of using a custom derive (whether the one I use in wasmbin or the one deku provides) is being able to use "natural" types in the final representation. Thanks for the examples, I think this |
99a33f8
to
75edd42
Compare
Closes: #217
This allows clients to pass context arguments down to all fields.
Example