fmt::ostream - aggregate buffer instead of inheriting it #3139

Youw · 2022-10-13T19:47:00Z

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 instantiets a local copy of it, which causes ODR violation.
With aggregation - there is no compiler assumptions that could cause an ODR violation.

Closes: #3132

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 instantiets a local copy of it, which causes ODR violation. With aggregation - there is no extra exporting of detail::buffer symbols.

Youw · 2022-10-13T19:50:50Z

Export table of fmt.dll before this change:

public: __cdecl fmt::v9::detail::locale_ref::locale_ref<class std::locale>(class std::locale const & __ptr64) __ptr64
public: void __cdecl fmt::v9::detail::buffer<char>::append<char>(char const * __ptr64,char const * __ptr64) __ptr64
public: void __cdecl fmt::v9::detail::buffer<wchar_t>::append<wchar_t>(wchar_t const * __ptr64,wchar_t const * __ptr64) __ptr64
char __cdecl fmt::v9::detail::decimal_point_impl<char>(class fmt::v9::detail::locale_ref)
wchar_t __cdecl fmt::v9::detail::decimal_point_impl<wchar_t>(class fmt::v9::detail::locale_ref)
public: class std::locale __cdecl fmt::v9::detail::locale_ref::get<class std::locale>(void)const __ptr64
struct fmt::v9::detail::thousands_sep_result<char> __cdecl fmt::v9::detail::thousands_sep_impl<char>(class fmt::v9::detail::locale_ref)
struct fmt::v9::detail::thousands_sep_result<wchar_t> __cdecl fmt::v9::detail::thousands_sep_impl<wchar_t>(class fmt::v9::detail::locale_ref)
struct fmt::v9::detail::dragonbox::decimal_fp<float> __cdecl fmt::v9::detail::dragonbox::to_decimal<float>(float)
struct fmt::v9::detail::dragonbox::decimal_fp<double> __cdecl fmt::v9::detail::dragonbox::to_decimal<double>(double)
void __cdecl fmt::v9::detail::vformat_to<char>(class fmt::v9::detail::buffer<char> & __ptr64,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >,class fmt::v9::detail::locale_ref)
protected: __cdecl fmt::v9::detail::buffer<char>::buffer<char>(class fmt::v9::detail::buffer<char> && __ptr64) __ptr64
protected: __cdecl fmt::v9::detail::buffer<char>::buffer<char>(char * __ptr64,unsigned __int64,unsigned __int64) __ptr64
protected: __cdecl fmt::v9::detail::buffer<char>::buffer<char>(unsigned __int64) __ptr64
public: __cdecl fmt::v9::buffered_file::buffered_file(class fmt::v9::basic_cstring_view<char>,class fmt::v9::basic_cstring_view<char>) __ptr64
private: __cdecl fmt::v9::file::file(int) __ptr64
public: __cdecl fmt::v9::file::file(class fmt::v9::file && __ptr64) __ptr64
public: __cdecl fmt::v9::file::file(class fmt::v9::basic_cstring_view<char>,int) __ptr64
public: __cdecl fmt::v9::file::file(void) __ptr64
public: __cdecl fmt::v9::format_error::format_error(class fmt::v9::format_error && __ptr64) __ptr64
public: __cdecl fmt::v9::format_error::format_error(class fmt::v9::format_error const & __ptr64) __ptr64
public: __cdecl fmt::v9::format_error::format_error(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const & __ptr64) __ptr64
public: __cdecl fmt::v9::format_error::format_error(char const * __ptr64) __ptr64
private: __cdecl fmt::v9::ostream::ostream(class fmt::v9::basic_cstring_view<char>,struct fmt::v9::detail::ostream_params const & __ptr64) __ptr64
public: __cdecl fmt::v9::ostream::ostream(class fmt::v9::ostream && __ptr64) __ptr64
public: __cdecl fmt::v9::detail::utf16_to_utf8::utf16_to_utf8(class fmt::v9::basic_string_view<wchar_t>) __ptr64
public: __cdecl fmt::v9::detail::utf8_to_utf16::utf8_to_utf16(class fmt::v9::basic_string_view<char>) __ptr64
protected: __cdecl fmt::v9::detail::buffer<char>::~buffer<char>(void) __ptr64
public: __cdecl fmt::v9::buffered_file::~buffered_file(void) __ptr64
public: __cdecl fmt::v9::file::~file(void) __ptr64
public: virtual __cdecl fmt::v9::format_error::~format_error(void) __ptr64
public: __cdecl fmt::v9::ostream::~ostream(void) __ptr64
public: class fmt::v9::file & __ptr64 __cdecl fmt::v9::file::operator=(class fmt::v9::file && __ptr64) __ptr64
public: class fmt::v9::format_error & __ptr64 __cdecl fmt::v9::format_error::operator=(class fmt::v9::format_error && __ptr64) __ptr64
public: class fmt::v9::format_error & __ptr64 __cdecl fmt::v9::format_error::operator=(class fmt::v9::format_error const & __ptr64) __ptr64
const fmt::v9::detail::buffer<char>::`vftable'
const fmt::v9::format_error::`vftable'
const fmt::v9::ostream::`vftable'
public: void __cdecl fmt::v9::detail::buffer<char>::`default constructor closure'(void) __ptr64
void __cdecl fmt::v9::detail::assert_fail(char const * __ptr64,int,char const * __ptr64)
public: char * __ptr64 __cdecl fmt::v9::detail::buffer<char>::begin(void) __ptr64
public: char const * __ptr64 __cdecl fmt::v9::detail::buffer<char>::begin(void)const __ptr64
public: unsigned __int64 __cdecl fmt::v9::detail::buffer<char>::capacity(void)const __ptr64
public: void __cdecl fmt::v9::detail::buffer<char>::clear(void) __ptr64
public: void __cdecl fmt::v9::buffered_file::close(void) __ptr64
public: void __cdecl fmt::v9::file::close(void) __ptr64
public: void __cdecl fmt::v9::ostream::close(void) __ptr64
public: int __cdecl fmt::v9::detail::utf16_to_utf8::convert(class fmt::v9::basic_string_view<wchar_t>) __ptr64
public: char * __ptr64 __cdecl fmt::v9::detail::buffer<char>::data(void) __ptr64
public: char const * __ptr64 __cdecl fmt::v9::detail::buffer<char>::data(void)const __ptr64
public: int __cdecl fmt::v9::buffered_file::descriptor(void)const __ptr64
public: int __cdecl fmt::v9::file::descriptor(void)const __ptr64
protected: virtual bool __cdecl fmt::v9::format_facet<class std::locale>::do_put(class fmt::v9::appender,class fmt::v9::loc_value,struct fmt::v9::basic_format_specs<char> const & __ptr64)const __ptr64
public: void __cdecl fmt::v9::file::dup2(int) __ptr64
public: void __cdecl fmt::v9::file::dup2(int,class std::error_code & __ptr64) __ptr64
public: static class fmt::v9::file __cdecl fmt::v9::file::dup(int)
public: char * __ptr64 __cdecl fmt::v9::detail::buffer<char>::end(void) __ptr64
public: char const * __ptr64 __cdecl fmt::v9::detail::buffer<char>::end(void)const __ptr64
public: class fmt::v9::buffered_file __cdecl fmt::v9::file::fdopen(char const * __ptr64) __ptr64
public: void __cdecl fmt::v9::ostream::flush(void) __ptr64
int __cdecl fmt::v9::detail::fmt_snprintf(char * __ptr64,unsigned __int64,char const * __ptr64,...)
void __cdecl fmt::v9::detail::format_error_code(class fmt::v9::detail::buffer<char> & __ptr64,int,class fmt::v9::basic_string_view<char>)
void __cdecl fmt::v9::format_system_error(class fmt::v9::detail::buffer<char> & __ptr64,int,char const * __ptr64)
void __cdecl fmt::v9::detail::format_windows_error(class fmt::v9::detail::buffer<char> & __ptr64,int,char const * __ptr64)
private: virtual void __cdecl fmt::v9::ostream::grow(unsigned __int64) __ptr64
public: static class std::locale::id fmt::v9::format_facet<class std::locale>::id
bool __cdecl fmt::v9::detail::is_printable(unsigned int)
public: static void __cdecl fmt::v9::file::pipe(class fmt::v9::file & __ptr64,class fmt::v9::file & __ptr64)
void __cdecl fmt::v9::detail::print(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>)
public: void __cdecl fmt::v9::detail::buffer<char>::push_back(char const & __ptr64) __ptr64
public: unsigned __int64 __cdecl fmt::v9::file::read(void * __ptr64,unsigned __int64) __ptr64
void __cdecl fmt::v9::detail::report_error(void (__cdecl*)(class fmt::v9::detail::buffer<char> & __ptr64,int,char const * __ptr64),int,char const * __ptr64)
void __cdecl fmt::v9::report_system_error(int,char const * __ptr64)
void __cdecl fmt::v9::report_windows_error(int,char const * __ptr64)
protected: void __cdecl fmt::v9::detail::buffer<char>::set(char * __ptr64,unsigned __int64) __ptr64
public: unsigned __int64 __cdecl fmt::v9::detail::buffer<char>::size(void)const __ptr64
public: __int64 __cdecl fmt::v9::file::size(void)const __ptr64
class std::error_category const & __ptr64 __cdecl fmt::v9::system_category(void)
void __cdecl fmt::v9::detail::throw_format_error(char const * __ptr64)
public: void __cdecl fmt::v9::detail::buffer<char>::try_reserve(unsigned __int64) __ptr64
public: void __cdecl fmt::v9::detail::buffer<char>::try_resize(unsigned __int64) __ptr64
class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > __cdecl fmt::v9::vformat(class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
void __cdecl fmt::v9::vprint(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
void __cdecl fmt::v9::vprint(class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
void __cdecl fmt::v9::detail::vprint_mojibake(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
class std::system_error __cdecl fmt::v9::vsystem_error(int,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
class std::system_error __cdecl fmt::v9::vwindows_error(int,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
public: unsigned __int64 __cdecl fmt::v9::file::write(void const * __ptr64,unsigned __int64) __ptr64
bool __cdecl fmt::v9::detail::write_console(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>)
bool __cdecl fmt::v9::detail::write_loc(class fmt::v9::appender,class fmt::v9::loc_value,struct fmt::v9::basic_format_specs<char> const & __ptr64,class fmt::v9::detail::locale_ref)

After:

public: __cdecl fmt::v9::detail::locale_ref::locale_ref<class std::locale>(class std::locale const & __ptr64) __ptr64
public: void __cdecl fmt::v9::detail::buffer<char>::append<char>(char const * __ptr64,char const * __ptr64) __ptr64
public: void __cdecl fmt::v9::detail::buffer<wchar_t>::append<wchar_t>(wchar_t const * __ptr64,wchar_t const * __ptr64) __ptr64
char __cdecl fmt::v9::detail::decimal_point_impl<char>(class fmt::v9::detail::locale_ref)
wchar_t __cdecl fmt::v9::detail::decimal_point_impl<wchar_t>(class fmt::v9::detail::locale_ref)
public: class std::locale __cdecl fmt::v9::detail::locale_ref::get<class std::locale>(void)const __ptr64
struct fmt::v9::detail::thousands_sep_result<char> __cdecl fmt::v9::detail::thousands_sep_impl<char>(class fmt::v9::detail::locale_ref)
struct fmt::v9::detail::thousands_sep_result<wchar_t> __cdecl fmt::v9::detail::thousands_sep_impl<wchar_t>(class fmt::v9::detail::locale_ref)
struct fmt::v9::detail::dragonbox::decimal_fp<float> __cdecl fmt::v9::detail::dragonbox::to_decimal<float>(float)
struct fmt::v9::detail::dragonbox::decimal_fp<double> __cdecl fmt::v9::detail::dragonbox::to_decimal<double>(double)
void __cdecl fmt::v9::detail::vformat_to<char>(class fmt::v9::detail::buffer<char> & __ptr64,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >,class fmt::v9::detail::locale_ref)
public: __cdecl fmt::v9::buffered_file::buffered_file(class fmt::v9::basic_cstring_view<char>,class fmt::v9::basic_cstring_view<char>) __ptr64
private: __cdecl fmt::v9::file::file(int) __ptr64
public: __cdecl fmt::v9::file::file(class fmt::v9::file && __ptr64) __ptr64
public: __cdecl fmt::v9::file::file(class fmt::v9::basic_cstring_view<char>,int) __ptr64
public: __cdecl fmt::v9::file::file(void) __ptr64
public: __cdecl fmt::v9::format_error::format_error(class fmt::v9::format_error && __ptr64) __ptr64
public: __cdecl fmt::v9::format_error::format_error(class fmt::v9::format_error const & __ptr64) __ptr64
public: __cdecl fmt::v9::format_error::format_error(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const & __ptr64) __ptr64
public: __cdecl fmt::v9::format_error::format_error(char const * __ptr64) __ptr64
private: __cdecl fmt::v9::ostream::ostream(class fmt::v9::basic_cstring_view<char>,struct fmt::v9::detail::ostream_params const & __ptr64) __ptr64
public: __cdecl fmt::v9::ostream::ostream(class fmt::v9::ostream && __ptr64) __ptr64
public: __cdecl fmt::v9::detail::utf16_to_utf8::utf16_to_utf8(class fmt::v9::basic_string_view<wchar_t>) __ptr64
public: __cdecl fmt::v9::detail::utf8_to_utf16::utf8_to_utf16(class fmt::v9::basic_string_view<char>) __ptr64
public: __cdecl fmt::v9::buffered_file::~buffered_file(void) __ptr64
public: __cdecl fmt::v9::file::~file(void) __ptr64
public: virtual __cdecl fmt::v9::format_error::~format_error(void) __ptr64
public: __cdecl fmt::v9::ostream::~ostream(void) __ptr64
public: class fmt::v9::file & __ptr64 __cdecl fmt::v9::file::operator=(class fmt::v9::file && __ptr64) __ptr64
public: class fmt::v9::format_error & __ptr64 __cdecl fmt::v9::format_error::operator=(class fmt::v9::format_error && __ptr64) __ptr64
public: class fmt::v9::format_error & __ptr64 __cdecl fmt::v9::format_error::operator=(class fmt::v9::format_error const & __ptr64) __ptr64
const fmt::v9::format_error::`vftable'
void __cdecl fmt::v9::detail::assert_fail(char const * __ptr64,int,char const * __ptr64)
public: void __cdecl fmt::v9::buffered_file::close(void) __ptr64
public: void __cdecl fmt::v9::file::close(void) __ptr64
public: void __cdecl fmt::v9::ostream::close(void) __ptr64
public: int __cdecl fmt::v9::detail::utf16_to_utf8::convert(class fmt::v9::basic_string_view<wchar_t>) __ptr64
public: int __cdecl fmt::v9::buffered_file::descriptor(void)const __ptr64
public: int __cdecl fmt::v9::file::descriptor(void)const __ptr64
protected: virtual bool __cdecl fmt::v9::format_facet<class std::locale>::do_put(class fmt::v9::appender,class fmt::v9::loc_value,struct fmt::v9::basic_format_specs<char> const & __ptr64)const __ptr64
public: void __cdecl fmt::v9::file::dup2(int) __ptr64
public: void __cdecl fmt::v9::file::dup2(int,class std::error_code & __ptr64) __ptr64
public: static class fmt::v9::file __cdecl fmt::v9::file::dup(int)
public: class fmt::v9::buffered_file __cdecl fmt::v9::file::fdopen(char const * __ptr64) __ptr64
public: void __cdecl fmt::v9::ostream::flush(void) __ptr64
int __cdecl fmt::v9::detail::fmt_snprintf(char * __ptr64,unsigned __int64,char const * __ptr64,...)
void __cdecl fmt::v9::detail::format_error_code(class fmt::v9::detail::buffer<char> & __ptr64,int,class fmt::v9::basic_string_view<char>)
void __cdecl fmt::v9::format_system_error(class fmt::v9::detail::buffer<char> & __ptr64,int,char const * __ptr64)
void __cdecl fmt::v9::detail::format_windows_error(class fmt::v9::detail::buffer<char> & __ptr64,int,char const * __ptr64)
private: virtual void __cdecl fmt::v9::detail::ostream_buffer::grow(unsigned __int64) __ptr64
public: static class std::locale::id fmt::v9::format_facet<class std::locale>::id
bool __cdecl fmt::v9::detail::is_printable(unsigned int)
public: static void __cdecl fmt::v9::file::pipe(class fmt::v9::file & __ptr64,class fmt::v9::file & __ptr64)
void __cdecl fmt::v9::detail::print(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>)
public: unsigned __int64 __cdecl fmt::v9::file::read(void * __ptr64,unsigned __int64) __ptr64
void __cdecl fmt::v9::detail::report_error(void (__cdecl*)(class fmt::v9::detail::buffer<char> & __ptr64,int,char const * __ptr64),int,char const * __ptr64)
void __cdecl fmt::v9::report_system_error(int,char const * __ptr64)
void __cdecl fmt::v9::report_windows_error(int,char const * __ptr64)
public: __int64 __cdecl fmt::v9::file::size(void)const __ptr64
class std::error_category const & __ptr64 __cdecl fmt::v9::system_category(void)
void __cdecl fmt::v9::detail::throw_format_error(char const * __ptr64)
class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > __cdecl fmt::v9::vformat(class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
void __cdecl fmt::v9::vprint(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
void __cdecl fmt::v9::vprint(class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
void __cdecl fmt::v9::detail::vprint_mojibake(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
class std::system_error __cdecl fmt::v9::vsystem_error(int,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
class std::system_error __cdecl fmt::v9::vwindows_error(int,class fmt::v9::basic_string_view<char>,class fmt::v9::basic_format_args<class fmt::v9::basic_format_context<class fmt::v9::appender,char> >)
public: unsigned __int64 __cdecl fmt::v9::file::write(void const * __ptr64,unsigned __int64) __ptr64
bool __cdecl fmt::v9::detail::write_console(struct _iobuf * __ptr64,class fmt::v9::basic_string_view<char>)
bool __cdecl fmt::v9::detail::write_loc(class fmt::v9::appender,class fmt::v9::loc_value,struct fmt::v9::basic_format_specs<char> const & __ptr64,class fmt::v9::detail::locale_ref)

Diff:

< protected: __cdecl fmt::v9::detail::buffer<char>::buffer<char>(class fmt::v9::detail::buffer<char> && __ptr64) __ptr64
< protected: __cdecl fmt::v9::detail::buffer<char>::buffer<char>(char * __ptr64,unsigned __int64,unsigned __int64) __ptr64
< protected: __cdecl fmt::v9::detail::buffer<char>::buffer<char>(unsigned __int64) __ptr64
< protected: __cdecl fmt::v9::detail::buffer<char>::~buffer<char>(void) __ptr64
< const fmt::v9::detail::buffer<char>::`vftable'
< const fmt::v9::ostream::`vftable'
< public: void __cdecl fmt::v9::detail::buffer<char>::`default constructor closure'(void) __ptr64
< public: char * __ptr64 __cdecl fmt::v9::detail::buffer<char>::begin(void) __ptr64
< public: char const * __ptr64 __cdecl fmt::v9::detail::buffer<char>::begin(void)const __ptr64
< public: unsigned __int64 __cdecl fmt::v9::detail::buffer<char>::capacity(void)const __ptr64
< public: void __cdecl fmt::v9::detail::buffer<char>::clear(void) __ptr64
< public: char * __ptr64 __cdecl fmt::v9::detail::buffer<char>::data(void) __ptr64
< public: char const * __ptr64 __cdecl fmt::v9::detail::buffer<char>::data(void)const __ptr64
< public: char * __ptr64 __cdecl fmt::v9::detail::buffer<char>::end(void) __ptr64
< public: char const * __ptr64 __cdecl fmt::v9::detail::buffer<char>::end(void)const __ptr64
< private: virtual void __cdecl fmt::v9::ostream::grow(unsigned __int64) __ptr64
> private: virtual void __cdecl fmt::v9::detail::ostream_buffer::grow(unsigned __int64) __ptr64
< public: void __cdecl fmt::v9::detail::buffer<char>::push_back(char const & __ptr64) __ptr64
< protected: void __cdecl fmt::v9::detail::buffer<char>::set(char * __ptr64,unsigned __int64) __ptr64
< public: unsigned __int64 __cdecl fmt::v9::detail::buffer<char>::size(void)const __ptr64
< public: void __cdecl fmt::v9::detail::buffer<char>::try_reserve(unsigned __int64) __ptr64
< public: void __cdecl fmt::v9::detail::buffer<char>::try_resize(unsigned __int64) __ptr64

Youw · 2022-10-13T19:54:21Z

CI failure:

undefined reference to `vtable for fmt::v9::detail::ostream_buffer'

Looks like the grow function has to be inlined as well, to avoid anchoring.
@vitaut do you think that's alright?

Youw · 2022-10-13T20:25:54Z

to avoid anchoring

But that would spawn the v-table for stream_buffer in each translation unit it is included (at least as per itanium/clang arch)...
I starting to realize why fmt::ostream is exported in a first place.

Youw · 2022-10-13T20:30:58Z

that would spawn the v-table for stream_buffer in each translation unit

But the same story with fmt::memory_buffer, so maybe that is not an issue.

src/os.cc

include/fmt/os.h

Youw · 2022-10-14T09:39:16Z

Interesting that with the latest change - the amount of export symbols is exactly the same (detail::buffer<char> still gets exported) but there is no longer linker issues/ODR violation in the scenario I've mentioned in #3132.

vitaut

Thanks for the PR.

Do I understand correctly that this is an alternative to #3132?

vitaut · 2022-10-16T16:37:12Z

include/fmt/os.h

-/** A fast output stream which is not thread-safe. */
-class FMT_API ostream final : private detail::buffer<char> {
- private:
+template <typename Char> class file_buffer final : public detail::buffer<Char> {


This doesn't need to be a template because file I/O is char-based.

doesn't need to be

That is exactly what I've tried in the previous commit.
But compilers generally are not happy about it.

I simply took the same approach as with memory_buffer.

You should be able to fix that warning by outlining some virtual method. memory_buffer is different because it has to be a template.

by outlining some virtual method

That I did and got a different problem - the v-table gets anchored into a shared library but not exported, which makes it unavailable outside of a shared library.

Initially I thought that the only fix would be to export the entire class (to export a v-table) which is the opposite of this entire change. But after giving it a second though - I think it would be enough to outline c-tor and other special functions. Let me try that.

done - it worked

Youw · 2022-10-16T21:08:44Z

Do I understand correctly that this is an alternative to #3132?

Yes.

vitaut

Overall looks good, a few minor comments inline.

include/fmt/os.h

vitaut · 2022-10-22T13:56:49Z

include/fmt/os.h

-  file file_;
-
-  void grow(size_t) override;
+  FMT_MSC_WARNING(suppress : 4251)


What is this for?

warning C4251: class 'detail::file_buffer' needs to have dll-interface ...

I.e. MSVC doesn't like that ostream is exported and detail::file_buffer - is not

vitaut · 2022-10-22T13:57:53Z

src/os.cc

+
+FMT_END_DETAIL_NAMESPACE
+
+ostream::~ostream() = default;


Does it need to be outlined?

it guess that's my old habit - always anchor class by its d-tor
otherwise - how would compiler know where to place definition of functions marked with FMT_API?

maybe that is not the case for this particular class - but that's the case for polymorphic classes
at least I didn't check if it would behave fine w/o it

vitaut · 2022-10-23T14:21:41Z

Thanks

Youw commented Oct 14, 2022

View reviewed changes

src/os.cc Outdated Show resolved Hide resolved

include/fmt/os.h Show resolved Hide resolved

Youw added 2 commits October 14, 2022 11:42

Remove v-table anchor for detail::ostream_buffer

e75f9b8

attempt to make the compiler happy

4e0c866

vitaut requested changes Oct 16, 2022

View reviewed changes

vitaut mentioned this pull request Oct 18, 2022

Explicitly export buffer<char> for MSVC #3132

Closed

non-template solution attempt

07ca088

vitaut requested changes Oct 22, 2022

View reviewed changes

Youw added 2 commits October 23, 2022 00:36

Apply suggestions from code review

8bf75f9

Apply suggestions from code review

7fa287d

vitaut approved these changes Oct 23, 2022

View reviewed changes

vitaut merged commit 80f8d34 into fmtlib:master Oct 23, 2022

Youw deleted the ostream_aggregate_buffer branch October 23, 2022 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fmt::ostream - aggregate buffer instead of inheriting it #3139

fmt::ostream - aggregate buffer instead of inheriting it #3139

Youw commented Oct 13, 2022 •

edited

Loading

Youw commented Oct 13, 2022

Youw commented Oct 13, 2022

Youw commented Oct 13, 2022

Youw commented Oct 13, 2022

Youw commented Oct 14, 2022 •

edited

Loading

vitaut left a comment

vitaut Oct 16, 2022

Youw Oct 16, 2022

vitaut Oct 18, 2022

Youw Oct 18, 2022

Youw Oct 20, 2022

Youw commented Oct 16, 2022

vitaut left a comment

vitaut Oct 22, 2022

Youw Oct 22, 2022 •

edited

Loading

vitaut Oct 22, 2022

Youw Oct 22, 2022

vitaut commented Oct 23, 2022


		FMT_END_DETAIL_NAMESPACE

		ostream::~ostream() = default;

fmt::ostream - aggregate buffer instead of inheriting it #3139

fmt::ostream - aggregate buffer instead of inheriting it #3139

Conversation

Youw commented Oct 13, 2022 • edited Loading

Youw commented Oct 13, 2022

Youw commented Oct 13, 2022

Youw commented Oct 13, 2022

Youw commented Oct 13, 2022

Youw commented Oct 14, 2022 • edited Loading

vitaut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Youw commented Oct 16, 2022

vitaut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Youw Oct 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaut commented Oct 23, 2022

Youw commented Oct 13, 2022 •

edited

Loading

Youw commented Oct 14, 2022 •

edited

Loading

Youw Oct 22, 2022 •

edited

Loading