Skip to content

Commit

Permalink
fmt::ostream - aggregate buffer instead of inheriting it (#3139)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Youw authored Oct 23, 2022
1 parent 64965bd commit 80f8d34
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 28 deletions.
59 changes: 32 additions & 27 deletions include/fmt/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,56 +379,61 @@ struct ostream_params {
# endif
};

class file_buffer final : public buffer<char> {
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
// occur in Xcode versions 7.2.1 and 8.2.1.
constexpr detail::buffer_size buffer_size{};

/** A fast output stream which is not thread-safe. */
class FMT_API ostream final : private detail::buffer<char> {
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<char>(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 <typename... T>
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 <typename... T> void print(format_string<T...> fmt, T&&... args) {
vformat_to(detail::buffer_appender<char>(*this), fmt,
vformat_to(detail::buffer_appender<char>(buffer_), fmt,
fmt::make_format_args(args...));
}
};
Expand Down
26 changes: 25 additions & 1 deletion src/os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<char>(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

0 comments on commit 80f8d34

Please sign in to comment.