BufWriter documentation should warn about ignored errors on drop

[jgallag88]

BufWriter calls flush_buf() in its destructor, which writes out the contents of its internal buffer to the underlying writer. The destructor ignores any errors that happen when flushing the buffer. This makes it easy to accidentally forget to handle such errors:

use std::io::BufWriter;
use std::io::Result;
use std::io::Write;
use std::process::{ChildStdin, Command, Stdio};
use std::time::Duration;
use std::thread;

fn mk_broken_pipe() -> ChildStdin {
    let child = Command::new("true").stdin(Stdio::piped()).spawn().unwrap();
    // Wait for child to exit and close it's end of the pipe
    thread::sleep(Duration::from_secs(1));
    child.stdin.unwrap() // All writes to this pipe should now fail
}

fn f1() -> Result<()> {
    let mut child_stdin = mk_broken_pipe();
    child_stdin.write_all(b"Some bytes")
}

fn f2() -> Result<()> {
    let mut child_stdin = BufWriter::new(mk_broken_pipe());
    child_stdin.write_all(b"Some bytes")
    // Oops, forgot to call flush()
}

fn f3() -> Result<()> {
    let mut child_stdin = BufWriter::new(mk_broken_pipe());
    try!(child_stdin.write_all(b"Some bytes"));
    child_stdin.flush()
}

fn main() {
    println!("Without BufWriter: {:?}", f1());
    println!("With BufWriter, but forgot to call flush: {:?}", f2());
    println!("With BufWriter, with call to flush: {:?}", f3());
}

The code above outputs

Without BufWriter: Err(Error { repr: Os { code: 32, message: "Broken pipe" } })
With BufWriter, but forgot to call flush: Ok(())
With BufWriter, with call to flush: Err(Error { repr: Os { code: 32, message: "Broken pipe" } })

It's tempting (at least to me) to think of BufWriter as preserving the semantics of the underlying writer, while offering possible performance improvements. It doesn't quite though, and it's unfortunately easy to forget to call flush(), and to ignore errors when using BufWriter, as in function f2() above.

It seems like a warning in the BufWriter documentation could be helpful here. Right now it says

The buffer will be written out when the writer is dropped.

Perhaps it could be changed to something like

When the writer is dropped, the BufWriter will be flushed and the contents of its buffer will be written out. However, any errors that happen when the writer is dropped will be ignored. Code that wishes to handle such errors must manually call flush() before the writer is dropped.

If a change along these lines is of interest, I'll make a pull request.

[nagisa]

Duplicate of #32677

[jgallag88]

Ah, didn't notice that one. Well, if you want a documentation change until that one gets sorted out, let me know.

[Mark-Simulacrum]

I'm going to close this in favor of #32677.