Skip to content

Commit dc9fb02

Browse files
Address review feedback
Created using spr 1.3.5
1 parent 664b08a commit dc9fb02

File tree

2 files changed

+26
-12
lines changed

2 files changed

+26
-12
lines changed

llvm/include/llvm/Support/raw_ostream_proxy.h

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ namespace llvm {
1717
/// template instantions.
1818
class raw_ostream_proxy_adaptor_base {
1919
protected:
20-
raw_ostream_proxy_adaptor_base() = delete;
2120
raw_ostream_proxy_adaptor_base(const raw_ostream_proxy_adaptor_base &) =
2221
delete;
2322

@@ -72,8 +71,7 @@ class raw_ostream_proxy_adaptor_base {
7271
/// changeColor(), resetColor(), and \a reverseColor() are not forwarded, since
7372
/// they need to call \a flush() and the buffer lives in the proxy.
7473
template <class RawOstreamT = raw_ostream>
75-
class raw_ostream_proxy_adaptor : public RawOstreamT,
76-
public raw_ostream_proxy_adaptor_base {
74+
class raw_ostream_proxy_adaptor : public RawOstreamT {
7775
void write_impl(const char *Ptr, size_t Size) override {
7876
getProxiedOS().write(Ptr, Size);
7977
}
@@ -92,23 +90,39 @@ class raw_ostream_proxy_adaptor : public RawOstreamT,
9290
RawOstreamT::enable_colors(enable);
9391
getProxiedOS().enable_colors(enable);
9492
}
93+
bool hasProxiedOS() const { return OS; }
94+
raw_ostream &getProxiedOS() const {
95+
assert(OS && "raw_ostream_proxy_adaptor use after reset");
96+
return *OS;
97+
}
98+
size_t getPreferredBufferSize() const { return PreferredBufferSize; }
9599

96100
~raw_ostream_proxy_adaptor() override { resetProxiedOS(); }
97101

98102
protected:
99103
template <class... ArgsT>
100104
explicit raw_ostream_proxy_adaptor(raw_ostream &OS, ArgsT &&...Args)
101-
: RawOstreamT(std::forward<ArgsT>(Args)...),
102-
raw_ostream_proxy_adaptor_base(OS) {}
105+
: RawOstreamT(std::forward<ArgsT>(Args)...), OS(&OS),
106+
PreferredBufferSize(OS.GetBufferSize()) {
107+
// Drop OS's buffer to make this->flush() forward. This proxy will add a
108+
// buffer in its place.
109+
OS.SetUnbuffered();
110+
}
103111

104112
/// Stop proxying the stream. Flush and set up a crash for future writes.
105113
///
106114
/// For example, this can simplify logic when a subclass might have a longer
107115
/// lifetime than the stream it proxies.
108116
void resetProxiedOS() {
109-
raw_ostream_proxy_adaptor_base::resetProxiedOS(*this);
117+
OS->SetUnbuffered();
118+
OS = nullptr;
110119
}
111-
void resetProxiedOS(raw_ostream &) = delete;
120+
121+
private:
122+
raw_ostream *OS;
123+
124+
/// Caches the value of OS->GetBufferSize() at construction time.
125+
size_t PreferredBufferSize;
112126
};
113127

114128
/// Adaptor for creating a stream that proxies a \a raw_pwrite_stream.
@@ -134,7 +148,7 @@ class raw_pwrite_stream_proxy_adaptor
134148
}
135149
};
136150

137-
/// Non-owning proxy for a \a raw_ostream. Enables passing a stream into of an
151+
/// Non-owning proxy for a \a raw_ostream. Enables passing a stream into an
138152
/// API that takes ownership.
139153
class raw_ostream_proxy : public raw_ostream_proxy_adaptor<> {
140154
void anchor() override;
@@ -144,7 +158,7 @@ class raw_ostream_proxy : public raw_ostream_proxy_adaptor<> {
144158
};
145159

146160
/// Non-owning proxy for a \a raw_pwrite_stream. Enables passing a stream
147-
/// into of an API that takes ownership.
161+
/// into an API that takes ownership.
148162
class raw_pwrite_stream_proxy : public raw_pwrite_stream_proxy_adaptor<> {
149163
void anchor() override;
150164

llvm/unittests/Support/raw_ostream_proxy_test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class BufferedNoPwriteSmallVectorStream : public raw_ostream {
2222
public:
2323
// Choose a strange buffer size to ensure it doesn't collide with the default
2424
// on \a raw_ostream.
25-
constexpr static const size_t PreferredBufferSize = 63;
25+
static constexpr size_t PreferredBufferSize = 63;
2626

2727
size_t preferred_buffer_size() const override { return PreferredBufferSize; }
2828
uint64_t current_pos() const override { return Vector.size(); }
@@ -40,7 +40,7 @@ class BufferedNoPwriteSmallVectorStream : public raw_ostream {
4040
bool IsDisplayed = false;
4141
};
4242

43-
constexpr const size_t BufferedNoPwriteSmallVectorStream::PreferredBufferSize;
43+
constexpr size_t BufferedNoPwriteSmallVectorStream::PreferredBufferSize;
4444

4545
TEST(raw_ostream_proxyTest, write) {
4646
// Besides confirming that "write" works, this test confirms that the proxy
@@ -157,7 +157,7 @@ TEST(raw_ostream_proxyTest, resetProxiedOS) {
157157
EXPECT_EQ(0u, ProxyOS.GetBufferSize());
158158
EXPECT_FALSE(ProxyOS.hasProxiedOS());
159159

160-
#if GTEST_HAS_DEATH_TEST
160+
#if GTEST_HAS_DEATH_TEST && !defined(NDEBUG)
161161
EXPECT_DEATH(ProxyOS << "e", "use after reset");
162162
EXPECT_DEATH(ProxyOS.getProxiedOS(), "use after reset");
163163
#endif

0 commit comments

Comments
 (0)