[llvm] Support: Add proxies for raw_ostream and raw_pwrite_stream (PR #113362)
Steven Wu via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 3 14:00:14 PST 2024
https://github.com/cachemeifyoucan updated https://github.com/llvm/llvm-project/pull/113362
>From 664b08a613d9a6727f2f7a3277a77816b42993ba Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Tue, 22 Oct 2024 11:57:29 -0700
Subject: [PATCH 1/5] rebase and review comments
Created using spr 1.3.5
---
llvm/include/llvm/Support/raw_ostream_proxy.h | 158 +++++++++++++
llvm/lib/Support/CMakeLists.txt | 1 +
llvm/lib/Support/raw_ostream_proxy.cpp | 15 ++
llvm/unittests/Support/CMakeLists.txt | 1 +
.../Support/raw_ostream_proxy_test.cpp | 219 ++++++++++++++++++
5 files changed, 394 insertions(+)
create mode 100644 llvm/include/llvm/Support/raw_ostream_proxy.h
create mode 100644 llvm/lib/Support/raw_ostream_proxy.cpp
create mode 100644 llvm/unittests/Support/raw_ostream_proxy_test.cpp
diff --git a/llvm/include/llvm/Support/raw_ostream_proxy.h b/llvm/include/llvm/Support/raw_ostream_proxy.h
new file mode 100644
index 00000000000000..093d0a927833f1
--- /dev/null
+++ b/llvm/include/llvm/Support/raw_ostream_proxy.h
@@ -0,0 +1,158 @@
+//===- raw_ostream_proxy.h - Proxies for raw output streams -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_RAW_OSTREAM_PROXY_H
+#define LLVM_SUPPORT_RAW_OSTREAM_PROXY_H
+
+#include "llvm/Support/raw_ostream.h"
+
+namespace llvm {
+
+/// Common bits for \a raw_ostream_proxy_adaptor<>, split out to dedup in
+/// 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;
+
+ explicit raw_ostream_proxy_adaptor_base(raw_ostream &OS)
+ : 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();
+ }
+
+ ~raw_ostream_proxy_adaptor_base() {
+ assert(!OS && "Derived objects should call resetProxiedOS()");
+ }
+
+ /// Stop proxying the stream, taking the derived object by reference as \p
+ /// ThisProxyOS. Updates \p ThisProxyOS to stop buffering before setting \a
+ /// OS to \c nullptr, ensuring that future writes crash immediately.
+ void resetProxiedOS(raw_ostream &ThisProxyOS) {
+ ThisProxyOS.SetUnbuffered();
+ OS = nullptr;
+ }
+
+ 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; }
+
+private:
+ raw_ostream *OS;
+
+ /// Caches the value of OS->GetBufferSize() at construction time.
+ size_t PreferredBufferSize;
+};
+
+/// Adaptor to create a stream class that proxies another \a raw_ostream.
+///
+/// Use \a raw_ostream_proxy_adaptor<> directly to implement an abstract
+/// derived class of \a raw_ostream as a proxy. Otherwise use \a
+/// raw_ostream_proxy.
+///
+/// Most operations are forwarded to the proxied stream.
+///
+/// If the proxied stream is buffered, the buffer is dropped and moved to this
+/// stream. This allows \a flush() to work correctly, flushing immediately from
+/// the proxy through to the final stream, and avoids any wasteful
+/// double-buffering.
+///
+/// \a enable_colors() changes both the proxied stream and the proxy itself.
+/// \a is_displayed() and \a has_colors() are forwarded to the proxy. \a
+/// 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 {
+ void write_impl(const char *Ptr, size_t Size) override {
+ getProxiedOS().write(Ptr, Size);
+ }
+ uint64_t current_pos() const override { return getProxiedOS().tell(); }
+ size_t preferred_buffer_size() const override {
+ return getPreferredBufferSize();
+ }
+
+public:
+ void reserveExtraSpace(uint64_t ExtraSize) override {
+ getProxiedOS().reserveExtraSpace(ExtraSize);
+ }
+ bool is_displayed() const override { return getProxiedOS().is_displayed(); }
+ bool has_colors() const override { return getProxiedOS().has_colors(); }
+ void enable_colors(bool enable) override {
+ RawOstreamT::enable_colors(enable);
+ getProxiedOS().enable_colors(enable);
+ }
+
+ ~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) {}
+
+ /// 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);
+ }
+ void resetProxiedOS(raw_ostream &) = delete;
+};
+
+/// Adaptor for creating a stream that proxies a \a raw_pwrite_stream.
+template <class RawPwriteStreamT = raw_pwrite_stream>
+class raw_pwrite_stream_proxy_adaptor
+ : public raw_ostream_proxy_adaptor<RawPwriteStreamT> {
+ using RawOstreamAdaptorT = raw_ostream_proxy_adaptor<RawPwriteStreamT>;
+
+ void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override {
+ this->flush();
+ getProxiedOS().pwrite(Ptr, Size, Offset);
+ }
+
+protected:
+ raw_pwrite_stream_proxy_adaptor() = default;
+ template <class... ArgsT>
+ explicit raw_pwrite_stream_proxy_adaptor(raw_pwrite_stream &OS,
+ ArgsT &&...Args)
+ : RawOstreamAdaptorT(OS, std::forward<ArgsT>(Args)...) {}
+
+ raw_pwrite_stream &getProxiedOS() const {
+ return static_cast<raw_pwrite_stream &>(RawOstreamAdaptorT::getProxiedOS());
+ }
+};
+
+/// Non-owning proxy for a \a raw_ostream. Enables passing a stream into of an
+/// API that takes ownership.
+class raw_ostream_proxy : public raw_ostream_proxy_adaptor<> {
+ void anchor() override;
+
+public:
+ raw_ostream_proxy(raw_ostream &OS) : raw_ostream_proxy_adaptor<>(OS) {}
+};
+
+/// Non-owning proxy for a \a raw_pwrite_stream. Enables passing a stream
+/// into of an API that takes ownership.
+class raw_pwrite_stream_proxy : public raw_pwrite_stream_proxy_adaptor<> {
+ void anchor() override;
+
+public:
+ raw_pwrite_stream_proxy(raw_pwrite_stream &OS)
+ : raw_pwrite_stream_proxy_adaptor<>(OS) {}
+};
+
+} // end namespace llvm
+
+#endif // LLVM_SUPPORT_RAW_OSTREAM_PROXY_H
diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
index 97188b0672f032..8c7a686892a3f0 100644
--- a/llvm/lib/Support/CMakeLists.txt
+++ b/llvm/lib/Support/CMakeLists.txt
@@ -261,6 +261,7 @@ add_llvm_component_library(LLVMSupport
YAMLTraits.cpp
raw_os_ostream.cpp
raw_ostream.cpp
+ raw_ostream_proxy.cpp
raw_socket_stream.cpp
regcomp.c
regerror.c
diff --git a/llvm/lib/Support/raw_ostream_proxy.cpp b/llvm/lib/Support/raw_ostream_proxy.cpp
new file mode 100644
index 00000000000000..2bbaa82f4afa7a
--- /dev/null
+++ b/llvm/lib/Support/raw_ostream_proxy.cpp
@@ -0,0 +1,15 @@
+//===- raw_ostream_proxy.cpp - Implement the raw_ostream proxies ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/raw_ostream_proxy.h"
+
+using namespace llvm;
+
+void raw_ostream_proxy::anchor() {}
+
+void raw_pwrite_stream_proxy::anchor() {}
diff --git a/llvm/unittests/Support/CMakeLists.txt b/llvm/unittests/Support/CMakeLists.txt
index d64f89847aa8e7..5e22b083cc18d2 100644
--- a/llvm/unittests/Support/CMakeLists.txt
+++ b/llvm/unittests/Support/CMakeLists.txt
@@ -104,6 +104,7 @@ add_llvm_unittest(SupportTests
formatted_raw_ostream_test.cpp
raw_fd_stream_test.cpp
raw_ostream_test.cpp
+ raw_ostream_proxy_test.cpp
raw_pwrite_stream_test.cpp
raw_sha1_ostream_test.cpp
raw_socket_stream_test.cpp
diff --git a/llvm/unittests/Support/raw_ostream_proxy_test.cpp b/llvm/unittests/Support/raw_ostream_proxy_test.cpp
new file mode 100644
index 00000000000000..ee97fe65b66003
--- /dev/null
+++ b/llvm/unittests/Support/raw_ostream_proxy_test.cpp
@@ -0,0 +1,219 @@
+//===- raw_ostream_proxy_test.cpp - Tests for raw ostream proxies ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/raw_ostream_proxy.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+/// Naive version of raw_svector_ostream that is buffered (by default) and
+/// doesn't support pwrite.
+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;
+
+ size_t preferred_buffer_size() const override { return PreferredBufferSize; }
+ uint64_t current_pos() const override { return Vector.size(); }
+ void write_impl(const char *Ptr, size_t Size) override {
+ Vector.append(Ptr, Ptr + Size);
+ }
+
+ bool is_displayed() const override { return IsDisplayed; }
+
+ explicit BufferedNoPwriteSmallVectorStream(SmallVectorImpl<char> &Vector)
+ : Vector(Vector) {}
+ ~BufferedNoPwriteSmallVectorStream() override { flush(); }
+
+ SmallVectorImpl<char> &Vector;
+ bool IsDisplayed = false;
+};
+
+constexpr const size_t BufferedNoPwriteSmallVectorStream::PreferredBufferSize;
+
+TEST(raw_ostream_proxyTest, write) {
+ // Besides confirming that "write" works, this test confirms that the proxy
+ // takes on the buffer from the stream it's proxying, such that writes to the
+ // proxy are flushed to the underlying stream as if no proxy were present.
+ SmallString<128> Dest;
+ {
+ // Confirm that BufferedNoPwriteSmallVectorStream is buffered by default,
+ // and that setting up a proxy effectively transfers a buffer of the same
+ // size to the proxy.
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ EXPECT_EQ(BufferedNoPwriteSmallVectorStream::PreferredBufferSize,
+ DestOS.GetBufferSize());
+ raw_ostream_proxy ProxyOS(DestOS);
+ EXPECT_EQ(0u, DestOS.GetBufferSize());
+ EXPECT_EQ(BufferedNoPwriteSmallVectorStream::PreferredBufferSize,
+ ProxyOS.GetBufferSize());
+
+ // Flushing should send through to Dest.
+ ProxyOS << "abcd";
+ EXPECT_EQ("", Dest);
+ ProxyOS.flush();
+ EXPECT_EQ("abcd", Dest);
+
+ // Buffer should still work.
+ ProxyOS << "e";
+ EXPECT_EQ("abcd", Dest);
+ }
+
+ // Destructing ProxyOS should flush (and not crash).
+ EXPECT_EQ("abcde", Dest);
+
+ {
+ // Set up another stream, this time unbuffered.
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ DestOS.SetUnbuffered();
+ EXPECT_EQ(0u, DestOS.GetBufferSize());
+ raw_ostream_proxy ProxyOS(DestOS);
+ EXPECT_EQ(0u, DestOS.GetBufferSize());
+ EXPECT_EQ(0u, ProxyOS.GetBufferSize());
+
+ // Flushing should not be required.
+ ProxyOS << "f";
+ EXPECT_EQ("abcdef", Dest);
+ }
+ EXPECT_EQ("abcdef", Dest);
+}
+
+TEST(raw_ostream_proxyTest, pwrite) {
+ // This test confirms that the proxy takes on the buffer from the stream it's
+ // proxying, such that writes to the proxy are flushed to the underlying
+ // stream as if no proxy were present.
+ SmallString<128> Dest;
+ raw_svector_ostream DestOS(Dest);
+ raw_pwrite_stream_proxy ProxyOS(DestOS);
+ EXPECT_EQ(0u, ProxyOS.GetBufferSize());
+
+ // Get some initial data.
+ ProxyOS << "abcd";
+ EXPECT_EQ("abcd", Dest);
+
+ // Confirm that pwrite works.
+ ProxyOS.pwrite("BC", 2, 1);
+ EXPECT_EQ("aBCd", Dest);
+}
+
+TEST(raw_ostream_proxyTest, pwriteWithBuffer) {
+ // This test confirms that when a buffer is configured, pwrite still works.
+ SmallString<128> Dest;
+ raw_svector_ostream DestOS(Dest);
+ DestOS.SetBufferSize(256);
+ EXPECT_EQ(256u, DestOS.GetBufferSize());
+
+ // Confirm that the proxy steals the buffer.
+ raw_pwrite_stream_proxy ProxyOS(DestOS);
+ EXPECT_EQ(0u, DestOS.GetBufferSize());
+ EXPECT_EQ(256u, ProxyOS.GetBufferSize());
+
+ // Check that the buffer is working.
+ ProxyOS << "abcd";
+ EXPECT_EQ("", Dest);
+
+ // Confirm that pwrite flushes.
+ ProxyOS.pwrite("BC", 2, 1);
+ EXPECT_EQ("aBCd", Dest);
+}
+
+class ProxyWithReset : public raw_ostream_proxy_adaptor<> {
+public:
+ ProxyWithReset(raw_ostream &OS) : raw_ostream_proxy_adaptor<>(OS) {}
+
+ // Allow this to be called outside the class.
+ using raw_ostream_proxy_adaptor<>::hasProxiedOS;
+ using raw_ostream_proxy_adaptor<>::getProxiedOS;
+ using raw_ostream_proxy_adaptor<>::resetProxiedOS;
+};
+
+TEST(raw_ostream_proxyTest, resetProxiedOS) {
+ // Confirm that base classes can drop the proxied OS before destruction and
+ // get consistent crashes.
+ SmallString<128> Dest;
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ ProxyWithReset ProxyOS(DestOS);
+ EXPECT_TRUE(ProxyOS.hasProxiedOS());
+ EXPECT_EQ(&DestOS, &ProxyOS.getProxiedOS());
+
+ // Write some data.
+ ProxyOS << "abcd";
+ EXPECT_EQ("", Dest);
+
+ // Reset the underlying stream.
+ ProxyOS.resetProxiedOS();
+ EXPECT_EQ("abcd", Dest);
+ EXPECT_EQ(0u, ProxyOS.GetBufferSize());
+ EXPECT_FALSE(ProxyOS.hasProxiedOS());
+
+#if GTEST_HAS_DEATH_TEST
+ EXPECT_DEATH(ProxyOS << "e", "use after reset");
+ EXPECT_DEATH(ProxyOS.getProxiedOS(), "use after reset");
+#endif
+}
+
+TEST(raw_ostream_proxyTest, ColorMode) {
+ {
+ SmallString<128> Dest;
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ raw_ostream_proxy ProxyOS(DestOS);
+ ProxyOS.enable_colors(true);
+
+ WithColor(ProxyOS, HighlightColor::Error, ColorMode::Disable) << "test";
+ EXPECT_EQ("", Dest);
+ ProxyOS.flush();
+ EXPECT_EQ("test", Dest);
+ }
+
+ {
+ SmallString<128> Dest;
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ raw_ostream_proxy ProxyOS(DestOS);
+ ProxyOS.enable_colors(true);
+
+ WithColor(ProxyOS, HighlightColor::Error, ColorMode::Auto) << "test";
+ EXPECT_EQ("", Dest);
+ ProxyOS.flush();
+ EXPECT_EQ("test", Dest);
+ }
+
+#ifdef LLVM_ON_UNIX
+ {
+ SmallString<128> Dest;
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ raw_ostream_proxy ProxyOS(DestOS);
+ ProxyOS.enable_colors(true);
+
+ WithColor(ProxyOS, HighlightColor::Error, ColorMode::Enable) << "test";
+ EXPECT_EQ("", Dest);
+ ProxyOS.flush();
+ EXPECT_EQ("\x1B[0;1;31mtest\x1B[0m", Dest);
+ }
+
+ {
+ SmallString<128> Dest;
+ BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ DestOS.IsDisplayed = true;
+ raw_ostream_proxy ProxyOS(DestOS);
+ ProxyOS.enable_colors(true);
+
+ WithColor(ProxyOS, HighlightColor::Error, ColorMode::Auto) << "test";
+ EXPECT_EQ("", Dest);
+ ProxyOS.flush();
+ EXPECT_EQ("\x1B[0;1;31mtest\x1B[0m", Dest);
+ }
+#endif
+}
+
+} // end namespace
>From dc9fb0207bb5afcc22d3c13788c830d4f79dc621 Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Tue, 22 Oct 2024 12:09:28 -0700
Subject: [PATCH 2/5] Address review feedback
Created using spr 1.3.5
---
llvm/include/llvm/Support/raw_ostream_proxy.h | 32 +++++++++++++------
.../Support/raw_ostream_proxy_test.cpp | 6 ++--
2 files changed, 26 insertions(+), 12 deletions(-)
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
>From fdaa3c1e428d4c0601b0c30b046a5bfecfd5a7b4 Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Tue, 22 Oct 2024 12:10:47 -0700
Subject: [PATCH 3/5] Remove raw_ostream_proxy_adaptor_base
Created using spr 1.3.5
---
llvm/include/llvm/Support/raw_ostream_proxy.h | 40 -------------------
1 file changed, 40 deletions(-)
diff --git a/llvm/include/llvm/Support/raw_ostream_proxy.h b/llvm/include/llvm/Support/raw_ostream_proxy.h
index d661ccbbbd616c..24d6b291657900 100644
--- a/llvm/include/llvm/Support/raw_ostream_proxy.h
+++ b/llvm/include/llvm/Support/raw_ostream_proxy.h
@@ -13,46 +13,6 @@
namespace llvm {
-/// Common bits for \a raw_ostream_proxy_adaptor<>, split out to dedup in
-/// template instantions.
-class raw_ostream_proxy_adaptor_base {
-protected:
- raw_ostream_proxy_adaptor_base(const raw_ostream_proxy_adaptor_base &) =
- delete;
-
- explicit raw_ostream_proxy_adaptor_base(raw_ostream &OS)
- : 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();
- }
-
- ~raw_ostream_proxy_adaptor_base() {
- assert(!OS && "Derived objects should call resetProxiedOS()");
- }
-
- /// Stop proxying the stream, taking the derived object by reference as \p
- /// ThisProxyOS. Updates \p ThisProxyOS to stop buffering before setting \a
- /// OS to \c nullptr, ensuring that future writes crash immediately.
- void resetProxiedOS(raw_ostream &ThisProxyOS) {
- ThisProxyOS.SetUnbuffered();
- OS = nullptr;
- }
-
- 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; }
-
-private:
- raw_ostream *OS;
-
- /// Caches the value of OS->GetBufferSize() at construction time.
- size_t PreferredBufferSize;
-};
-
/// Adaptor to create a stream class that proxies another \a raw_ostream.
///
/// Use \a raw_ostream_proxy_adaptor<> directly to implement an abstract
>From fbed8dc1e249232e606ecf04c9052be3c9ca105c Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Tue, 22 Oct 2024 13:13:23 -0700
Subject: [PATCH 4/5] Fix a bug during destruction when rewrite
Created using spr 1.3.5
---
llvm/include/llvm/Support/raw_ostream_proxy.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Support/raw_ostream_proxy.h b/llvm/include/llvm/Support/raw_ostream_proxy.h
index 24d6b291657900..5729cdef4b6ce6 100644
--- a/llvm/include/llvm/Support/raw_ostream_proxy.h
+++ b/llvm/include/llvm/Support/raw_ostream_proxy.h
@@ -74,7 +74,7 @@ class raw_ostream_proxy_adaptor : public RawOstreamT {
/// For example, this can simplify logic when a subclass might have a longer
/// lifetime than the stream it proxies.
void resetProxiedOS() {
- OS->SetUnbuffered();
+ this->SetUnbuffered();
OS = nullptr;
}
>From 41d0f375d36657abfa0ab085131b53adbe4fbcc5 Mon Sep 17 00:00:00 2001
From: Steven Wu <stevenwu at apple.com>
Date: Tue, 3 Dec 2024 14:00:01 -0800
Subject: [PATCH 5/5] Address review feedback
Created using spr 1.3.5
---
llvm/unittests/Support/raw_ostream_proxy_test.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/llvm/unittests/Support/raw_ostream_proxy_test.cpp b/llvm/unittests/Support/raw_ostream_proxy_test.cpp
index 1233ed54fd5a52..f3f0277bac23a6 100644
--- a/llvm/unittests/Support/raw_ostream_proxy_test.cpp
+++ b/llvm/unittests/Support/raw_ostream_proxy_test.cpp
@@ -167,10 +167,11 @@ TEST(raw_ostream_proxyTest, ColorMode) {
{
SmallString<128> Dest;
BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ DestOS.IsDisplayed = true;
raw_ostream_proxy ProxyOS(DestOS);
ProxyOS.enable_colors(true);
- WithColor(ProxyOS, HighlightColor::Error, ColorMode::Disable) << "test";
+ WithColor(ProxyOS, raw_ostream::Colors::RED, /*Bold=*/true, /*BG=*/false, ColorMode::Disable) << "test";
EXPECT_EQ("", Dest);
ProxyOS.flush();
EXPECT_EQ("test", Dest);
@@ -179,13 +180,14 @@ TEST(raw_ostream_proxyTest, ColorMode) {
{
SmallString<128> Dest;
BufferedNoPwriteSmallVectorStream DestOS(Dest);
+ DestOS.IsDisplayed = true;
raw_ostream_proxy ProxyOS(DestOS);
ProxyOS.enable_colors(true);
- WithColor(ProxyOS, HighlightColor::Error, ColorMode::Auto) << "test";
+ WithColor(ProxyOS, raw_ostream::Colors::RED, /*Bold=*/true, /*BG=*/false, ColorMode::Auto) << "test";
EXPECT_EQ("", Dest);
ProxyOS.flush();
- EXPECT_EQ("test", Dest);
+ EXPECT_EQ("\x1B[0;1;31mtest\x1B[0m", Dest);
}
#ifdef LLVM_ON_UNIX
@@ -195,7 +197,7 @@ TEST(raw_ostream_proxyTest, ColorMode) {
raw_ostream_proxy ProxyOS(DestOS);
ProxyOS.enable_colors(true);
- WithColor(ProxyOS, HighlightColor::Error, ColorMode::Enable) << "test";
+ WithColor(ProxyOS, raw_ostream::Colors::RED, /*Bold=*/true, /*BG=*/false, ColorMode::Enable) << "test";
EXPECT_EQ("", Dest);
ProxyOS.flush();
EXPECT_EQ("\x1B[0;1;31mtest\x1B[0m", Dest);
More information about the llvm-commits
mailing list