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

Implementation of Buf for VecDeque #249

Merged
merged 1 commit into from Mar 6, 2019
Merged

Implementation of Buf for VecDeque #249

merged 1 commit into from Mar 6, 2019

Conversation

vorner
Copy link
Contributor

@vorner vorner commented Mar 4, 2019

Since it can be easily re-filled.

I'll think about how (if possible) to implement BufMut too.

Related to #248.

Since it can be easily re-filled.
@vorner
Copy link
Contributor Author

vorner commented Mar 4, 2019

Ah, I haven't read the comment on #248 yet… ok.

@vorner vorner closed this Mar 4, 2019
@carllerche
Copy link
Member

The primary reason I am not a huge fan of implementing Buf and BufMut for VecDeque<u8> is that it feels weird to me that bytes() and bytes_mut() return different ranges. This is also why RingBuf used to have reader() and writer() accessors.

@carllerche
Copy link
Member

BTW, if you release a crate, I'd be happy to include a section in the README that links to additional implementations of Buf / BufMut.

@vorner
Copy link
Contributor Author

vorner commented Mar 5, 2019

Isn't the different ranges somewhat a problem of Vec<u8> too? Well, the IntoBuf wraps it into a Cursor, but still.

Hmm, it seems it isn't reasonably possible to implement BytesMut for VecDeque anyway ‒ it doesn't expose a way to access the unused part of allocation or raw pointers inside it.

I'll keep the offer with readme in mind, but considering the above, I'll have to think if I even want to create the crate. It might still work in the use case I described, though, even without that ability.

@carllerche
Copy link
Member

You are probably right about the inconsistency. I guess the question should be if it is reasonable for single types to impl both Buf and BufMut such that data is read from the front and written to the back.

@carllerche
Copy link
Member

Cc @seanmonstar

@carllerche
Copy link
Member

@vorner I'll re-open. I think you are probably right.

@carllerche carllerche reopened this Mar 5, 2019
@carllerche
Copy link
Member

@vorner Ok, this looks good. Do you want to add a BufMut impl or do you want it merged as is?

@vorner
Copy link
Contributor Author

vorner commented Mar 5, 2019

Probably as it is. As I said, I don't see a way to implement BufMut right now. If I come up with one later on, I'll open another PR.

Anyway, if this is becomes a problem or too much work when going to the 0.5, it's always possible to drop the support or you can ask me to port it. I'm not really a good maintainer of stuff, but I should at least be able to take care of what I wrote.

@carllerche carllerche merged commit 0e8b440 into tokio-rs:v0.4.x Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants