Skip to content

Conversation

@cachemeifyoucan
Copy link
Collaborator

No description provided.

Created using spr 1.3.5
@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

@llvm/pr-subscribers-llvm-support

Author: Steven Wu (cachemeifyoucan)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/113361.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Support/raw_ostream_proxy.h (+23-9)
  • (modified) llvm/unittests/Support/raw_ostream_proxy_test.cpp (+3-3)
diff --git a/llvm/include/llvm/Support/raw_ostream_proxy.h b/llvm/include/llvm/Support/raw_ostream_proxy.h
index 093d0a927833f1..d661ccbbbd616c 100644
--- a/llvm/include/llvm/Support/raw_ostream_proxy.h
+++ b/llvm/include/llvm/Support/raw_ostream_proxy.h
@@ -17,7 +17,6 @@ namespace llvm {
 /// template instantions.
 class raw_ostream_proxy_adaptor_base {
 protected:
-  raw_ostream_proxy_adaptor_base() = delete;
   raw_ostream_proxy_adaptor_base(const raw_ostream_proxy_adaptor_base &) =
       delete;
 
@@ -72,8 +71,7 @@ class raw_ostream_proxy_adaptor_base {
 /// changeColor(), resetColor(), and \a reverseColor() are not forwarded, since
 /// they need to call \a flush() and the buffer lives in the proxy.
 template <class RawOstreamT = raw_ostream>
-class raw_ostream_proxy_adaptor : public RawOstreamT,
-                                  public raw_ostream_proxy_adaptor_base {
+class raw_ostream_proxy_adaptor : public RawOstreamT {
   void write_impl(const char *Ptr, size_t Size) override {
     getProxiedOS().write(Ptr, Size);
   }
@@ -92,23 +90,39 @@ class raw_ostream_proxy_adaptor : public RawOstreamT,
     RawOstreamT::enable_colors(enable);
     getProxiedOS().enable_colors(enable);
   }
+  bool hasProxiedOS() const { return OS; }
+  raw_ostream &getProxiedOS() const {
+    assert(OS && "raw_ostream_proxy_adaptor use after reset");
+    return *OS;
+  }
+  size_t getPreferredBufferSize() const { return PreferredBufferSize; }
 
   ~raw_ostream_proxy_adaptor() override { resetProxiedOS(); }
 
 protected:
   template <class... ArgsT>
   explicit raw_ostream_proxy_adaptor(raw_ostream &OS, ArgsT &&...Args)
-      : RawOstreamT(std::forward<ArgsT>(Args)...),
-        raw_ostream_proxy_adaptor_base(OS) {}
+      : RawOstreamT(std::forward<ArgsT>(Args)...), OS(&OS),
+        PreferredBufferSize(OS.GetBufferSize()) {
+    // Drop OS's buffer to make this->flush() forward. This proxy will add a
+    // buffer in its place.
+    OS.SetUnbuffered();
+  }
 
   /// Stop proxying the stream. Flush and set up a crash for future writes.
   ///
   /// For example, this can simplify logic when a subclass might have a longer
   /// lifetime than the stream it proxies.
   void resetProxiedOS() {
-    raw_ostream_proxy_adaptor_base::resetProxiedOS(*this);
+    OS->SetUnbuffered();
+    OS = nullptr;
   }
-  void resetProxiedOS(raw_ostream &) = delete;
+
+private:
+  raw_ostream *OS;
+
+  /// Caches the value of OS->GetBufferSize() at construction time.
+  size_t PreferredBufferSize;
 };
 
 /// Adaptor for creating a stream that proxies a \a raw_pwrite_stream.
@@ -134,7 +148,7 @@ class raw_pwrite_stream_proxy_adaptor
   }
 };
 
-/// Non-owning proxy for a \a raw_ostream. Enables passing a stream into of an
+/// Non-owning proxy for a \a raw_ostream. Enables passing a stream into an
 /// API that takes ownership.
 class raw_ostream_proxy : public raw_ostream_proxy_adaptor<> {
   void anchor() override;
@@ -144,7 +158,7 @@ class raw_ostream_proxy : public raw_ostream_proxy_adaptor<> {
 };
 
 /// Non-owning proxy for a \a raw_pwrite_stream. Enables passing a stream
-/// into of an API that takes ownership.
+/// into an API that takes ownership.
 class raw_pwrite_stream_proxy : public raw_pwrite_stream_proxy_adaptor<> {
   void anchor() override;
 
diff --git a/llvm/unittests/Support/raw_ostream_proxy_test.cpp b/llvm/unittests/Support/raw_ostream_proxy_test.cpp
index ee97fe65b66003..1233ed54fd5a52 100644
--- a/llvm/unittests/Support/raw_ostream_proxy_test.cpp
+++ b/llvm/unittests/Support/raw_ostream_proxy_test.cpp
@@ -22,7 +22,7 @@ class BufferedNoPwriteSmallVectorStream : public raw_ostream {
 public:
   // Choose a strange buffer size to ensure it doesn't collide with the default
   // on \a raw_ostream.
-  constexpr static const size_t PreferredBufferSize = 63;
+  static constexpr size_t PreferredBufferSize = 63;
 
   size_t preferred_buffer_size() const override { return PreferredBufferSize; }
   uint64_t current_pos() const override { return Vector.size(); }
@@ -40,7 +40,7 @@ class BufferedNoPwriteSmallVectorStream : public raw_ostream {
   bool IsDisplayed = false;
 };
 
-constexpr const size_t BufferedNoPwriteSmallVectorStream::PreferredBufferSize;
+constexpr size_t BufferedNoPwriteSmallVectorStream::PreferredBufferSize;
 
 TEST(raw_ostream_proxyTest, write) {
   // Besides confirming that "write" works, this test confirms that the proxy
@@ -157,7 +157,7 @@ TEST(raw_ostream_proxyTest, resetProxiedOS) {
   EXPECT_EQ(0u, ProxyOS.GetBufferSize());
   EXPECT_FALSE(ProxyOS.hasProxiedOS());
 
-#if GTEST_HAS_DEATH_TEST
+#if GTEST_HAS_DEATH_TEST && !defined(NDEBUG)
   EXPECT_DEATH(ProxyOS << "e", "use after reset");
   EXPECT_DEATH(ProxyOS.getProxiedOS(), "use after reset");
 #endif

@cachemeifyoucan cachemeifyoucan deleted the users/cachemeifyoucan/spr/address-review-feedback branch October 29, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants