Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider splitting try_take_n on de_flavors::Flavor #93

Open
jamesmunns opened this issue Feb 18, 2023 · 2 comments
Open

Consider splitting try_take_n on de_flavors::Flavor #93

jamesmunns opened this issue Feb 18, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@jamesmunns
Copy link
Owner

Right now, we don't really distinguish between cases where we want to take N bytes from the stream because we might want to borrow the data (e.g. for zero-copy &'de str or &'de [u8] cases), and where we just happen to know we want the next N bytes in the stream (e.g. the current deserializers for f32, f64, and char).

While discussing an "interning flavor" with @cr1901, I realized that the approach I took in this proof of concept would inadvertently intern bytes for the f32/f64/char types as well, even though these only need to be "transient" borrows.

It might be worth it to add a new default trait method, like try_take_n_transient, which defaults to passing through to try_take_n, to allow flavors that care to distinguish between these kinds of takes.

Could be relevant to #91.

@jamesmunns jamesmunns added the enhancement New feature or request label Feb 18, 2023
@cr1901
Copy link

cr1901 commented Feb 19, 2023

Would it be worth adding a reworked Interner flavor from that gist you made to postcard proper in the future?

(I didn't get to work on tailoring it to my needs today, sorry!)

@jamesmunns
Copy link
Owner Author

Yeah, I think it could make sense. It might be reusable by this and the IO stuff.

Not sure yet though, I need to actually do a more in depth look at pr91 first, to see if that matches what they were doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants