Initialize raw_ostream buffer mode using an enum instead of a boolean

[mehdi_amini]

Comment the new enum

[dexonsmith]

Resigning to take this off my queue since this is 5+ years old, but this looks like a nice change that I'd be happy to review. Add me back if you want me to take a look at this or a rebased patch.

[mehdi_amini]

I can try to bring this back if there is interest in this.

As far as I remember I got feedback that this wasn't worth it compared to a
bool and a comment at call sites (was it Justin's feedback maybe?)
Basically:

raw_fd_ostream OF(TestFD, true, /*unbuffered=*/true);

vs

raw_fd_ostream OF(TestFD, true, StreamMode::Unbuffered);

[dexonsmith]

Hmm, that rings a bell. I don't feel strongly either way.

include/llvm/Support/circular_raw_ostream.h

public:
     /// buffering to be happening underneath it.
     ///
     /// "Owns" tells the circular_raw_ostream whether it is
     /// responsible for managing the held stream, doing memory
     /// management of it, etc.
     ///
     circular_raw_ostream(raw_ostream &Stream, const char *Header,
                          size_t BuffSize = 0, bool Owns = REFERENCE_ONLY)
-        : raw_ostream(/*unbuffered*/ true), TheStream(nullptr),
+        : raw_ostream(StreamMode::Unbuffered), TheStream(nullptr),
           OwnsStream(Owns), BufferSize(BuffSize), BufferArray(nullptr),
           Filled(false), Banner(Header) {
       if (BufferSize != 0)
         BufferArray = new char[BufferSize];
       Cur = BufferArray;
       setStream(Stream, Owns);
     }
 

include/llvm/Support/raw_ostream.h

 template <typename T> class SmallVectorImpl;
 
 namespace sys {
 namespace fs {
 enum OpenFlags : unsigned;
 }
 }
 
+/// Enable or disable buffering when creating a raw_ostream-derived class.
+enum class StreamMode : bool { Buffered, Unbuffered };
+
 /// This class implements an extremely fast bulk output stream that can *only*
 /// output to a stream.  It does not support seeking, reopening, rewinding, line
 /// buffered disciplines etc. It is a simple buffer that outputs
 /// a chunk at a time.
 class raw_ostream {
 private:
   void operator=(const raw_ostream &) = delete;
   raw_ostream(const raw_ostream &) = delete;
private:
   /// InternalBuffer); otherwise the buffer has been set via SetBuffer and is
   /// managed by the subclass.
   ///
   /// If a subclass installs an external buffer using SetBuffer then it can wait
   /// for a \see write_impl() call to handle the data which has been put into
   /// this buffer.
   char *OutBufStart, *OutBufEnd, *OutBufCur;
 
-  enum BufferKind {
-    Unbuffered = 0,
-    InternalBuffer,
-    ExternalBuffer
-  } BufferMode;
+  /// Specify the current state of the stream.
+  enum BufferKind { Unbuffered, InternalBuffer, ExternalBuffer } BufferMode;
 
 public:
   // color order matches ANSI escape sequence, don't change
   enum Colors {
     BLACK=0,
     RED,
     GREEN,
     YELLOW,
     BLUE,
     MAGENTA,
     CYAN,
     WHITE,
     SAVEDCOLOR
   };
 
-  explicit raw_ostream(bool unbuffered = false)
-      : BufferMode(unbuffered ? Unbuffered : InternalBuffer) {
+  explicit raw_ostream(StreamMode Mode = StreamMode::Buffered)
+      : BufferMode(Mode == StreamMode::Buffered ? InternalBuffer : Unbuffered) {
     // Start out ready to flush.
     OutBufStart = OutBufEnd = OutBufCur = nullptr;
   }
 
   virtual ~raw_ostream();
 
   /// tell - Return the current offset with the file.
   uint64_t tell() const { return current_pos() + GetNumBytesInBuffer(); }

 
 /// An abstract base class for streams implementations that also support a
 /// pwrite operation. This is usefull for code that can mostly stream out data,
 /// but needs to patch in a header that needs to know the output size.
 class raw_pwrite_stream : public raw_ostream {
   virtual void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) = 0;
 
 public:
-  explicit raw_pwrite_stream(bool Unbuffered = false)
-      : raw_ostream(Unbuffered) {}
+  explicit raw_pwrite_stream(StreamMode Mode = StreamMode::Buffered)
+      : raw_ostream(Mode) {}
   void pwrite(const char *Ptr, size_t Size, uint64_t Offset) {
 #ifndef NDBEBUG
     uint64_t Pos = tell();
     // /dev/null always reports a pos of 0, so we cannot perform this check
     // in that case.
     if (Pos)
       assert(Size + Offset <= Pos && "We don't support extending the stream");
 #endif
public:
   /// itself to own the file descriptor. In particular, it will close the
   /// file descriptor when it is done (this is necessary to detect
   /// output errors).
   raw_fd_ostream(StringRef Filename, std::error_code &EC,
                  sys::fs::OpenFlags Flags);
 
   /// FD is the file descriptor that this writes to.  If ShouldClose is true,
   /// this closes the file when the stream is destroyed.
-  raw_fd_ostream(int fd, bool shouldClose, bool unbuffered=false);
+  raw_fd_ostream(int fd, bool shouldClose,
+                 StreamMode Mode = StreamMode::Buffered);
 
   ~raw_fd_ostream() override;
 
   /// Manually flush the stream and close the file. Note that this does not call
   /// fsync.
   void close();
 
   bool supportsSeeking() { return SupportsSeeking; }

lib/Bitcode/Writer/BitWriter.cpp

if (EC)
     return -1;
 
   WriteBitcodeToFile(unwrap(M), OS);
   return 0;
 }
 
 int LLVMWriteBitcodeToFD(LLVMModuleRef M, int FD, int ShouldClose,
                          int Unbuffered) {
-  raw_fd_ostream OS(FD, ShouldClose, Unbuffered);
+  StreamMode Mode = Unbuffered ? StreamMode::Unbuffered : StreamMode::Buffered;
+  raw_fd_ostream OS(FD, ShouldClose, Mode);
 
   WriteBitcodeToFile(unwrap(M), OS);
   return 0;
 }
 
 int LLVMWriteBitcodeToFileHandle(LLVMModuleRef M, int FileHandle) {
   return LLVMWriteBitcodeToFD(M, FileHandle, true, false);
 }
 
 LLVMMemoryBufferRef LLVMWriteBitcodeToMemoryBuffer(LLVMModuleRef M) {
   std::string Data;
   raw_string_ostream OS(Data);
 
   WriteBitcodeToFile(unwrap(M), OS);
   return wrap(MemoryBuffer::getMemBufferCopy(OS.str()).release());
 }

lib/Support/raw_ostream.cpp

 }
 
 raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
                                sys::fs::OpenFlags Flags)
     : raw_fd_ostream(getFD(Filename, EC, Flags), true) {}
 
 /// FD is the file descriptor that this writes to.  If ShouldClose is true, this
 /// closes the file when the stream is destroyed.
-raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, bool unbuffered)
-    : raw_pwrite_stream(unbuffered), FD(fd), ShouldClose(shouldClose),
-      Error(false), UseAtomicWrites(false) {
+raw_fd_ostream::raw_fd_ostream(int fd, bool shouldClose, StreamMode Mode)
+    : raw_pwrite_stream(Mode), FD(fd), ShouldClose(shouldClose), Error(false),
+      UseAtomicWrites(false) {
   if (FD < 0 ) {
     ShouldClose = false;
     return;
   }
 
   // Get the starting position.
   off_t loc = ::lseek(FD, 0, SEEK_CUR);
 #ifdef LLVM_ON_WIN32
raw_ostream &llvm::outs() {
   assert(!EC);
   return S;
 }
 
 /// errs() - This returns a reference to a raw_ostream for standard error.
 /// Use it like: errs() << "foo" << "bar";
 raw_ostream &llvm::errs() {
   // Set standard error to be unbuffered by default.
-  static raw_fd_ostream S(STDERR_FILENO, false, true);
+  static raw_fd_ostream S(STDERR_FILENO, false, StreamMode::Unbuffered);
   return S;
 }
 
 /// nulls() - This returns a reference to a raw_ostream which discards output.
 raw_ostream &llvm::nulls() {
   static raw_null_ostream S;
   return S;
 }

unittests/Support/MemoryBufferTest.cpp

 
 TEST_F(MemoryBufferTest, NullTerminator4K) {
   // Test that a file with size that is a multiple of the page size can be null
   // terminated correctly by MemoryBuffer.
   int TestFD;
   SmallString<64> TestPath;
   sys::fs::createTemporaryFile("MemoryBufferTest_NullTerminator4K", "temp",
                                TestFD, TestPath);
-  raw_fd_ostream OF(TestFD, true, /*unbuffered=*/true);
+  raw_fd_ostream OF(TestFD, true, StreamMode::Unbuffered);
   for (unsigned i = 0; i < 4096 / 16; ++i) {
     OF << "0123456789abcdef";
   }
   OF.close();
 
   ErrorOr<OwningBuffer> MB = MemoryBuffer::getFile(TestPath.c_str());
   std::error_code EC = MB.getError();
   ASSERT_FALSE(EC);
void MemoryBufferTest::testGetOpenFileSlice(bool Reopen) {
   // the usage of memory mapping.
   int TestFD;
   SmallString<64> TestPath;
   // Create a temporary file and write data into it.
   sys::fs::createTemporaryFile("prefix", "temp", TestFD, TestPath);
   // OF is responsible for closing the file; If the file is not
   // reopened, it will be unbuffered so that the results are
   // immediately visible through the fd.
-  raw_fd_ostream OF(TestFD, true, !Reopen);
+  raw_fd_ostream OF(TestFD, true,
+                    Reopen ? StreamMode::Buffered : StreamMode::Unbuffered);
   for (int i = 0; i < 60000; ++i) {
     OF << "0123456789";
   }
 
   if (Reopen) {
     OF.close();
     EXPECT_FALSE(sys::fs::openFileForRead(TestPath.c_str(), TestFD));
   }

 }
 
 
 TEST_F(MemoryBufferTest, slice) {
   // Create a file that is six pages long with different data on each page.
   int FD;
   SmallString<64> TestPath;
   sys::fs::createTemporaryFile("MemoryBufferTest_Slice", "temp", FD, TestPath);
-  raw_fd_ostream OF(FD, true, /*unbuffered=*/true);
+  raw_fd_ostream OF(FD, true, StreamMode::Unbuffered);
   for (unsigned i = 0; i < 0x2000 / 8; ++i) {
     OF << "12345678";
   }
   for (unsigned i = 0; i < 0x2000 / 8; ++i) {
     OF << "abcdefgh";
   }
   for (unsigned i = 0; i < 0x2000 / 8; ++i) {
     OF << "ABCDEFGH";

unittests/Support/SpecialCaseListTest.cpp

std::unique_ptr<SpecialCaseList> makeSpecialCaseList(StringRef List) {
     assert(Error == "");
     return SCL;
   }
 
   std::string makeSpecialCaseListFile(StringRef Contents) {
     int FD;
     SmallString<64> Path;
     sys::fs::createTemporaryFile("SpecialCaseListTest", "temp", FD, Path);
-    raw_fd_ostream OF(FD, true, true);
+    raw_fd_ostream OF(FD, true, StreamMode::Unbuffered);
     OF << Contents;
     OF.close();
     return Path.str();
   }
 };
 
 TEST_F(SpecialCaseListTest, Basic) {
   std::unique_ptr<SpecialCaseList> SCL =