From 1ce831912c797df1cb6d313d8e576a3f86175b6d Mon Sep 17 00:00:00 2001 From: James Henderson Date: Mon, 8 Jun 2020 12:58:04 +0100 Subject: [PATCH] [Support] Add stream tie function and use it for errs() errs() is now tied to outs() so that if something prints to errs(), outs() will be flushed before the printing occurs. This avoids interleaving output between the two and is consistent with standard cout and cerr behaviour. Reviewed by: labath, JDevlieghere, MaskRay Differential Revision: https://reviews.llvm.org/D81156 --- llvm/include/llvm/Support/raw_ostream.h | 18 ++++++- llvm/lib/Support/raw_ostream.cpp | 17 +++++-- llvm/unittests/Support/raw_ostream_test.cpp | 78 +++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/Support/raw_ostream.h b/llvm/include/llvm/Support/raw_ostream.h index 504de39..8d289f7 100644 --- a/llvm/include/llvm/Support/raw_ostream.h +++ b/llvm/include/llvm/Support/raw_ostream.h @@ -66,6 +66,10 @@ private: char *OutBufStart, *OutBufEnd, *OutBufCur; bool ColorEnabled = false; + /// Optional stream this stream is tied to. If this stream is written to, the + /// tied-to stream will be flushed first. + raw_ostream *TiedStream = nullptr; + enum class BufferKind { Unbuffered = 0, InternalBuffer, @@ -294,6 +298,10 @@ public: // changeColor() has no effect until enable_colors(true) is called. virtual void enable_colors(bool enable) { ColorEnabled = enable; } + /// Tie this stream to the specified stream. Replaces any existing tied-to + /// stream. Specifying a nullptr unties the stream. + void tie(raw_ostream *TieTo) { TiedStream = TieTo; } + //===--------------------------------------------------------------------===// // Subclass Interface //===--------------------------------------------------------------------===// @@ -352,6 +360,9 @@ private: /// flushing. The result is affected by calls to enable_color(). bool prepare_colors(); + /// Flush the tied-to stream (if present) and then write the required data. + void flush_tied_then_write(const char *Ptr, size_t Size); + virtual void anchor(); }; @@ -491,8 +502,11 @@ public: /// like: outs() << "foo" << "bar"; raw_fd_ostream &outs(); -/// This returns a reference to a raw_fd_ostream for standard error. Use it -/// like: errs() << "foo" << "bar"; +/// This returns a reference to a raw_ostream for standard error. +/// Use it like: errs() << "foo" << "bar"; +/// By default, the stream is tied to stdout to ensure stdout is flushed before +/// stderr is written, to ensure the error messages are written in their +/// expected place. raw_fd_ostream &errs(); /// This returns a reference to a raw_ostream which simply discards output. diff --git a/llvm/lib/Support/raw_ostream.cpp b/llvm/lib/Support/raw_ostream.cpp index bd0a5d4..c06ea55 100644 --- a/llvm/lib/Support/raw_ostream.cpp +++ b/llvm/lib/Support/raw_ostream.cpp @@ -216,7 +216,7 @@ void raw_ostream::flush_nonempty() { assert(OutBufCur > OutBufStart && "Invalid call to flush_nonempty."); size_t Length = OutBufCur - OutBufStart; OutBufCur = OutBufStart; - write_impl(OutBufStart, Length); + flush_tied_then_write(OutBufStart, Length); } raw_ostream &raw_ostream::write(unsigned char C) { @@ -224,7 +224,7 @@ raw_ostream &raw_ostream::write(unsigned char C) { if (LLVM_UNLIKELY(OutBufCur >= OutBufEnd)) { if (LLVM_UNLIKELY(!OutBufStart)) { if (BufferMode == BufferKind::Unbuffered) { - write_impl(reinterpret_cast(&C), 1); + flush_tied_then_write(reinterpret_cast(&C), 1); return *this; } // Set up a buffer and start over. @@ -244,7 +244,7 @@ raw_ostream &raw_ostream::write(const char *Ptr, size_t Size) { if (LLVM_UNLIKELY(size_t(OutBufEnd - OutBufCur) < Size)) { if (LLVM_UNLIKELY(!OutBufStart)) { if (BufferMode == BufferKind::Unbuffered) { - write_impl(Ptr, Size); + flush_tied_then_write(Ptr, Size); return *this; } // Set up a buffer and start over. @@ -260,7 +260,7 @@ raw_ostream &raw_ostream::write(const char *Ptr, size_t Size) { if (LLVM_UNLIKELY(OutBufCur == OutBufStart)) { assert(NumBytes != 0 && "undefined behavior"); size_t BytesToWrite = Size - (Size % NumBytes); - write_impl(Ptr, BytesToWrite); + flush_tied_then_write(Ptr, BytesToWrite); size_t BytesRemaining = Size - BytesToWrite; if (BytesRemaining > size_t(OutBufEnd - OutBufCur)) { // Too much left over to copy into our buffer. @@ -301,6 +301,12 @@ void raw_ostream::copy_to_buffer(const char *Ptr, size_t Size) { OutBufCur += Size; } +void raw_ostream::flush_tied_then_write(const char *Ptr, size_t Size) { + if (TiedStream) + TiedStream->flush(); + write_impl(Ptr, Size); +} + // Formatted output. raw_ostream &raw_ostream::operator<<(const format_object_base &Fmt) { // If we have more than a few bytes left in our output buffer, try @@ -870,8 +876,9 @@ raw_fd_ostream &llvm::outs() { } raw_fd_ostream &llvm::errs() { - // Set standard error to be unbuffered by default. + // Set standard error to be unbuffered and tied to outs() by default. static raw_fd_ostream S(STDERR_FILENO, false, true); + S.tie(&outs()); return S; } diff --git a/llvm/unittests/Support/raw_ostream_test.cpp b/llvm/unittests/Support/raw_ostream_test.cpp index 267ef19..55ac5d0 100644 --- a/llvm/unittests/Support/raw_ostream_test.cpp +++ b/llvm/unittests/Support/raw_ostream_test.cpp @@ -376,4 +376,82 @@ TEST(raw_fd_ostreamTest, multiple_raw_fd_ostream_to_stdout) { { raw_fd_ostream("-", EC, sys::fs::OpenFlags::OF_None); } { raw_fd_ostream("-", EC, sys::fs::OpenFlags::OF_None); } } + +TEST(raw_ostreamTest, flush_tied_to_stream_on_write) { + std::string TiedToBuffer; + raw_string_ostream TiedTo(TiedToBuffer); + TiedTo.SetBuffered(); + TiedTo << "a"; + + std::string Buffer; + raw_string_ostream TiedStream(Buffer); + TiedStream.tie(&TiedTo); + // Sanity check that the stream hasn't already been flushed. + EXPECT_EQ("", TiedToBuffer); + + // Empty string doesn't cause a flush of TiedTo. + TiedStream << ""; + EXPECT_EQ("", TiedToBuffer); + + // Non-empty strings trigger flush of TiedTo. + TiedStream << "abc"; + EXPECT_EQ("a", TiedToBuffer); + + // Single char write flushes TiedTo. + TiedTo << "c"; + TiedStream << 'd'; + EXPECT_EQ("ac", TiedToBuffer); + + // Write to buffered stream without flush does not flush TiedTo. + TiedStream.SetBuffered(); + TiedStream.SetBufferSize(2); + TiedTo << "e"; + TiedStream << "f"; + EXPECT_EQ("ac", TiedToBuffer); + + // Explicit flush of buffered stream flushes TiedTo. + TiedStream.flush(); + EXPECT_EQ("ace", TiedToBuffer); + + // Explicit flush of buffered stream with empty buffer does not flush TiedTo. + TiedTo << "g"; + TiedStream.flush(); + EXPECT_EQ("ace", TiedToBuffer); + + // Write of data to empty buffer that is greater than buffer size flushes + // TiedTo. + TiedStream << "hijklm"; + EXPECT_EQ("aceg", TiedToBuffer); + + // Write of data that overflows buffer size also flushes TiedTo. + TiedStream.flush(); + TiedStream << "n"; + TiedTo << "o"; + TiedStream << "pq"; + EXPECT_EQ("acego", TiedToBuffer); + + // Streams can be tied to each other safely. + TiedStream.flush(); + Buffer = ""; + TiedTo.tie(&TiedStream); + TiedTo.SetBufferSize(2); + TiedStream << "r"; + TiedTo << "s"; + EXPECT_EQ("", Buffer); + EXPECT_EQ("acego", TiedToBuffer); + TiedTo << "tuv"; + EXPECT_EQ("r", Buffer); + TiedStream << "wxy"; + EXPECT_EQ("acegostuv", TiedToBuffer); + // The x remains in the buffer, since it was written after the flush of + // TiedTo. + EXPECT_EQ("rwx", Buffer); + + // Calling tie with nullptr unties stream. + TiedStream.SetUnbuffered(); + TiedStream.tie(nullptr); + TiedTo << "y"; + TiedStream << "0"; + EXPECT_EQ("acegostuv", TiedToBuffer); +} } -- 2.7.4