Expose types implementing serde::Serializer and Deserializer#729
Expose types implementing serde::Serializer and Deserializer#729VictorKoenders merged 2 commits intobincode-org:trunkfrom
serde::Serializer and Deserializer#729Conversation
e0cf3e6 to
f286ab3
Compare
|
After some experimentation, I decided to not use The PR exposes two new entities: |
|
Sorry for the late response, the last month has been very busy. We're still thinking on if we want this change in bincode or not. Thank you for your patience 🙏 |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Bump, sorry life has been busy |
|
We are considering making a bincode-utils or -contrib crate. If we do this, do you think option 4 would be best? |
|
Option 4 would be best for that kind of crate, but it will still require the main library to expose something that implements Edit: this PR actually just exposes |
|
I merged our trunk into your branch but it seems like there's a CI issue, once that's resolved I think this is good to merge! |
1ac069b to
938ad60
Compare
|
Rebased to trunk, the tests pass for me locally |
|
No idea why I can't reproduce it locally, but I think this should fix it. |
|
Thanks! |
This is sort of a proof-of-concept PR, I would like your input on it even though it's still in draft.
The goal here is to expose types implementing
serde::SerializerandDeserializerso thatbincodecould be used with https://docs.rs/erased-serde.Serializeris straightforward, butDeserializercan be exposed in two ways:D, wherefor<'a> &'a mut D: Deserializer<'de>. This is done e.g. inpostcard. The problem here is that using such types generically involves passing around an ugly bound on lifetimes, which cannot be encapsulated in a trait. Also, dealing with such types inerased_serdeis somewhat complicated (albeit possible).D: Deserializer<'de>. The downside of this is that in the format implementations (likebincode)serde::Deserializeris generally implemented for&mut Something, or a wrapper of it (SerdeDecoderinbincode's case), because it has to be passed to nested list/struct/etc deserializers. So creating another implementation forDinvolves writing down the massive trait impl one more time.So, I would like your input on the way to proceed with this. I see the following options:
bincodeat all. Feel free to close the PR then.D, wherefor<'a> &'a mut D: Deserializer<'de>. This requires minimum changes, but creates problems for the users, as described above.D: Deserializer<'de>, writing down anotherDeserializerimpl (of which there are already two in the codebase).D: Deserializer<'de>, relying on a helper crate that I made (https://docs.rs/serde-persistent-deserializer), which is implemented in this PR. The reason I put it there is that I want to create a similar PR for other formats, and I would like to avoid writingDeserializerimpls in each of them. I understand if you want to minimize the number of dependencies, then the contents of that crate can be brought in verbatim.(This PR currently does not expose
Serializer, but that's easy, and I'll add it later if you decide not to go with Option 1).A few things about the PR itself.
Deserializerimpl inde_borrowed, and it's almost possible by just replacingde_borrowed::SerdeDecoderwithde_owned::SerdeDecoder, but a single test fails because these impls are slightly different. I am not sure if it's possible to merge them.de_owned::decode_from_slice()("Note that this does not work with borrowed types like&stror&[u8]. For that use [borrow_decode_from_slice].") Seems to be working fine, as I can just callde_borrowed::borrow_decode_from_slice()from it.