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

Specialize some methods of io::Chain #105917

Merged
merged 3 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 55 additions & 3 deletions library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2389,16 +2389,50 @@ impl<T: Read, U: Read> Read for Chain<T, U> {
}
self.second.read_vectored(bufs)
}

#[inline]
fn is_read_vectored(&self) -> bool {
self.first.is_read_vectored() || self.second.is_read_vectored()
}
Comment on lines +2393 to +2396
Copy link
Member

@workingjubilee workingjubilee Jul 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this require both to be efficient? I could be persuaded otherwise, certainly, but can you explain why you chose this logic instead? If one half of the chain doesn't have an efficient implementation, then why is the chained type efficient? At the extreme end, we're allowed to apply Chain recursively, so this version you propose would yield true even if we'd chained 16 different things together and only 1 reported true...

Suggested change
#[inline]
fn is_read_vectored(&self) -> bool {
self.first.is_read_vectored() || self.second.is_read_vectored()
}
#[inline]
fn is_read_vectored(&self) -> bool {
self.first.is_read_vectored() & self.second.is_read_vectored()
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you could also use a more complicated answer which checks done_first, but that seems prone to confusion, even if it might be "more accurate".


fn read_to_end(&mut self, buf: &mut Vec<u8>) -> Result<usize> {
let mut read = 0;
if !self.done_first {
read += self.first.read_to_end(buf)?;
self.done_first = true;
}
read += self.second.read_to_end(buf)?;
Ok(read)
}
a1phyr marked this conversation as resolved.
Show resolved Hide resolved

// We don't override `read_to_string` here because an UTF-8 sequence could
// be split between the two parts of the chain

fn read_buf(&mut self, mut buf: BorrowedCursor<'_>) -> Result<()> {
if buf.capacity() == 0 {
return Ok(());
}

if !self.done_first {
let old_len = buf.written();
self.first.read_buf(buf.reborrow())?;

if buf.written() != old_len {
return Ok(());
} else {
self.done_first = true;
a1phyr marked this conversation as resolved.
Show resolved Hide resolved
}
}
self.second.read_buf(buf)
}
}

#[stable(feature = "chain_bufread", since = "1.9.0")]
impl<T: BufRead, U: BufRead> BufRead for Chain<T, U> {
fn fill_buf(&mut self) -> Result<&[u8]> {
if !self.done_first {
match self.first.fill_buf()? {
buf if buf.is_empty() => {
self.done_first = true;
}
buf if buf.is_empty() => self.done_first = true,
buf => return Ok(buf),
}
}
Expand All @@ -2408,6 +2442,24 @@ impl<T: BufRead, U: BufRead> BufRead for Chain<T, U> {
fn consume(&mut self, amt: usize) {
if !self.done_first { self.first.consume(amt) } else { self.second.consume(amt) }
}

fn read_until(&mut self, byte: u8, buf: &mut Vec<u8>) -> Result<usize> {
let mut read = 0;
if !self.done_first {
let n = self.first.read_until(byte, buf)?;
read += n;

match buf.last() {
Some(b) if *b == byte && n != 0 => return Ok(read),
_ => self.done_first = true,
}
}
read += self.second.read_until(byte, buf)?;
Ok(read)
}

// We don't override `read_line` here because an UTF-8 sequence could be
// split between the two parts of the chain
}

impl<T, U> SizeHint for Chain<T, U> {
Expand Down
11 changes: 11 additions & 0 deletions library/std/src/io/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,17 @@ fn chain_bufread() {
cmp_bufread(chain1, chain2, &testdata[..]);
}

#[test]
fn chain_splitted_char() {
let chain = b"\xc3".chain(b"\xa9".as_slice());
assert_eq!(crate::io::read_to_string(chain).unwrap(), "é");

let mut chain = b"\xc3".chain(b"\xa9\n".as_slice());
let mut buf = String::new();
assert_eq!(chain.read_line(&mut buf).unwrap(), 3);
assert_eq!(buf, "é\n");
}

#[test]
fn bufreader_size_hint() {
let testdata = b"ABCDEFGHIJKL";
Expand Down