auto merge of #12630 : alexcrichton/rust/flush-buffered, r=brson

Now that we can call `flush()` in destructors, I think that it's appropriate for stdout/stderr to return buffered writers by default.

This doesn't enable certain functionality like a buffered stdin does, but it's what you want 90% of the time for performance reasons.

Author: bors

src/librustc/lib.rs

@@ -297,8 +297,7 @@ pub fn run_compiler(args: &[~str]) {
         match input {
           d::FileInput(ref ifile) => {
             let mut stdout = io::stdout();
-            d::list_metadata(sess, &(*ifile),
-                             &mut stdout as &mut io::Writer).unwrap();
+            d::list_metadata(sess, &(*ifile), &mut stdout).unwrap();
           }
           d::StrInput(_) => {
             d::early_error("can not list metadata for stdin");

src/libstd/fmt/mod.rs

@@ -654,8 +654,8 @@ uniform_fn_call_workaround! {
 /// use std::fmt;
 /// use std::io;
 ///
-/// let w = &mut io::stdout() as &mut io::Writer;
-/// format_args!(|args| { fmt::write(w, args); }, "Hello, {}!", "world");
+/// let mut w = io::stdout();
+/// format_args!(|args| { fmt::write(&mut w, args); }, "Hello, {}!", "world");
 /// ```
 pub fn write(output: &mut io::Writer, args: &Arguments) -> Result {
     unsafe { write_unsafe(output, args.fmt, args.args) }

src/libstd/io/buffered.rs

@@ -14,7 +14,8 @@ use cmp;
 use container::Container;
 use io::{Reader, Writer, Stream, Buffer, DEFAULT_BUF_SIZE, IoResult};
 use iter::ExactSize;
-use option::{Some, None};
+use ops::Drop;
+use option::{Some, None, Option};
 use result::{Ok, Err};
 use vec::{OwnedVector, ImmutableVector, MutableVector};
 use vec;
@@ -115,7 +116,7 @@ impl<R: Reader> Reader for BufferedReader<R> {
 
 /// Wraps a Writer and buffers output to it
 ///
-/// Note that `BufferedWriter` will NOT flush its buffer when dropped.
+/// This writer will be flushed when it is dropped.
 ///
 /// # Example
 ///
@@ -130,7 +131,7 @@ impl<R: Reader> Reader for BufferedReader<R> {
 /// writer.flush();
 /// ```
 pub struct BufferedWriter<W> {
-    priv inner: W,
+    priv inner: Option<W>,
     priv buf: ~[u8],
     priv pos: uint
 }
@@ -142,7 +143,7 @@ impl<W: Writer> BufferedWriter<W> {
         let mut buf = vec::with_capacity(cap);
         unsafe { buf.set_len(cap); }
         BufferedWriter {
-            inner: inner,
+            inner: Some(inner),
             buf: buf,
             pos: 0
         }
@@ -155,7 +156,7 @@ impl<W: Writer> BufferedWriter<W> {
 
     fn flush_buf(&mut self) -> IoResult<()> {
         if self.pos != 0 {
-            let ret = self.inner.write(self.buf.slice_to(self.pos));
+            let ret = self.inner.get_mut_ref().write(self.buf.slice_to(self.pos));
             self.pos = 0;
             ret
         } else {
@@ -167,15 +168,15 @@ impl<W: Writer> BufferedWriter<W> {
     ///
     /// This type does not expose the ability to get a mutable reference to the
     /// underlying reader because that could possibly corrupt the buffer.
-    pub fn get_ref<'a>(&'a self) -> &'a W { &self.inner }
+    pub fn get_ref<'a>(&'a self) -> &'a W { self.inner.get_ref() }
 
     /// Unwraps this buffer, returning the underlying writer.
     ///
     /// The buffer is flushed before returning the writer.
     pub fn unwrap(mut self) -> W {
-        // FIXME: is failing the right thing to do if flushing fails?
+        // FIXME(#12628): is failing the right thing to do if flushing fails?
         self.flush_buf().unwrap();
-        self.inner
+        self.inner.take_unwrap()
     }
 }
 
@@ -186,7 +187,7 @@ impl<W: Writer> Writer for BufferedWriter<W> {
         }
 
         if buf.len() > self.buf.len() {
-            self.inner.write(buf)
+            self.inner.get_mut_ref().write(buf)
         } else {
             let dst = self.buf.mut_slice_from(self.pos);
             vec::bytes::copy_memory(dst, buf);
@@ -196,14 +197,24 @@ impl<W: Writer> Writer for BufferedWriter<W> {
     }
 
     fn flush(&mut self) -> IoResult<()> {
-        self.flush_buf().and_then(|()| self.inner.flush())
+        self.flush_buf().and_then(|()| self.inner.get_mut_ref().flush())
+    }
+}
+
+#[unsafe_destructor]
+impl<W: Writer> Drop for BufferedWriter<W> {
+    fn drop(&mut self) {
+        if self.inner.is_some() {
+            // FIXME(#12628): should this error be ignored?
+            let _ = self.flush_buf();
+        }
     }
 }
 
 /// Wraps a Writer and buffers output to it, flushing whenever a newline (`0x0a`,
 /// `'\n'`) is detected.
 ///
-/// Note that this structure does NOT flush the output when dropped.
+/// This writer will be flushed when it is dropped.
 pub struct LineBufferedWriter<W> {
     priv inner: BufferedWriter<W>,
 }
@@ -256,13 +267,13 @@ impl<W> InternalBufferedWriter<W> {
 
 impl<W: Reader> Reader for InternalBufferedWriter<W> {
     fn read(&mut self, buf: &mut [u8]) -> IoResult<uint> {
-        self.get_mut_ref().inner.read(buf)
+        self.get_mut_ref().inner.get_mut_ref().read(buf)
     }
 }
 
-/// Wraps a Stream and buffers input and output to and from it
+/// Wraps a Stream and buffers input and output to and from it.
 ///
-/// Note that `BufferedStream` will NOT flush its output buffer when dropped.
+/// The output half will be flushed when this stream is dropped.
 ///
 /// # Example
 ///

src/libstd/io/stdio.rs

@@ -90,6 +90,12 @@ fn src<T>(fd: libc::c_int, readable: bool, f: |StdSource| -> T) -> T {
 /// buffered access is not desired, the `stdin_raw` function is provided to
 /// provided unbuffered access to stdin.
 ///
+/// Care should be taken when creating multiple handles to the stdin of a
+/// process. Beause this is a buffered reader by default, it's possible for
+/// pending input to be unconsumed in one reader and unavailable to other
+/// readers. It is recommended that only one handle at a time is created for the
+/// stdin of a process.
+///
 /// See `stdout()` for more notes about this function.
 pub fn stdin() -> BufferedReader<StdReader> {
     BufferedReader::new(stdin_raw())
@@ -104,20 +110,38 @@ pub fn stdin_raw() -> StdReader {
     src(libc::STDIN_FILENO, true, |src| StdReader { inner: src })
 }
 
-/// Creates a new non-blocking handle to the stdout of the current process.
+/// Creates a line-buffered handle to the stdout of the current process.
 ///
 /// Note that this is a fairly expensive operation in that at least one memory
 /// allocation is performed. Additionally, this must be called from a runtime
 /// task context because the stream returned will be a non-blocking object using
 /// the local scheduler to perform the I/O.
-pub fn stdout() -> StdWriter {
+///
+/// Care should be taken when creating multiple handles to an output stream for
+/// a single process. While usage is still safe, the output may be surprising if
+/// no synchronization is performed to ensure a sane output.
+pub fn stdout() -> LineBufferedWriter<StdWriter> {
+    LineBufferedWriter::new(stdout_raw())
+}
+
+/// Creates an unbuffered handle to the stdout of the current process
+///
+/// See notes in `stdout()` for more information.
+pub fn stdout_raw() -> StdWriter {
     src(libc::STDOUT_FILENO, false, |src| StdWriter { inner: src })
 }
 
-/// Creates a new non-blocking handle to the stderr of the current process.
+/// Creates a line-buffered handle to the stderr of the current process.
 ///
 /// See `stdout()` for notes about this function.
-pub fn stderr() -> StdWriter {
+pub fn stderr() -> LineBufferedWriter<StdWriter> {
+    LineBufferedWriter::new(stderr_raw())
+}
+
+/// Creates an unbuffered handle to the stderr of the current process
+///
+/// See notes in `stdout()` for more information.
+pub fn stderr_raw() -> StdWriter {
     src(libc::STDERR_FILENO, false, |src| StdWriter { inner: src })
 }
 
@@ -182,7 +206,7 @@ fn with_task_stdout(f: |&mut Writer| -> IoResult<()> ) {
             Local::put(task);
 
             if my_stdout.is_none() {
-                my_stdout = Some(~LineBufferedWriter::new(stdout()) as ~Writer);
+                my_stdout = Some(~stdout() as ~Writer);
             }
             let ret = f(*my_stdout.get_mut_ref());
 

src/libstd/logging.rs

@@ -166,9 +166,7 @@ pub fn log(level: u32, args: &fmt::Arguments) {
     };
 
     if logger.is_none() {
-        logger = Some(~DefaultLogger {
-            handle: LineBufferedWriter::new(io::stderr()),
-        } as ~Logger);
+        logger = Some(~DefaultLogger { handle: io::stderr(), } as ~Logger);
     }
     logger.get_mut_ref().log(level, args);
 

src/libsyntax/diagnostic.rs

@@ -227,8 +227,8 @@ enum Destination {
 impl EmitterWriter {
     pub fn stderr() -> EmitterWriter {
         let stderr = io::stderr();
-        if stderr.isatty() {
-            let dst = match term::Terminal::new(stderr) {
+        if stderr.get_ref().isatty() {
+            let dst = match term::Terminal::new(stderr.unwrap()) {
                 Ok(t) => Terminal(t),
                 Err(..) => Raw(~io::stderr()),
             };

src/libtest/lib.rs

@@ -415,8 +415,8 @@ impl<T: Writer> ConsoleTestState<T> {
             Some(ref path) => Some(try!(File::create(path))),
             None => None
         };
-        let out = match term::Terminal::new(io::stdout()) {
-            Err(_) => Raw(io::stdout()),
+        let out = match term::Terminal::new(io::stdio::stdout_raw()) {
+            Err(_) => Raw(io::stdio::stdout_raw()),
             Ok(t) => Pretty(t)
         };
         Ok(ConsoleTestState {

src/test/bench/shootout-fasta-redux.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use std::cmp::min;
-use std::io::{stdout, BufferedWriter, IoResult};
+use std::io::{stdout, IoResult};
 use std::os;
 use std::vec::bytes::copy_memory;
 use std::vec;
@@ -183,7 +183,7 @@ fn main() {
         5
     };
 
-    let mut out = BufferedWriter::new(stdout());
+    let mut out = stdout();
 
     out.write_line(">ONE Homo sapiens alu").unwrap();
     {

src/test/bench/shootout-fasta.rs

@@ -117,6 +117,6 @@ fn main() {
         let mut file = BufferedWriter::new(File::create(&Path::new("./shootout-fasta.data")));
         run(&mut file);
     } else {
-        run(&mut BufferedWriter::new(io::stdout()));
+        run(&mut io::stdout());
     }
 }

src/test/bench/shootout-mandelbrot.rs

@@ -9,7 +9,6 @@
 // except according to those terms.
 
 use std::io;
-use std::io::BufferedWriter;
 
 struct DummyWriter;
 impl Writer for DummyWriter {
@@ -27,7 +26,7 @@ fn main() {
         (1000, ~DummyWriter as ~Writer)
     } else {
         (from_str(args[1]).unwrap(),
-         ~BufferedWriter::new(std::io::stdout()) as ~Writer)
+         ~std::io::stdout() as ~Writer)
     };
     let h = w;
     let mut byte_acc = 0u8;