From 80f8d34427d40ec5e7ce3b10ededc46bd4bd5759 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Sun, 23 Oct 2022 17:21:36 +0300 Subject: [PATCH] fmt::ostream - aggregate buffer instead of inheriting it (#3139) Some MSVC-specific behavior: When class fmt::ostream inherits detail::buffer - the last gets implicitly exported when fmt is built as a shared library. Unless os.h is included, the compiler assumes detail::buffer is not externally exported and instantiates a local copy of it, which causes ODR violation. With aggregation - there is no extra exporting of detail::buffer symbols. --- include/fmt/os.h | 59 ++++++++++++++++++++++++++---------------------- src/os.cc | 26 ++++++++++++++++++++- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/include/fmt/os.h b/include/fmt/os.h index 8e697ec4..d94075a8 100644 --- a/include/fmt/os.h +++ b/include/fmt/os.h @@ -379,6 +379,28 @@ struct ostream_params { # endif }; +class file_buffer final : public buffer { + file file_; + + FMT_API void grow(size_t) override; + + public: + FMT_API file_buffer(cstring_view path, const ostream_params& params); + FMT_API file_buffer(file_buffer&& other); + FMT_API ~file_buffer(); + + void flush() { + if (size() == 0) return; + file_.write(data(), size() * sizeof(data()[0])); + clear(); + } + + void close() { + flush(); + file_.close(); + } +}; + FMT_END_DETAIL_NAMESPACE // Added {} below to work around default constructor error known to @@ -386,49 +408,32 @@ FMT_END_DETAIL_NAMESPACE constexpr detail::buffer_size buffer_size{}; /** A fast output stream which is not thread-safe. */ -class FMT_API ostream final : private detail::buffer { +class FMT_API ostream { private: - file file_; - - void grow(size_t) override; + FMT_MSC_WARNING(suppress : 4251) + detail::file_buffer buffer_; ostream(cstring_view path, const detail::ostream_params& params) - : file_(path, params.oflag) { - set(new char[params.buffer_size], params.buffer_size); - } + : buffer_(path, params) {} public: - ostream(ostream&& other) - : detail::buffer(other.data(), other.size(), other.capacity()), - file_(std::move(other.file_)) { - other.clear(); - other.set(nullptr, 0); - } - ~ostream() { - flush(); - delete[] data(); - } + ostream(ostream&& other) : buffer_(std::move(other.buffer_)) {} - void flush() { - if (size() == 0) return; - file_.write(data(), size()); - clear(); - } + ~ostream(); + + void flush() { buffer_.flush(); } template friend ostream output_file(cstring_view path, T... params); - void close() { - flush(); - file_.close(); - } + void close() { buffer_.close(); } /** Formats ``args`` according to specifications in ``fmt`` and writes the output to the file. */ template void print(format_string fmt, T&&... args) { - vformat_to(detail::buffer_appender(*this), fmt, + vformat_to(detail::buffer_appender(buffer_), fmt, fmt::make_format_args(args...)); } }; diff --git a/src/os.cc b/src/os.cc index 08bc749e..999d2a75 100644 --- a/src/os.cc +++ b/src/os.cc @@ -366,8 +366,32 @@ long getpagesize() { # endif } -FMT_API void ostream::grow(size_t) { +FMT_BEGIN_DETAIL_NAMESPACE + +void file_buffer::grow(size_t) { if (this->size() == this->capacity()) flush(); } + +file_buffer::file_buffer(cstring_view path, + const detail::ostream_params& params) + : file_(path, params.oflag) { + set(new char[params.buffer_size], params.buffer_size); +} + +file_buffer::file_buffer(file_buffer&& other) + : detail::buffer(other.data(), other.size(), other.capacity()), + file_(std::move(other.file_)) { + other.clear(); + other.set(nullptr, 0); +} + +file_buffer::~file_buffer() { + flush(); + delete[] data(); +} + +FMT_END_DETAIL_NAMESPACE + +ostream::~ostream() = default; #endif // FMT_USE_FCNTL FMT_END_NAMESPACE