jam1garner / binrw

A Rust crate for helping parse and rebuild binary data using ✨macro magic✨.
https://binrw.rs
MIT License
585 stars 35 forks source link

Allow use of unsized readers and writers #138

Closed csnover closed 2 years ago

csnover commented 2 years ago

This fixes #104, but also will require every custom implementation of read_options/write_options/parse_with/write_with to also change their constraints. I don’t know if I am smart enough to really say whether or not this is a good idea, but it was easy to do!

jam1garner commented 2 years ago

Correct me if I'm wrong but the choice here is a decision between "should every parser implementation include ?Sized" or "should unsized readers require an addition layer of indirection"?

Because tbh if so I think it's a bad idea. It'd maybe be possible to make a unified trait to take the place of the trait bounds but that's still a lot of churn to avoid rare indirection...

csnover commented 2 years ago

When you say usability regression you are talking about how anyone who manually writes a function has to write binrw::io::Read/Write + binrw::io::Seek + ?Sized instead of just binrw::io::Read/Write + binrw::io::Seek, right? And the workaround is to &mut Box<dyn T> instead of being able to &mut dyn T? Want to make sure it is clear before I go back and reject the corresponding issue.

kitlith commented 2 years ago

another workaround could be to wrap the &mut dyn T in a newtype, meaning that it doesn't have to be on the heap.

jam1garner commented 2 years ago

@csnover correct

csnover commented 2 years ago

What if there were a macro for people to write the stand-alone custom parser/serialiser functions like:

binread_fn!(fn foo(reader, options, args: Args) -> Ret {
  // …
});

binwrite_fn!(fn foo(writer, options, args: Args) {
  // …
});
jam1garner commented 2 years ago

What if there were a macro for people to write the stand-alone custom parser/serialiser functions like:

binread_fn!(fn foo(reader, options, args: Args) -> Ret {
  // …
});

binwrite_fn!(fn foo(writer, options, args: Args) {
  // …
});

I think making custom functions easier is potentially beneficial (albeit I can't think of a syntax I would really like tbh? so maybe not...) but I don't think it would actually solve the problem at hand.

I know it's a bit ironic given the whole point of the crate is "macro magic" but without a good enough idea for how to make it feel natural, I'm not sure a macro helps that much. (For example, I'd want the transition from writing the most basic custom parser to one that is generic in some form to feel natural.

Honestly the best I can think of is like... this? I guess?

#[binrw::parse_fn(reader, options)]
fn foo() -> u64 {
    // implementation that takes no args
}

#[binrw::parse_fn(reader)] // read options can be omitted
fn foo(x: u32, y: u32) -> u32 {
    // implementation that takes an args of (u32, u32)
}

#[binrw::parse_fn(reader)]
fn foo<T: BinRead<Args = ()>>() -> T {
    // ...
}

Which... I don't hate. But I'm still not sure how I feel about the ?Sized bound even with this, idk.