Implement write_all_vectored for VecDeque#136819
Implement write_all_vectored for VecDeque#136819a1phyr wants to merge 1 commit intorust-lang:masterfrom
write_all_vectored for VecDeque#136819Conversation
|
r? @Noratrieb rustbot has assigned @Noratrieb. Use |
rustbot
added
S-waiting-on-review
| Ok(len) | ||
| } | ||
|
|
||
| fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { |
There was a problem hiding this comment.
Is there a reason this is not #[inline]?
There was a problem hiding this comment.
I think that the policy is to avoid #[inline] by default, but I can add it for consistency with other methods.
There was a problem hiding this comment.
Almost all the other methods in this file have #[inline], so I'd think these should have it too. The only two exceptions seem to have been mistakes and I'm adding them in a soon-to-appear PR.
| } | ||
|
|
||
| fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { | ||
| self.write_vectored(bufs)?; |
There was a problem hiding this comment.
Can you please write a comment above that says that this is OK because write_vectored for VecDeque always writes all data? It's not immediately obvious.
That said, I'm not convinced that this method here should be overriden. The default implementation will simply call write_vectored as well with some stuff around but I think it's perfectly fine to leave the default here. I don't think we win anything from overriding it.
There was a problem hiding this comment.
Or alternatively, move the body of write_vectored into write_all_vectored and call in the other direction.
Edit: This won't work, because write_all_vectored doesn't return the length.
There was a problem hiding this comment.
In #137051, I inspected the codegen for default implementation of write_all_vectored for io::Empty, and it has a lot of slicing logic that can't be optimized out. The same would apply here. I think it's useful to specialize it.
|
FYI, this was superseded by #137107, which just merged. |
5 participants
cc #136756