[llvm] 7d1cd8e - [Support] No longer require flushing raw_string_ostream

[Logan Smith via llvm-commits]

Author: Logan Smith
Date: 2022-01-07T09:25:22-08:00
New Revision: 7d1cd8e02636be7f4a177e53c77bb9b80d0f6376

URL: https://github.com/llvm/llvm-project/commit/7d1cd8e02636be7f4a177e53c77bb9b80d0f6376
DIFF: https://github.com/llvm/llvm-project/commit/7d1cd8e02636be7f4a177e53c77bb9b80d0f6376.diff

LOG: [Support] No longer require flushing raw_string_ostream

Since 65b13610a5226b84889b923bae884ba395ad084d, raw_string_ostream
has been unbuffered by default, making .flush() a no-op. This diff
formalizes this by no longer .flush()ing in the .str() method or
the destructor. .str() has been marked as "consider removing", since
its primary use case used to be making .flush()+access a one-liner,
and it also has issues such as preventing NRVO/implicit move when used
in return statements.

Differential Revision: https://reviews.llvm.org/D115421

Added: 
    

Modified: 
    llvm/include/llvm/Support/raw_ostream.h
    llvm/lib/Support/raw_ostream.cpp
    llvm/unittests/Support/raw_ostream_test.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/raw_ostream.h b/llvm/include/llvm/Support/raw_ostream.h
index 995b660695d28..fc46ec0d74564 100644
--- a/llvm/include/llvm/Support/raw_ostream.h
+++ b/llvm/include/llvm/Support/raw_ostream.h
@@ -625,6 +625,9 @@ class raw_fd_stream : public raw_fd_ostream {
 
 /// A raw_ostream that writes to an std::string.  This is a simple adaptor
 /// class. This class does not encounter output errors.
+/// raw_string_ostream operates without a buffer, delegating all memory
+/// management to the std::string. Thus the std::string is always up-to-date,
+/// may be used directly and there is no need to call flush().
 class raw_string_ostream : public raw_ostream {
   std::string &OS;
 
@@ -639,14 +642,11 @@ class raw_string_ostream : public raw_ostream {
   explicit raw_string_ostream(std::string &O) : OS(O) {
     SetUnbuffered();
   }
-  ~raw_string_ostream() override;
 
-  /// Flushes the stream contents to the target string and returns  the string's
-  /// reference.
-  std::string& str() {
-    flush();
-    return OS;
-  }
+  /// Returns the string's reference. In most cases it is better to simply use
+  /// the underlying std::string directly.
+  /// TODO: Consider removing this API.
+  std::string &str() { return OS; }
 
   void reserveExtraSpace(uint64_t ExtraSize) override {
     OS.reserve(tell() + ExtraSize);

diff  --git a/llvm/lib/Support/raw_ostream.cpp b/llvm/lib/Support/raw_ostream.cpp
index 6881c7be039cd..073f370ba34e3 100644
--- a/llvm/lib/Support/raw_ostream.cpp
+++ b/llvm/lib/Support/raw_ostream.cpp
@@ -937,10 +937,6 @@ bool raw_fd_stream::classof(const raw_ostream *OS) {
 //  raw_string_ostream
 //===----------------------------------------------------------------------===//
 
-raw_string_ostream::~raw_string_ostream() {
-  flush();
-}
-
 void raw_string_ostream::write_impl(const char *Ptr, size_t Size) {
   OS.append(Ptr, Size);
 }

diff  --git a/llvm/unittests/Support/raw_ostream_test.cpp b/llvm/unittests/Support/raw_ostream_test.cpp
index 6884ea568ff81..74587c76236a6 100644
--- a/llvm/unittests/Support/raw_ostream_test.cpp
+++ b/llvm/unittests/Support/raw_ostream_test.cpp
@@ -21,11 +21,10 @@ namespace {
 
 template<typename T> std::string printToString(const T &Value) {
   std::string res;
-  {
-    llvm::raw_string_ostream OS(res);
-    OS.SetBuffered();
-    OS << Value;
-  }
+  llvm::raw_string_ostream OS(res);
+  OS.SetBuffered();
+  OS << Value;
+  OS.flush();
   return res;
 }
 
@@ -141,7 +140,8 @@ TEST(raw_ostreamTest, TinyBuffer) {
   OS << "hello";
   OS << 1;
   OS << 'w' << 'o' << 'r' << 'l' << 'd';
-  EXPECT_EQ("hello1world", OS.str());
+  OS.flush();
+  EXPECT_EQ("hello1world", Str);
 }
 
 TEST(raw_ostreamTest, WriteEscaped) {
@@ -457,6 +457,9 @@ TEST(raw_ostreamTest, flush_tied_to_stream_on_write) {
   TiedTo << "y";
   TiedStream << "0";
   EXPECT_EQ("acegostuv", TiedToBuffer);
+
+  TiedTo.flush();
+  TiedStream.flush();
 }
 
 TEST(raw_ostreamTest, reserve_stream) {