Feat: Add IO traits support

[xgroleau]

Added IO support using embedded-io and also supports std after #27. It does requires the user to pass a buffer for memory. Serialization uses embedded-io compatibility layer while deserialization reimplements it since I encountered lifetime issues

Couple points that I'm not sure with this implementation

• I'm not sure if it's DeserializeUnexpectedEnd is the right error to return when the IO fails, should an new internal error be created and used instead?
• Currently the pop function doesn't consume a byte on the provided buffer, is this an issue?

Comments are always appreciated!

[netlify[bot]]

✅ Deploy Preview for cute-starship-2d9c9b canceled.

Name	Link
🔨 Latest commit	049862ba844425d479911cb0130adff8d7a2eb2f
🔍 Latest deploy log	https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/63e55a12c910430007c51c06

[schuetzm]

Serialization uses embedded-io compatibility layer while deserialization reimplements it since I encountered lifetime issues

Which errors exactly? It would be nice if this could be solved, because then use-std wouldn't need to pull in embedded-io.

Also, std::iter::Extend could be useful to implement a flavor for (together with a helper to_extend or similar), as it's less heavy-weight than std::io::Write.

[xgroleau]

Which errors exactly? It would be nice if this could be solved, because then use-std wouldn't need to pull in embedded-io.

I could definitely implement it to std and not depend on embedded-io if that is desired. Since embedded-io had compat layer, I simply reused it to avoid duplication but I don't mind changing it to remove the dep. The error reffered was when I was trying to use the embedded-io compat layer for deserialization, if we implement both separately, we won't have any issue

Also, std::iter::Extend could be useful to implement a flavor for (together with a helper to_extend or similar), as it's less heavy-weight than std::io::Write.

Yes I can see how it could be useful. I think it could remove the need for the AllocVec wrapper if that is the case?

[schuetzm]

Yes I can see how it could be useful. I think it could remove the need for the AllocVec wrapper if that is the case?

Right, or it could be implemented in terms of the Extend flavor. It can still be useful to have a method that just returns a new vector in one step if that is all you need.

[jamesmunns]

Hey @xgroleau, this looks great, thanks! I'll take a deeper look at it this weekend.

Currently the pop function doesn't consume a byte on the provided buffer, is this an issue?

I don't totally understand what you mean here - the two invariants that I can think of that must hold:

1. the deserializer should never see "repeat data" on calls to pop/take_n
2. the "remainder" provided by the deserializer at the end should not contain any bytes used for deserialization

In general - it would be nice to have some kind of basic testing here, like round-tripping to/from a file in a temp directory, or a loopback socket or something. I don't use Read/Write, so any tests to keep me from accidentally breaking things in the future would be appreciated :)

Hopefully I'll have better feedback after I fully review this, thanks also to @schuetzm for the back and forth suggestions so far.

[jamesmunns]

Oh, and regarding

I encountered lifetime issues

If you have a branch or something for that somewhere, feel free to link it, I might be able to help.

[xgroleau]

Thanks all for the quick first feedback! I've made changes to fit the provided comments.

Hey @xgroleau, this looks great, thanks! I'll take a deeper look at it this weekend.

Currently the pop function doesn't consume a byte on the provided buffer, is this an issue?

I don't totally understand what you mean here - the two invariants that I can think of that must hold:

1. the deserializer should never see "repeat data" on calls to `pop`/`take_n`

2. the "remainder" provided by the deserializer at the end should not contain any bytes used for deserialization

For the io traits, they take a buffer since memory needs to be borrowed, but the current pop implementation doesn't consume a byte in the buffer, it only uses the stack (i.e. doesn't advance de pointer as try_take_n). Here is the line for reference. I don't see why it would be an issue, as it only saves space for the provided buffer. The only thing I could see is if the user want's to see the binary representation in the buffer, all field which uses pop won't be there as it uses the stack, but it would be a misuse as they should simply deserialize to a slice instead. i.e.

let mut buff1 = [0; 2048];
let _, T = from_eio((writer, &mut buff)); // Some of the values are buffered in `buff` when `try_take_n` is invoked
let _: T = from_bytes(& buff); /// Won't work properly since all the values that use `pop` won't be represented in the  buffer.

### Test
I added some tests I wanted to test when using reference and the provided buffer for `io` traits, but it doesn't seem to compile when using a struct with a reference. Is this a known issue?

error: implementation of Deserialize is not general enough --> tests/loopback.rs:200:5	200	/ test_io( 201		RefStruct { 202		bytes: &[0, 1, 2, 3, 4], 203		str_s: "Hello wordl!", ...	208		], 209		);		_____^ implementation of Deserialize is not general enough

= note: `RefStruct<'_>` must implement `Deserialize<'0>`, for any lifetime `'0`...
= note: ...but `RefStruct<'_>` actually implements `Deserialize<'1>`, for some specific lifetime `'1`

error: could not compile postcard due to previous error

The test in question 
```rs
    test_io(
        RefStruct {
            bytes: &[0, 1],
            str_s: "hello",
        },
        &[ 0x00 ],
    );

[jamesmunns]

Hey, I haven't had a chance to look at this in detail yet, but someone asked a related question, and I ended up putting together a proof of concept of how you would do "interning" of transient borrows, like I think you have in the case of the Read trait (e.g. once you read the next block, you lose access to the current one).

Sharing it here in case this is illustrative for you, if not, feel free to ignore: https://gist.github.com/jamesmunns/de99d22c7dbfd0e47f8ac87e0c1a8872

[xgroleau]

Thanks for the snippet! What I've done is actually pretty similar to your gist, so I guess it's a good sign. But instead, it's an io instead of a vec for the input and uses raw pointers just like the Slice wrapper that was already provided.

Maybe we could add a generic interned? Not sure if there would be value there since the user currently only need to implement the io traits, a generic interned would require the user to impl something else anyway. I don't know, just throwing ideas.

[dignifiedquire]

This would be really useful for us, having to read and write from files. Any chance this could get merged/finished? Happy to help if useful.

[xgroleau]

I totally forgot about this PR, but I've been using it in a project for quite some time via git dependencies. If James has change requests I'm more than happy to complete them, fix the merge conflicts to get the PR merged.

[jamesmunns]

I totally forgot about this PR, but I've been using it in a project for quite some time via git dependencies. If James has change requests I'm more than happy to complete them, fix the merge conflicts to get the PR merged.

If you've been using this and happy with it, I'm happy to merge after a rebase and get a release cut with these changes.

Apologies for the delay, and thanks for the reping!

[netlify[bot]]

✅ Deploy Preview for cute-starship-2d9c9b canceled.

Name	Link
🔨 Latest commit	bb428bb95ea7f4b7aaaee6d4bd2591144247a252
🔍 Latest deploy log	https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/650dbee4b6744f0008e2628a

[jamesmunns]

@xgroleau @dignifiedquire this is now released as postcard v1.0.8!