Added TryFromCtx impl for &[u8] with no ctx.
Hi, this PR adds an implementation of TryFromCtx for &[u8], with no context. Instead, the whole input slice is converted. This is useful, if a struct has a variable length body and users may further specify that type through a generic.
I'd also like to learn more about your usecase, could perhaps post a snippet of the type of pread you're attempting/want to do, along with the type, etc.
also thanks for all your work on this crate so far and following up on issues ! :)
My usecase for this is, that in my library I have a type ManagementFrame, which is generic over its body and there are multiple type aliases, which define for example a BeaconFrame as a ManagementFrame<BeaconFrameBody>. This is all well and good, as long as we actually have a strongly typed version of that body, that implements TryFromCtx<()>. In other situations, it's better to have something more generic, like ManagementFrame<&[u8]>, which currently doesn't work through the same generic implementation of TryFromCtx. There are other usecases for this, for example strongly typing certain data frames.
Regarding the implications for Pread, I was surprised, that plain pread just uses a default ctx, which in most cases is just () (It's in the docs, I know, but it still surprised me). Perhaps, we should consider either of two options here:
- Change
pread's behavior, to assume a unit type as ctx (Potentially a very breaking change, but maybe more intuitive) and create something likepread_default - Create a
pread_without, which assumes a unit type as ctx (Not breaking at all, but less intuitive IMO)
Getting back to this now, so:
Regarding the implications for Pread, I was surprised, that plain pread just uses a default ctx, which in most cases is just () (It's in the docs, I know, but it still surprised me)
Maybe I'm misunderstanding you, but pread doesn't use a "default" ctx, insofar as it only requires the Ctx: Default bound if you invoke pread<T>; if you invoke pread_with<T>(ctx) this does not require Ctx: Default.
That being said, none of this actually matters w.r.t. the ambiguous trait dispatch. The heart of the issue is that when someone implements > 1 TryFromCtx, e.g. TryFromCtx<Ctx1> for T and TryFromCtx<Ctx2> for T, due to how rust dispatches to trait methods, it can't resolve which of those it should pick; so it requires the user to pick whichTryFromCtx to use via the UFCS (or whatever it's called these days).
This was a conscious decision to not burden the user with this, or to have the Ctx appear in the generic bound of a pread invocation, e.g., I originally experimented with various forms of something like pread::<T, Ctx1>() but I decided I didn't like the inelegance of that second generic parameter, and it being fairly onerous on the user (me at the time).
The original design goals of scroll were roughly, ergonomic user api (not really ergonomic for library author(s) :laughing: ), very generic (see how far could take it), offset based, allow user defined types, support deriving.
I don't think it's in the cards that scroll will ever change in that regard (somewhat ergonomic user api), but if you have some idea theoretically how the existing design could potentially be redesigned (while keeping most/all of the ergonomics of current design) to allow multiple TryFromCtx impls, I would be interested (even if it only ever is theoretical, different generic designs are interesting in and of themselves); however just realistically, if it is a bunch of breaking changes, or significantly diverges from the design of the library it's unlikely to get merged, just to be straight with you :)
======================
I'd like to understand your usecase a bit more/solve it though; is it possible for you to paste maybe a simple repro with what you'd like to do/expect?
Maybe something like:
let data = bytes.pread::<Foo<Bar>>(0)?;
let data2 = bytes.pread::<Foo<&[u8]>>()?; // does not work today?
I'm also curious if you tested out this patch on your specific usecase, and how you did not hit the ambiguous trait dispatch error i posted above?
Anyway, this was a long comment, thanks for reading this far, and hope you're having a good day/night :)
Closing due to last comment suggesting this won’t ever work for our intended use case. Feel free to reopen if this changes somehow