Documentation for BufRead is confusing

[vandry]

Location (URL)

https://doc.rust-lang.org/std/io/trait.BufRead.html#tymethod.fill_buf

Summary

I'm filing this new issue because I do not have permission to reopen #85394

I think that the documentation for std::io::BufRead is still misleading.

For BufRead::fill_buf

Returns the contents of the internal buffer, filling it with more data, via Read methods, if empty.

If an internal buffer were filled by calling a Read method, that implies that all of the bytes read in this fashion would be consumed and in particular would not be returned again by any future calls to any Read method. But that's just not what happens with any common implementation of BufRead including 2 implementations from the standard library. To wit, if that specification were true you would not expect any of these tests to pass:

use bytes::buf::Buf;
use std::io::{BufRead, Read};

fn bufread_fill_buf_returns_data_that_is_later_read_with_read_read<R>(
    r: &mut R,
)
where
    R: BufRead + Read,
{
    // Assuming the BufRead internal buffer size is at least 3,
    // this gets all of it.
    assert_eq!(r.fill_buf().unwrap(), b"foo");
    let mut array = [0];
    // Documentation for "fill_buf" says "Returns the contents of the
    // internal buffer, filling it with more data, via Read methods, if
    // empty.". Therefore we might expect all 3 bytes from the source
    // to have been consumed by fill_buf, and there are now 0 bytes
    // left for Read::read to return:
    // 💥 assert_eq!(r.read(&mut array).unwrap(), 0);
    // But what happens instead is that we read b"f"
    assert_eq!(r.read(&mut array).unwrap(), 1);
    assert_eq!(array[0], b'f');
}

#[test]
fn std_io_bufreader_fill_buf_does_not_eat_the_buffer() {
    let b = b"foo";
    let mut rb = std::io::BufReader::new(&b[..]);
    bufread_fill_buf_returns_data_that_is_later_read_with_read_read(&mut rb);
    // However the _underlying_ reader does indeed have nothing left:
    let mut unbuffered_reader = rb.into_inner();
    let mut array = [0];
    assert_eq!(unbuffered_reader.read(&mut array).unwrap(), 0);
}

#[test]
fn std_io_cursor_fill_buf_does_not_eat_the_buffer() {
    let b = b"foo";
    let mut c = std::io::Cursor::new(&b);
    bufread_fill_buf_returns_data_that_is_later_read_with_read_read(&mut c);
}

#[test]
fn bytes_reader_fill_buf_does_not_eat_the_buffer() {
    let b = b"foo";
    let mut r = b.reader();
    bufread_fill_buf_returns_data_that_is_later_read_with_read_read(&mut r);
}

but you would instead expect the assertion marked with 💥 to pass.

Indeed before the change 51c5700 there used to be a sentence that made that explicit:

When calling this method, none of the contents will be "read" in the sense that later calling read may return the same contents.

but that sentence has been removed.

Whereas:

• I believe the intended design of this trait is that fill_buf should not "consume" the data that it returns
• None of the implementations of the trait tested above actually implement fill_buf in terms of calling any method of the Read trait

I propose that:

1. The documentation should be changed to not specify any particular implementation of fill_buf but instead perhaps discuss the semanticcs of a notional "read position".
2. For good measure, the sentence about read returning the same contents as fill_buf should be restored.