Implement correct move constructor and assignment for absl::strings_internal::OStringStream, and mark that class final.

This should be explicit per https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types, and is currently hard to infer due to multiple inheritance.

PiperOrigin-RevId: 474311032
Change-Id: I72d7adcb9f7a991c19c26bc7083a4df31de5da49

absl/strings/internal/ostringstream.cc

@@ -14,20 +14,27 @@
 
 #include "absl/strings/internal/ostringstream.h"
 
+#include <cassert>
+#include <cstddef>
+#include <ios>
+#include <streambuf>
+
 namespace absl {
 ABSL_NAMESPACE_BEGIN
 namespace strings_internal {
 
-OStringStream::Buf::int_type OStringStream::overflow(int c) {
-  assert(s_);
-  if (!Buf::traits_type::eq_int_type(c, Buf::traits_type::eof()))
-    s_->push_back(static_cast<char>(c));
+OStringStream::Streambuf::int_type OStringStream::Streambuf::overflow(int c) {
+  assert(str_);
+  if (!std::streambuf::traits_type::eq_int_type(
+          c, std::streambuf::traits_type::eof()))
+    str_->push_back(static_cast<char>(c));
   return 1;
 }
 
-std::streamsize OStringStream::xsputn(const char* s, std::streamsize n) {
-  assert(s_);
-  s_->append(s, static_cast<size_t>(n));
+std::streamsize OStringStream::Streambuf::xsputn(const char* s,
+                                                 std::streamsize n) {
+  assert(str_);
+  str_->append(s, static_cast<size_t>(n));
   return n;
 }
 

absl/strings/internal/ostringstream.h

@@ -16,11 +16,13 @@
 #define ABSL_STRINGS_INTERNAL_OSTRINGSTREAM_H_
 
 #include <cassert>
+#include <ios>
 #include <ostream>
 #include <streambuf>
 #include <string>
+#include <utility>
 
-#include "absl/base/port.h"
+#include "absl/base/config.h"
 
 namespace absl {
 ABSL_NAMESPACE_BEGIN
@@ -60,26 +62,49 @@ namespace strings_internal {
 //   strm << 3.14;
 //
 // Note: flush() has no effect. No reason to call it.
-class OStringStream : private std::basic_streambuf<char>, public std::ostream {
+class OStringStream final : public std::ostream {
  public:
   // The argument can be null, in which case you'll need to call str(p) with a
   // non-null argument before you can write to the stream.
   //
   // The destructor of OStringStream doesn't use the std::string. It's OK to
   // destroy the std::string before the stream.
-  explicit OStringStream(std::string* s) : std::ostream(this), s_(s) {}
+  explicit OStringStream(std::string* str)
+      : std::ostream(&buf_), buf_(str) {}
+  OStringStream(OStringStream&& that)
+      : std::ostream(std::move(static_cast<std::ostream&>(that))),
+        buf_(that.buf_) {
+    rdbuf(&buf_);
+  }
+  OStringStream& operator=(OStringStream&& that) {
+    std::ostream::operator=(std::move(static_cast<std::ostream&>(that)));
+    buf_ = that.buf_;
+    rdbuf(&buf_);
+    return *this;
+  }
 
-  std::string* str() { return s_; }
-  const std::string* str() const { return s_; }
-  void str(std::string* s) { s_ = s; }
+  std::string* str() { return buf_.str(); }
+  const std::string* str() const { return buf_.str(); }
+  void str(std::string* str) { buf_.str(str); }
 
  private:
-  using Buf = std::basic_streambuf<char>;
+  class Streambuf final : public std::streambuf {
+   public:
+    explicit Streambuf(std::string* str) : str_(str) {}
+    Streambuf(const Streambuf&) = default;
+    Streambuf& operator=(const Streambuf&) = default;
 
-  Buf::int_type overflow(int c) override;
-  std::streamsize xsputn(const char* s, std::streamsize n) override;
+    std::string* str() { return str_; }
+    const std::string* str() const { return str_; }
+    void str(std::string* str) { str_ = str; }
 
-  std::string* s_;
+   protected:
+    int_type overflow(int c) override;
+    std::streamsize xsputn(const char* s, std::streamsize n) override;
+
+   private:
+    std::string* str_;
+  } buf_;
 };
 
 }  // namespace strings_internal

absl/strings/internal/ostringstream_test.cc

@@ -14,10 +14,12 @@
 
 #include "absl/strings/internal/ostringstream.h"
 
+#include <ios>
 #include <memory>
 #include <ostream>
 #include <string>
 #include <type_traits>
+#include <utility>
 
 #include "gtest/gtest.h"
 
@@ -29,24 +31,51 @@ TEST(OStringStream, IsOStream) {
       "");
 }
 
-TEST(OStringStream, ConstructDestroy) {
+TEST(OStringStream, ConstructNullptr) {
+  absl::strings_internal::OStringStream strm(nullptr);
+  EXPECT_EQ(nullptr, strm.str());
+}
+
+TEST(OStringStream, ConstructStr) {
+  std::string s = "abc";
   {
-    absl::strings_internal::OStringStream strm(nullptr);
-    EXPECT_EQ(nullptr, strm.str());
+    absl::strings_internal::OStringStream strm(&s);
+    EXPECT_EQ(&s, strm.str());
   }
+  EXPECT_EQ("abc", s);
+}
+
+TEST(OStringStream, Destroy) {
+  std::unique_ptr<std::string> s(new std::string);
+  absl::strings_internal::OStringStream strm(s.get());
+  s.reset();
+}
+
+TEST(OStringStream, MoveConstruct) {
+  std::string s = "abc";
   {
-    std::string s = "abc";
-    {
-      absl::strings_internal::OStringStream strm(&s);
-      EXPECT_EQ(&s, strm.str());
-    }
-    EXPECT_EQ("abc", s);
+    absl::strings_internal::OStringStream strm1(&s);
+    strm1 << std::hex << 16;
+    EXPECT_EQ(&s, strm1.str());
+    absl::strings_internal::OStringStream strm2(std::move(strm1));
+    strm2 << 16;  // We should still be in base 16.
+    EXPECT_EQ(&s, strm2.str());
   }
+  EXPECT_EQ("abc1010", s);
+}
+
+TEST(OStringStream, MoveAssign) {
+  std::string s = "abc";
   {
-    std::unique_ptr<std::string> s(new std::string);
-    absl::strings_internal::OStringStream strm(s.get());
-    s.reset();
+    absl::strings_internal::OStringStream strm1(&s);
+    strm1 << std::hex << 16;
+    EXPECT_EQ(&s, strm1.str());
+    absl::strings_internal::OStringStream strm2(nullptr);
+    strm2 = std::move(strm1);
+    strm2 << 16;  // We should still be in base 16.
+    EXPECT_EQ(&s, strm2.str());
   }
+  EXPECT_EQ("abc1010", s);
 }
 
 TEST(OStringStream, Str) {