llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-libc Author: Alexey Samsonov (vonosmas) <details> <summary>Changes</summary> This change doesn't yet modify Writer::write(string_view) public API, which has a lot of uses through printf_core. For wide-char write functions (like swprintf) we'd need to templatize WriteBuffer and Writer to support wide characrters. We'd need to either support helper cpp::string_view class to support wide-character strings, or replace them with pure pointer/size pairs. The latter option seems to be a more straightforward one, given that majority of printf_core operates on buffers anyway (and only constructs string_views right before invoking the buffer), and we don't have a use case internal wide-character string-views. --- Full diff: https://github.com/llvm/llvm-project/pull/170959.diff 6 Files Affected: - (modified) libc/src/stdio/baremetal/printf.cpp (+3-2) - (modified) libc/src/stdio/baremetal/vprintf.cpp (+3-2) - (modified) libc/src/stdio/printf_core/vasprintf_internal.h (+3-3) - (modified) libc/src/stdio/printf_core/vfprintf_internal.h (+5-4) - (modified) libc/src/stdio/printf_core/writer.h (+56-49) - (modified) libc/test/src/stdio/printf_core/writer_test.cpp (+5-7) ``````````diff diff --git a/libc/src/stdio/baremetal/printf.cpp b/libc/src/stdio/baremetal/printf.cpp index 2fa9cf7c9f3cd..3329d4fecdad5 100644 --- a/libc/src/stdio/baremetal/printf.cpp +++ b/libc/src/stdio/baremetal/printf.cpp @@ -24,8 +24,9 @@ namespace LIBC_NAMESPACE_DECL { namespace { -LIBC_INLINE int stdout_write_hook(cpp::string_view new_str, void *) { - write_to_stdout(new_str); +LIBC_INLINE int stdout_write_hook(const char *new_str, size_t new_str_len, + void *) { + write_to_stdout({new_str, new_str_len}); return printf_core::WRITE_OK; } diff --git a/libc/src/stdio/baremetal/vprintf.cpp b/libc/src/stdio/baremetal/vprintf.cpp index d89f26cd72b0a..30faa9ad54b8b 100644 --- a/libc/src/stdio/baremetal/vprintf.cpp +++ b/libc/src/stdio/baremetal/vprintf.cpp @@ -24,8 +24,9 @@ namespace LIBC_NAMESPACE_DECL { namespace { -LIBC_INLINE int stdout_write_hook(cpp::string_view new_str, void *) { - write_to_stdout(new_str); +LIBC_INLINE int stdout_write_hook(const char *new_str, size_t new_str_len, + void *) { + write_to_stdout({new_str, new_str_len}); return printf_core::WRITE_OK; } diff --git a/libc/src/stdio/printf_core/vasprintf_internal.h b/libc/src/stdio/printf_core/vasprintf_internal.h index db6b95d49aaca..a36e9ca749305 100644 --- a/libc/src/stdio/printf_core/vasprintf_internal.h +++ b/libc/src/stdio/printf_core/vasprintf_internal.h @@ -18,9 +18,9 @@ namespace LIBC_NAMESPACE_DECL { namespace printf_core { -LIBC_INLINE int resize_overflow_hook(cpp::string_view new_str, +LIBC_INLINE int resize_overflow_hook(const char *new_str, size_t new_str_len, ResizingBuffer *wb) { - size_t new_size = new_str.size() + wb->buff_cur; + size_t new_size = new_str_len + wb->buff_cur; const bool isBuffOnStack = (wb->buff == wb->init_buff); char *new_buff = static_cast<char *>( isBuffOnStack ? malloc(new_size + 1) @@ -33,7 +33,7 @@ LIBC_INLINE int resize_overflow_hook(cpp::string_view new_str, if (isBuffOnStack) inline_memcpy(new_buff, wb->buff, wb->buff_cur); wb->buff = new_buff; - inline_memcpy(wb->buff + wb->buff_cur, new_str.data(), new_str.size()); + inline_memcpy(wb->buff + wb->buff_cur, new_str, new_str_len); wb->buff_cur = new_size; wb->buff_len = new_size; return printf_core::WRITE_OK; diff --git a/libc/src/stdio/printf_core/vfprintf_internal.h b/libc/src/stdio/printf_core/vfprintf_internal.h index 321b0693ad339..b2b282fbf7b78 100644 --- a/libc/src/stdio/printf_core/vfprintf_internal.h +++ b/libc/src/stdio/printf_core/vfprintf_internal.h @@ -62,19 +62,20 @@ LIBC_INLINE FileIOResult fwrite_unlocked(const void *ptr, size_t size, namespace printf_core { -LIBC_INLINE int file_write_hook(cpp::string_view new_str, void *fp) { +LIBC_INLINE int file_write_hook(const char *new_str, size_t new_str_len, + void *fp) { ::FILE *target_file = reinterpret_cast<::FILE *>(fp); // Write new_str to the target file. The logic preventing a zero-length write // is in the writer, so we don't check here. - auto write_result = internal::fwrite_unlocked(new_str.data(), sizeof(char), - new_str.size(), target_file); + auto write_result = internal::fwrite_unlocked(new_str, sizeof(char), + new_str_len, target_file); // Propagate actual system error in FileIOResult. if (write_result.has_error()) return -write_result.error; // In case short write occured or error was not set on FileIOResult for some // reason. - if (write_result.value != new_str.size() || + if (write_result.value != new_str_len || internal::ferror_unlocked(target_file)) return FILE_WRITE_ERROR; diff --git a/libc/src/stdio/printf_core/writer.h b/libc/src/stdio/printf_core/writer.h index cb45b105597d1..1eeffee2f5dde 100644 --- a/libc/src/stdio/printf_core/writer.h +++ b/libc/src/stdio/printf_core/writer.h @@ -41,7 +41,8 @@ template <WriteMode write_mode> struct Mode { template <WriteMode write_mode> class Writer; template <WriteMode write_mode> struct WriteBuffer { - char *buff; + using CharType = char; + CharType *buff; size_t buff_len; size_t buff_cur = 0; // The current writing mode in case the user wants runtime dispatch of the @@ -49,7 +50,7 @@ template <WriteMode write_mode> struct WriteBuffer { [[maybe_unused]] WriteMode write_mode_; protected: - LIBC_INLINE WriteBuffer(char *buff, size_t buff_len, WriteMode mode) + LIBC_INLINE WriteBuffer(CharType *buff, size_t buff_len, WriteMode mode) : buff(buff), buff_len(buff_len), write_mode_(mode) {} private: @@ -57,24 +58,26 @@ template <WriteMode write_mode> struct WriteBuffer { // The overflow_write method will handle the case when adding new_str to // the buffer would overflow it. Specific actions will depend on the buffer // type / write_mode. - LIBC_INLINE int overflow_write(cpp::string_view new_str); + LIBC_INLINE int overflow_write(const CharType *new_str, size_t new_str_len); }; // Buffer variant that discards characters that don't fit into the buffer. struct DropOverflowBuffer : public WriteBuffer<Mode<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value> { - LIBC_INLINE DropOverflowBuffer(char *buff, size_t buff_len) + LIBC_INLINE DropOverflowBuffer(CharType *buff, size_t buff_len) : WriteBuffer<Mode<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>( buff, buff_len, WriteMode::FILL_BUFF_AND_DROP_OVERFLOW) {} - LIBC_INLINE int fill_remaining_to_buff(cpp::string_view new_str) { + LIBC_INLINE int fill_remaining_to_buff(const CharType *new_str, + size_t new_str_len) { if (buff_cur < buff_len) { - size_t bytes_to_write = buff_len - buff_cur; - if (bytes_to_write > new_str.size()) { - bytes_to_write = new_str.size(); + size_t chars_to_write = buff_len - buff_cur; + if (chars_to_write > new_str_len) { + chars_to_write = new_str_len; } - inline_memcpy(buff + buff_cur, new_str.data(), bytes_to_write); - buff_cur += bytes_to_write; + inline_memcpy(buff + buff_cur, new_str, + chars_to_write * sizeof(CharType)); + buff_cur += chars_to_write; } return WRITE_OK; } @@ -84,27 +87,27 @@ struct DropOverflowBuffer struct FlushingBuffer : public WriteBuffer<Mode<WriteMode::FLUSH_TO_STREAM>::value> { // The stream writer will be called when the buffer is full. It will be passed - // string_views to write to the stream. - using StreamWriter = int (*)(cpp::string_view, void *); + // buffers to write, and the target provided at construction time. + using StreamWriter = int (*)(const CharType *, size_t, void *); const StreamWriter stream_writer; void *output_target; - LIBC_INLINE FlushingBuffer(char *buff, size_t buff_len, StreamWriter hook, + LIBC_INLINE FlushingBuffer(CharType *buff, size_t buff_len, StreamWriter hook, void *target) : WriteBuffer<Mode<WriteMode::FLUSH_TO_STREAM>::value>( buff, buff_len, WriteMode::FLUSH_TO_STREAM), stream_writer(hook), output_target(target) {} - // Flushes the entire current buffer to stream, followed by the new_str (if - // non-empty). - LIBC_INLINE int flush_to_stream(cpp::string_view new_str) { + // Flushes the entire current buffer to stream, followed by the new_str + // buffer. + LIBC_INLINE int flush_to_stream(const CharType *new_str, size_t new_str_len) { if (buff_cur > 0) { - int retval = stream_writer({buff, buff_cur}, output_target); + int retval = stream_writer(buff, buff_cur, output_target); if (retval < 0) return retval; } - if (new_str.size() > 0) { - int retval = stream_writer(new_str, output_target); + if (new_str_len > 0) { + int retval = stream_writer(new_str, new_str_len, output_target); if (retval < 0) return retval; } @@ -112,66 +115,73 @@ struct FlushingBuffer return WRITE_OK; } - LIBC_INLINE int flush_to_stream() { return flush_to_stream({}); } + LIBC_INLINE int flush_to_stream() { return flush_to_stream(nullptr, 0); } }; // Buffer variant that calls a resizing callback when it gets full. struct ResizingBuffer : public WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value> { - using ResizeWriter = int (*)(cpp::string_view, ResizingBuffer *); + using ResizeWriter = int (*)(const CharType *, size_t, ResizingBuffer *); const ResizeWriter resize_writer; - const char *init_buff; // for checking when resize. + const CharType *init_buff; // for checking when resize. - LIBC_INLINE ResizingBuffer(char *buff, size_t buff_len, ResizeWriter hook) + LIBC_INLINE ResizingBuffer(CharType *buff, size_t buff_len, ResizeWriter hook) : WriteBuffer<Mode<WriteMode::RESIZE_AND_FILL_BUFF>::value>( buff, buff_len, WriteMode::RESIZE_AND_FILL_BUFF), resize_writer(hook), init_buff(buff) {} // Invokes the callback that is supposed to resize the buffer and make // it large enough to fit the new_str addition. - LIBC_INLINE int resize_and_write(cpp::string_view new_str) { - return resize_writer(new_str, this); + LIBC_INLINE int resize_and_write(const CharType *new_str, + size_t new_str_len) { + return resize_writer(new_str, new_str_len, this); } }; template <> LIBC_INLINE int WriteBuffer<WriteMode::RUNTIME_DISPATCH>::overflow_write( - cpp::string_view new_str) { + const CharType *new_str, size_t new_str_len) { if (write_mode_ == WriteMode::FILL_BUFF_AND_DROP_OVERFLOW) return reinterpret_cast<DropOverflowBuffer *>(this)->fill_remaining_to_buff( - new_str); + new_str, new_str_len); else if (write_mode_ == WriteMode::FLUSH_TO_STREAM) - return reinterpret_cast<FlushingBuffer *>(this)->flush_to_stream(new_str); + return reinterpret_cast<FlushingBuffer *>(this)->flush_to_stream( + new_str, new_str_len); else if (write_mode_ == WriteMode::RESIZE_AND_FILL_BUFF) - return reinterpret_cast<ResizingBuffer *>(this)->resize_and_write(new_str); + return reinterpret_cast<ResizingBuffer *>(this)->resize_and_write( + new_str, new_str_len); __builtin_unreachable(); } template <> LIBC_INLINE int WriteBuffer<WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::overflow_write( - cpp::string_view new_str) { + const CharType *new_str, size_t new_str_len) { return reinterpret_cast<DropOverflowBuffer *>(this)->fill_remaining_to_buff( - new_str); + new_str, new_str_len); } template <> -LIBC_INLINE int WriteBuffer<WriteMode::FLUSH_TO_STREAM>::overflow_write( - cpp::string_view new_str) { - return reinterpret_cast<FlushingBuffer *>(this)->flush_to_stream(new_str); +LIBC_INLINE int +WriteBuffer<WriteMode::FLUSH_TO_STREAM>::overflow_write(const CharType *new_str, + size_t new_str_len) { + return reinterpret_cast<FlushingBuffer *>(this)->flush_to_stream(new_str, + new_str_len); } template <> LIBC_INLINE int WriteBuffer<WriteMode::RESIZE_AND_FILL_BUFF>::overflow_write( - cpp::string_view new_str) { - return reinterpret_cast<ResizingBuffer *>(this)->resize_and_write(new_str); + const CharType *new_str, size_t new_str_len) { + return reinterpret_cast<ResizingBuffer *>(this)->resize_and_write( + new_str, new_str_len); } template <WriteMode write_mode> class Writer final { + using CharType = char; WriteBuffer<write_mode> &wb; size_t chars_written = 0; - LIBC_INLINE int pad(char new_char, size_t length) { + LIBC_INLINE int pad(CharType new_char, size_t length) { // First, fill as much of the buffer as possible with the padding char. size_t written = 0; const size_t buff_space = wb.buff_len - wb.buff_cur; @@ -184,17 +194,15 @@ template <WriteMode write_mode> class Writer final { // Next, overflow write the rest of length using the mini_buff. constexpr size_t MINI_BUFF_SIZE = 64; - char mini_buff[MINI_BUFF_SIZE]; + CharType mini_buff[MINI_BUFF_SIZE]; inline_memset(mini_buff, new_char, MINI_BUFF_SIZE); - cpp::string_view mb_string_view(mini_buff, MINI_BUFF_SIZE); while (written + MINI_BUFF_SIZE < length) { - int result = wb.overflow_write(mb_string_view); + int result = wb.overflow_write(mini_buff, MINI_BUFF_SIZE); if (result != WRITE_OK) return result; written += MINI_BUFF_SIZE; } - cpp::string_view mb_substr = mb_string_view.substr(0, length - written); - return wb.overflow_write(mb_substr); + return wb.overflow_write(mini_buff, length - written); } public: @@ -202,6 +210,7 @@ template <WriteMode write_mode> class Writer final { // Takes a string, copies it into the buffer if there is space, else passes it // to the overflow mechanism to be handled separately. + // TODO: Migrate all callers away from string_view to CharType*/size_t pair. LIBC_INLINE int write(cpp::string_view new_string) { chars_written += new_string.size(); if (LIBC_LIKELY(wb.buff_cur + new_string.size() <= wb.buff_len)) { @@ -210,18 +219,17 @@ template <WriteMode write_mode> class Writer final { wb.buff_cur += new_string.size(); return WRITE_OK; } - return wb.overflow_write(new_string); + return wb.overflow_write(new_string.data(), new_string.size()); } // Takes a char and a length, memsets the next length characters of the buffer // if there is space, else calls pad which will loop and call the overflow // mechanism on a secondary buffer. - LIBC_INLINE int write(char new_char, size_t length) { + LIBC_INLINE int write(CharType new_char, size_t length) { chars_written += length; if (LIBC_LIKELY(wb.buff_cur + length <= wb.buff_len)) { - inline_memset(wb.buff + wb.buff_cur, static_cast<unsigned char>(new_char), - length); + inline_memset(wb.buff + wb.buff_cur, new_char, length); wb.buff_cur += length; return WRITE_OK; } @@ -230,15 +238,14 @@ template <WriteMode write_mode> class Writer final { // Takes a char, copies it into the buffer if there is space, else passes it // to the overflow mechanism to be handled separately. - LIBC_INLINE int write(char new_char) { + LIBC_INLINE int write(CharType new_char) { chars_written += 1; if (LIBC_LIKELY(wb.buff_cur + 1 <= wb.buff_len)) { wb.buff[wb.buff_cur] = new_char; wb.buff_cur += 1; return WRITE_OK; } - cpp::string_view char_string_view(&new_char, 1); - return wb.overflow_write(char_string_view); + return wb.overflow_write(&new_char, 1); } LIBC_INLINE size_t get_chars_written() { return chars_written; } diff --git a/libc/test/src/stdio/printf_core/writer_test.cpp b/libc/test/src/stdio/printf_core/writer_test.cpp index e0128505b2766..8f14e38aab922 100644 --- a/libc/test/src/stdio/printf_core/writer_test.cpp +++ b/libc/test/src/stdio/printf_core/writer_test.cpp @@ -8,13 +8,11 @@ #include "src/stdio/printf_core/writer.h" -#include "src/__support/CPP/string_view.h" #include "src/string/memory_utils/inline_memcpy.h" #include "test/UnitTest/Test.h" namespace { -using LIBC_NAMESPACE::cpp::string_view; using LIBC_NAMESPACE::printf_core::DropOverflowBuffer; using LIBC_NAMESPACE::printf_core::FlushingBuffer; using LIBC_NAMESPACE::printf_core::WriteMode; @@ -196,17 +194,17 @@ struct OutBuff { size_t cur_pos = 0; }; -int copy_to_out(string_view new_str, void *raw_out_buff) { - if (new_str.size() == 0) { +int copy_to_out(const char *new_str, size_t new_str_len, void *raw_out_buff) { + if (new_str_len == 0) { return 0; } OutBuff *out_buff = reinterpret_cast<OutBuff *>(raw_out_buff); - LIBC_NAMESPACE::inline_memcpy(out_buff->out_str + out_buff->cur_pos, - new_str.data(), new_str.size()); + LIBC_NAMESPACE::inline_memcpy(out_buff->out_str + out_buff->cur_pos, new_str, + new_str_len); - out_buff->cur_pos += new_str.size(); + out_buff->cur_pos += new_str_len; return 0; } `````````` </details> https://github.com/llvm/llvm-project/pull/170959 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
