Skip to content

Commit f6522d8

Browse files
committed
[libc] Refactor static polymorphism in WriteBuffer (NFC).
There are three flavors of WriteBuffer currently, all of which could be passed into printf_core::Writer class. It's a tricky class, since it chooses a flavor-specific logic either based on runtime dispatch (to save code size and prevent generating three versions of the entirety of printf_core), or based on template arguments (to avoid dealing with function pointers in codegen for FILL_BUFF_AND_DROP_OVERFLOW path. Refactor this somewhat convoluted logic to have three concrete subclasses inheriting from the templated base class, and use static polymorphism with reinterpret_cast to implement dispatching above. Now we can actually have flavor-specific fields, constructors, and methods (e.g. "flush_to_stream" is now a method of FlushingBuffer), and the code on the user side is cleaner: the complexity of enabling/disabling runtime-dispatch and using proper template arguments is now localized in writer.h. This code will need to be further templatized to support buffers of type wchar_t to implement swprintf() and friends. This change would make it (ever so slightly) easier.
1 parent 76d614b commit f6522d8

File tree

17 files changed

+162
-154
lines changed

17 files changed

+162
-154
lines changed

libc/src/__support/RPC/rpc_server.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,7 @@ LIBC_INLINE static void handle_printf(rpc::Server::Port &port,
169169
if (!format[lane])
170170
continue;
171171

172-
printf_core::WriteBuffer<
173-
printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>
174-
wb(nullptr, 0);
172+
printf_core::DropOverflowBuffer wb(nullptr, 0);
175173
printf_core::Writer writer(wb);
176174

177175
internal::DummyArgList<packed> printf_args;
@@ -198,9 +196,7 @@ LIBC_INLINE static void handle_printf(rpc::Server::Port &port,
198196
if (!format[lane])
199197
continue;
200198

201-
printf_core::WriteBuffer<
202-
printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>
203-
wb(nullptr, 0);
199+
printf_core::DropOverflowBuffer wb(nullptr, 0);
204200
printf_core::Writer writer(wb);
205201

206202
internal::StructArgList<packed> printf_args(args[lane], args_sizes[lane]);
@@ -262,9 +258,7 @@ LIBC_INLINE static void handle_printf(rpc::Server::Port &port,
262258
continue;
263259

264260
char *buffer = temp_storage.alloc(buffer_size[lane]);
265-
printf_core::WriteBuffer<
266-
printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>
267-
wb(buffer, buffer_size[lane]);
261+
printf_core::DropOverflowBuffer wb(buffer, buffer_size[lane]);
268262
printf_core::Writer writer(wb);
269263

270264
internal::StructArgList<packed> printf_args(args[lane], args_sizes[lane]);

libc/src/stdio/baremetal/printf.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,17 @@ LLVM_LIBC_FUNCTION(int, printf, (const char *__restrict format, ...)) {
4141
static constexpr size_t BUFF_SIZE = 1024;
4242
char buffer[BUFF_SIZE];
4343

44-
printf_core::WriteBuffer<printf_core::WriteMode::FLUSH_TO_STREAM> wb(
45-
buffer, BUFF_SIZE, &stdout_write_hook, nullptr);
46-
printf_core::Writer<printf_core::WriteMode::FLUSH_TO_STREAM> writer(wb);
44+
printf_core::FlushingBuffer wb(buffer, BUFF_SIZE, &stdout_write_hook,
45+
nullptr);
46+
printf_core::Writer writer(wb);
4747

4848
auto retval = printf_core::printf_main(&writer, format, args);
4949
if (!retval.has_value()) {
5050
libc_errno = printf_core::internal_error_to_errno(retval.error());
5151
return -1;
5252
}
5353

54-
int flushval = wb.overflow_write("");
54+
int flushval = wb.flush_to_stream();
5555
if (flushval != printf_core::WRITE_OK) {
5656
libc_errno = printf_core::internal_error_to_errno(-flushval);
5757
return -1;

libc/src/stdio/baremetal/vprintf.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,17 @@ LLVM_LIBC_FUNCTION(int, vprintf,
3939
static constexpr size_t BUFF_SIZE = 1024;
4040
char buffer[BUFF_SIZE];
4141

42-
printf_core::WriteBuffer<printf_core::WriteMode::FLUSH_TO_STREAM> wb(
43-
buffer, BUFF_SIZE, &stdout_write_hook, nullptr);
44-
printf_core::Writer<printf_core::WriteMode::FLUSH_TO_STREAM> writer(wb);
42+
printf_core::FlushingBuffer wb(buffer, BUFF_SIZE, &stdout_write_hook,
43+
nullptr);
44+
printf_core::Writer writer(wb);
4545

4646
auto retval = printf_core::printf_main(&writer, format, args);
4747
if (!retval.has_value()) {
4848
libc_errno = printf_core::internal_error_to_errno(retval.error());
4949
return -1;
5050
}
5151

52-
int flushval = wb.overflow_write("");
52+
int flushval = wb.flush_to_stream();
5353
if (flushval != printf_core::WRITE_OK) {
5454
libc_errno = printf_core::internal_error_to_errno(-flushval);
5555
return -1;

libc/src/stdio/printf_core/vasprintf_internal.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,8 @@
1818
namespace LIBC_NAMESPACE_DECL {
1919
namespace printf_core {
2020

21-
LIBC_INLINE int resize_overflow_hook(cpp::string_view new_str, void *target) {
22-
WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value> *wb =
23-
reinterpret_cast<
24-
WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value> *>(target);
21+
LIBC_INLINE int resize_overflow_hook(cpp::string_view new_str,
22+
ResizingBuffer *wb) {
2523
size_t new_size = new_str.size() + wb->buff_cur;
2624
const bool isBuffOnStack = (wb->buff == wb->init_buff);
2725
char *new_buff = static_cast<char *>(
@@ -47,8 +45,8 @@ LIBC_INLINE ErrorOr<size_t> vasprintf_internal(char **ret,
4745
const char *__restrict format,
4846
internal::ArgList args) {
4947
char init_buff_on_stack[DEFAULT_BUFFER_SIZE];
50-
printf_core::WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value> wb(
51-
init_buff_on_stack, DEFAULT_BUFFER_SIZE, resize_overflow_hook);
48+
printf_core::ResizingBuffer wb(init_buff_on_stack, DEFAULT_BUFFER_SIZE,
49+
resize_overflow_hook);
5250
printf_core::Writer writer(wb);
5351

5452
auto ret_val = printf_core::printf_main(&writer, format, args);

libc/src/stdio/printf_core/vfprintf_internal.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,16 @@ LIBC_INLINE ErrorOr<size_t> vfprintf_internal(::FILE *__restrict stream,
8686
internal::ArgList &args) {
8787
constexpr size_t BUFF_SIZE = 1024;
8888
char buffer[BUFF_SIZE];
89-
printf_core::WriteBuffer<Mode<WriteMode::FLUSH_TO_STREAM>::value> wb(
90-
buffer, BUFF_SIZE, &file_write_hook, reinterpret_cast<void *>(stream));
89+
printf_core::FlushingBuffer wb(buffer, BUFF_SIZE, &file_write_hook,
90+
reinterpret_cast<void *>(stream));
9191
Writer writer(wb);
9292
internal::flockfile(stream);
9393
auto retval = printf_main(&writer, format, args);
9494
if (!retval.has_value()) {
9595
internal::funlockfile(stream);
9696
return retval;
9797
}
98-
int flushval = wb.overflow_write("");
98+
int flushval = wb.flush_to_stream();
9999
if (flushval != WRITE_OK)
100100
retval = Error(-flushval);
101101
internal::funlockfile(stream);

libc/src/stdio/printf_core/writer.h

Lines changed: 98 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -38,36 +38,65 @@ template <WriteMode write_mode> struct Mode {
3838
#endif
3939
};
4040

41+
template <WriteMode write_mode> class Writer;
42+
4143
template <WriteMode write_mode> struct WriteBuffer {
42-
using StreamWriter = int (*)(cpp::string_view, void *);
4344
char *buff;
44-
const char *init_buff; // for checking when resize.
4545
size_t buff_len;
4646
size_t buff_cur = 0;
47-
48-
// The stream writer will be called when the buffer is full. It will be passed
49-
// string_views to write to the stream.
50-
const StreamWriter stream_writer;
51-
void *output_target;
52-
5347
// The current writing mode in case the user wants runtime dispatch of the
5448
// stream writer with function pointers.
5549
[[maybe_unused]] WriteMode write_mode_;
5650

57-
LIBC_INLINE WriteBuffer(char *buff, size_t buff_len, StreamWriter hook,
58-
void *target)
59-
: buff(buff), init_buff(buff), buff_len(buff_len), stream_writer(hook),
60-
output_target(target), write_mode_(WriteMode::FLUSH_TO_STREAM) {}
51+
protected:
52+
LIBC_INLINE WriteBuffer(char *buff, size_t buff_len, WriteMode mode)
53+
: buff(buff), buff_len(buff_len), write_mode_(mode) {}
54+
55+
private:
56+
friend class Writer<write_mode>;
57+
// The overflow_write method will handle the case when adding new_str to
58+
// the buffer would overflow it. Specific actions will depend on the buffer
59+
// type / write_mode.
60+
LIBC_INLINE int overflow_write(cpp::string_view new_str);
61+
};
62+
63+
// Buffer variant that discards characters that don't fit into the buffer.
64+
struct DropOverflowBuffer
65+
: public WriteBuffer<Mode<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value> {
66+
LIBC_INLINE DropOverflowBuffer(char *buff, size_t buff_len)
67+
: WriteBuffer<Mode<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>(
68+
buff, buff_len, WriteMode::FILL_BUFF_AND_DROP_OVERFLOW) {}
69+
70+
LIBC_INLINE int fill_remaining_to_buff(cpp::string_view new_str) {
71+
if (buff_cur < buff_len) {
72+
size_t bytes_to_write = buff_len - buff_cur;
73+
if (bytes_to_write > new_str.size()) {
74+
bytes_to_write = new_str.size();
75+
}
76+
inline_memcpy(buff + buff_cur, new_str.data(), bytes_to_write);
77+
buff_cur += bytes_to_write;
78+
}
79+
return WRITE_OK;
80+
}
81+
};
6182

62-
LIBC_INLINE WriteBuffer(char *buff, size_t buff_len)
63-
: buff(buff), init_buff(buff), buff_len(buff_len), stream_writer(nullptr),
64-
output_target(nullptr),
65-
write_mode_(WriteMode::FILL_BUFF_AND_DROP_OVERFLOW) {}
83+
// Buffer variant that flushes to stream when it gets full.
84+
struct FlushingBuffer
85+
: public WriteBuffer<Mode<WriteMode::FLUSH_TO_STREAM>::value> {
86+
// The stream writer will be called when the buffer is full. It will be passed
87+
// string_views to write to the stream.
88+
using StreamWriter = int (*)(cpp::string_view, void *);
89+
const StreamWriter stream_writer;
90+
void *output_target;
6691

67-
LIBC_INLINE WriteBuffer(char *buff, size_t buff_len, StreamWriter hook)
68-
: buff(buff), init_buff(buff), buff_len(buff_len), stream_writer(hook),
69-
output_target(this), write_mode_(WriteMode::RESIZE_AND_FILL_BUFF) {}
92+
LIBC_INLINE FlushingBuffer(char *buff, size_t buff_len, StreamWriter hook,
93+
void *target)
94+
: WriteBuffer<Mode<WriteMode::FLUSH_TO_STREAM>::value>(
95+
buff, buff_len, WriteMode::FLUSH_TO_STREAM),
96+
stream_writer(hook), output_target(target) {}
7097

98+
// Flushes the entire current buffer to stream, followed by the new_str (if
99+
// non-empty).
71100
LIBC_INLINE int flush_to_stream(cpp::string_view new_str) {
72101
if (buff_cur > 0) {
73102
int retval = stream_writer({buff, buff_cur}, output_target);
@@ -83,48 +112,61 @@ template <WriteMode write_mode> struct WriteBuffer {
83112
return WRITE_OK;
84113
}
85114

86-
LIBC_INLINE int fill_remaining_to_buff(cpp::string_view new_str) {
87-
if (buff_cur < buff_len) {
88-
size_t bytes_to_write = buff_len - buff_cur;
89-
if (bytes_to_write > new_str.size()) {
90-
bytes_to_write = new_str.size();
91-
}
92-
inline_memcpy(buff + buff_cur, new_str.data(), bytes_to_write);
93-
buff_cur += bytes_to_write;
94-
}
95-
return WRITE_OK;
96-
}
115+
LIBC_INLINE int flush_to_stream() { return flush_to_stream({}); }
116+
};
97117

98-
LIBC_INLINE int resize_and_write(cpp::string_view new_str) {
99-
return stream_writer(new_str, output_target);
100-
}
118+
// Buffer variant that calls a resizing callback when it gets full.
119+
struct ResizingBuffer
120+
: public WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value> {
121+
using ResizeWriter = int (*)(cpp::string_view, ResizingBuffer *);
122+
const ResizeWriter resize_writer;
123+
const char *init_buff; // for checking when resize.
101124

102-
// The overflow_write method is intended to be called to write the contents of
103-
// the buffer and new_str to the stream_writer if it exists. If a resizing
104-
// hook is provided, it will resize the buffer and write the contents. If
105-
// neither a stream_writer nor a resizing hook is provided, it will fill the
106-
// remaining space in the buffer with new_str and drop the overflow. Calling
107-
// this with an empty string will flush the buffer if relevant.
108-
109-
LIBC_INLINE int overflow_write(cpp::string_view new_str) {
110-
if constexpr (write_mode == WriteMode::RUNTIME_DISPATCH) {
111-
if (write_mode_ == WriteMode::FILL_BUFF_AND_DROP_OVERFLOW)
112-
return fill_remaining_to_buff(new_str);
113-
else if (write_mode_ == WriteMode::FLUSH_TO_STREAM)
114-
return flush_to_stream(new_str);
115-
else if (write_mode_ == WriteMode::RESIZE_AND_FILL_BUFF)
116-
return resize_and_write(new_str);
117-
} else if constexpr (write_mode == WriteMode::FILL_BUFF_AND_DROP_OVERFLOW) {
118-
return fill_remaining_to_buff(new_str);
119-
} else if constexpr (write_mode == WriteMode::FLUSH_TO_STREAM) {
120-
return flush_to_stream(new_str);
121-
} else if constexpr (write_mode == WriteMode::RESIZE_AND_FILL_BUFF) {
122-
return resize_and_write(new_str);
123-
}
124-
__builtin_unreachable();
125+
LIBC_INLINE ResizingBuffer(char *buff, size_t buff_len, ResizeWriter hook)
126+
: WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value>(
127+
buff, buff_len, WriteMode::RESIZE_AND_FILL_BUFF),
128+
resize_writer(hook), init_buff(buff) {}
129+
130+
// Invokes the callback that is supposed to resize the buffer and make
131+
// it large enough to fit the new_str addition.
132+
LIBC_INLINE int resize_and_write(cpp::string_view new_str) {
133+
return resize_writer(new_str, this);
125134
}
126135
};
127136

137+
template <>
138+
LIBC_INLINE int WriteBuffer<WriteMode::RUNTIME_DISPATCH>::overflow_write(
139+
cpp::string_view new_str) {
140+
if (write_mode_ == WriteMode::FILL_BUFF_AND_DROP_OVERFLOW)
141+
return reinterpret_cast<DropOverflowBuffer *>(this)->fill_remaining_to_buff(
142+
new_str);
143+
else if (write_mode_ == WriteMode::FLUSH_TO_STREAM)
144+
return reinterpret_cast<FlushingBuffer *>(this)->flush_to_stream(new_str);
145+
else if (write_mode_ == WriteMode::RESIZE_AND_FILL_BUFF)
146+
return reinterpret_cast<ResizingBuffer *>(this)->resize_and_write(new_str);
147+
__builtin_unreachable();
148+
}
149+
150+
template <>
151+
LIBC_INLINE int
152+
WriteBuffer<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::overflow_write(
153+
cpp::string_view new_str) {
154+
return reinterpret_cast<DropOverflowBuffer *>(this)->fill_remaining_to_buff(
155+
new_str);
156+
}
157+
158+
template <>
159+
LIBC_INLINE int WriteBuffer<WriteMode::FLUSH_TO_STREAM>::overflow_write(
160+
cpp::string_view new_str) {
161+
return reinterpret_cast<FlushingBuffer *>(this)->flush_to_stream(new_str);
162+
}
163+
164+
template <>
165+
LIBC_INLINE int WriteBuffer<WriteMode::RESIZE_AND_FILL_BUFF>::overflow_write(
166+
cpp::string_view new_str) {
167+
return reinterpret_cast<ResizingBuffer *>(this)->resize_and_write(new_str);
168+
}
169+
128170
template <WriteMode write_mode> class Writer final {
129171
WriteBuffer<write_mode> &wb;
130172
size_t chars_written = 0;

libc/src/stdio/snprintf.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ LLVM_LIBC_FUNCTION(int, snprintf,
3131
// and pointer semantics, as well as handling
3232
// destruction automatically.
3333
va_end(vlist);
34-
printf_core::WriteBuffer<printf_core::Mode<
35-
printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>
36-
wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
34+
printf_core::DropOverflowBuffer wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
3735
printf_core::Writer writer(wb);
3836

3937
auto ret_val = printf_core::printf_main(&writer, format, args);

libc/src/stdio/sprintf.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ LLVM_LIBC_FUNCTION(int, sprintf,
3131
// destruction automatically.
3232
va_end(vlist);
3333

34-
printf_core::WriteBuffer<
35-
printf_core::Mode<printf_core::WriteMode::RESIZE_AND_FILL_BUFF>::value>
36-
wb(buffer, cpp::numeric_limits<size_t>::max());
34+
printf_core::DropOverflowBuffer wb(buffer,
35+
cpp::numeric_limits<size_t>::max());
3736
printf_core::Writer writer(wb);
3837

3938
auto ret_val = printf_core::printf_main(&writer, format, args);

libc/src/stdio/vsnprintf.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ LLVM_LIBC_FUNCTION(int, vsnprintf,
2828
internal::ArgList args(vlist); // This holder class allows for easier copying
2929
// and pointer semantics, as well as handling
3030
// destruction automatically.
31-
printf_core::WriteBuffer<printf_core::Mode<
32-
printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>
33-
wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
31+
printf_core::DropOverflowBuffer wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
3432
printf_core::Writer writer(wb);
3533

3634
auto ret_val = printf_core::printf_main(&writer, format, args);

libc/src/stdio/vsprintf.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,8 @@ LLVM_LIBC_FUNCTION(int, vsprintf,
2828
// and pointer semantics, as well as handling
2929
// destruction automatically.
3030

31-
printf_core::WriteBuffer<printf_core::Mode<
32-
printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>
33-
wb(buffer, cpp::numeric_limits<size_t>::max());
31+
printf_core::DropOverflowBuffer wb(buffer,
32+
cpp::numeric_limits<size_t>::max());
3433
printf_core::Writer writer(wb);
3534

3635
auto ret_val = printf_core::printf_main(&writer, format, args);

0 commit comments

Comments
 (0)