bincode-org / bincode

A binary encoder / decoder implementation in Rust.
MIT License
2.63k stars 265 forks source link

Decode context #710

Open branchseer opened 4 months ago

branchseer commented 4 months ago

Why

With the current Decode/BorrowDecode, it's impossible to implement them on a type that borrows something other than the input data. A concrete example would be a Vec<'bump, T: 'bump> which is allocated by an arena allocator.

How

This PR introduces the following changes:

You can see how these changes work together in tests/ctx.rs.

I know this is a big change and would break existing 2.0.0 users. But it's still rc so I guess it's not too late to discuss it? ;)

Alternative Design

Leaves (Borrow)Decode untouched, and add (Borrow)DecodeWithContext<C>.

Pros:

Con:

TODOs

This PR hasn't been polished to a mergeable state yet. I will do that if the overall design is accepted.

VictorKoenders commented 4 months ago

I like this concept a lot, but I have a vague memory I had a discussion about this with @ZoeyR a while back and we decided not to do this.

We'll get back to you

VictorKoenders commented 4 months ago

I had a discussion with @ZoeyR and we decided that this is a useful feature and we're willing to break open the API for this.

Could you get this in a state where the CI succeeds? Specifically test --no-default-features, as this tends to fail a lot.

As for the TODOs:

I like writing out things fully, so Context please

I maintain virtue so feel free to make a PR to that

Let's start with the minimum effort you need to do to get this merged, we can enhance the macros later

Thanks a lot for your proposal!

branchseer commented 4 months ago

That's great to hear. Thanks! I'll work on it as soon as I'm back from vacation.