Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module linkage fixes for shared build #4169

Merged
merged 10 commits into from
Sep 26, 2024
Merged

Module linkage fixes for shared build #4169

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e1789cf
Mark some in-class defined member functions as explicitly inline/cons…
kamrann Sep 20, 2024
27f9966
Merge branch 'master' into pr/module-linkage-fixes
kamrann Sep 20, 2024
9ca41fa
Switch constexpr to inline for context::arg(string_view).
kamrann Sep 20, 2024
a67e251
Work around apparent non-conformance of older MSVC compilers.
kamrann Sep 20, 2024
f9e99e5
Switch format_int::str() from constexpr to inline to satisfy libstdc+…
kamrann Sep 20, 2024
afb5f21
Replace usages of macros for constexpr/inline with keywords.
kamrann Sep 23, 2024
8765197
Merge branch 'master' into pr/module-linkage-fixes
kamrann Sep 23, 2024
076afe3
Fix for locations requiring C++14 constexpr.
kamrann Sep 23, 2024
7226c0e
Further minor constexpr tweaks.
kamrann Sep 23, 2024
d6c7200
Apply clang format
kamrann Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions include/fmt/base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1792,9 +1792,9 @@ template <typename T> class buffer {
};

struct buffer_traits {
explicit buffer_traits(size_t) {}
auto count() const -> size_t { return 0; }
auto limit(size_t size) -> size_t { return size; }
FMT_CONSTEXPR explicit buffer_traits(size_t) {}
FMT_CONSTEXPR auto count() const -> size_t { return 0; }
FMT_CONSTEXPR auto limit(size_t size) -> size_t { return size; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be constexpr. FMT_CONSTEXPR is only for cases when C++14 relaxed constexpr is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay my bad, hadn't realized the distinction. I think all the changes are now constexpr where they can be.

};

class fixed_buffer_traits {
Expand All @@ -1803,9 +1803,9 @@ class fixed_buffer_traits {
size_t limit_;

public:
explicit fixed_buffer_traits(size_t limit) : limit_(limit) {}
auto count() const -> size_t { return count_; }
auto limit(size_t size) -> size_t {
FMT_CONSTEXPR explicit fixed_buffer_traits(size_t limit) : limit_(limit) {}
FMT_CONSTEXPR auto count() const -> size_t { return count_; }
FMT_CONSTEXPR auto limit(size_t size) -> size_t {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here and in a few more places below.

size_t n = limit_ > count_ ? limit_ - count_ : 0;
count_ += size;
return size < n ? size : n;
Expand Down Expand Up @@ -2223,7 +2223,7 @@ struct locale_ref {
public:
constexpr locale_ref() : locale_(nullptr) {}
template <typename Locale> explicit locale_ref(const Locale& loc);
explicit operator bool() const noexcept { return locale_ != nullptr; }
FMT_INLINE explicit operator bool() const noexcept { return locale_ != nullptr; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere, please use inline instead of FMT_INLINE since the latter expands to always inline in optimized mode.

#endif

template <typename Locale> auto get() const -> Locale;
Expand Down Expand Up @@ -2598,7 +2598,7 @@ class context : private detail::locale_ref {
void operator=(const context&) = delete;

FMT_CONSTEXPR auto arg(int id) const -> format_arg { return args_.get(id); }
auto arg(string_view name) -> format_arg { return args_.get(name); }
FMT_INLINE auto arg(string_view name) -> format_arg { return args_.get(name); }
FMT_CONSTEXPR auto arg_id(string_view name) -> int {
return args_.get_id(name);
}
Expand All @@ -2607,7 +2607,7 @@ class context : private detail::locale_ref {
FMT_CONSTEXPR auto out() -> iterator { return out_; }

// Advances the begin iterator to `it`.
void advance_to(iterator) {}
FMT_CONSTEXPR void advance_to(iterator) {}

FMT_CONSTEXPR auto locale() -> detail::locale_ref { return *this; }
};
Expand Down
34 changes: 17 additions & 17 deletions include/fmt/chrono.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,24 +540,24 @@ inline auto localtime(std::time_t time) -> std::tm {
std::time_t time_;
std::tm tm_;

dispatcher(std::time_t t) : time_(t) {}
FMT_INLINE dispatcher(std::time_t t) : time_(t) {}

auto run() -> bool {
FMT_INLINE auto run() -> bool {
using namespace fmt::detail;
return handle(localtime_r(&time_, &tm_));
}

auto handle(std::tm* tm) -> bool { return tm != nullptr; }
FMT_INLINE auto handle(std::tm* tm) -> bool { return tm != nullptr; }

auto handle(detail::null<>) -> bool {
FMT_INLINE auto handle(detail::null<>) -> bool {
using namespace fmt::detail;
return fallback(localtime_s(&tm_, &time_));
}

auto fallback(int res) -> bool { return res == 0; }
FMT_INLINE auto fallback(int res) -> bool { return res == 0; }

#if !FMT_MSC_VERSION
auto fallback(detail::null<>) -> bool {
FMT_INLINE auto fallback(detail::null<>) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed since the struct where these functions are defined is inaccessible to module consumers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I may have branched out a little when making the changes, the vast majority of them were made on functions that were generating linker errors. I've verified again, these are necessary for the linker to succeed.

Essentially, the issue being addressed here is one of dll symbol export, not module name export, and so the visibility of names (which is what module export/detail namespaces relate to) I think has no bearing. Since this struct is in an inline function which is exposed, a consumer TU that calls this function (directly or transitively) will either need to inline the member functions of the struct, or emit an external symbol reference for them. So the same choice still applies.

using namespace fmt::detail;
std::tm* tm = std::localtime(&time_);
if (tm) tm_ = *tm;
Expand Down Expand Up @@ -591,24 +591,24 @@ inline auto gmtime(std::time_t time) -> std::tm {
std::time_t time_;
std::tm tm_;

dispatcher(std::time_t t) : time_(t) {}
FMT_INLINE dispatcher(std::time_t t) : time_(t) {}

auto run() -> bool {
FMT_INLINE auto run() -> bool {
using namespace fmt::detail;
return handle(gmtime_r(&time_, &tm_));
}

auto handle(std::tm* tm) -> bool { return tm != nullptr; }
FMT_INLINE auto handle(std::tm* tm) -> bool { return tm != nullptr; }

auto handle(detail::null<>) -> bool {
FMT_INLINE auto handle(detail::null<>) -> bool {
using namespace fmt::detail;
return fallback(gmtime_s(&tm_, &time_));
}

auto fallback(int res) -> bool { return res == 0; }
FMT_INLINE auto fallback(int res) -> bool { return res == 0; }

#if !FMT_MSC_VERSION
auto fallback(detail::null<>) -> bool {
FMT_INLINE auto fallback(detail::null<>) -> bool {
std::tm* tm = std::gmtime(&time_);
if (tm) tm_ = *tm;
return tm != nullptr;
Expand Down Expand Up @@ -912,7 +912,7 @@ template <typename Derived> struct null_chrono_spec_handler {
};

struct tm_format_checker : null_chrono_spec_handler<tm_format_checker> {
FMT_NORETURN void unsupported() { FMT_THROW(format_error("no format")); }
FMT_NORETURN FMT_INLINE void unsupported() { FMT_THROW(format_error("no format")); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and in other places in the detail namespace - inline is not needed since it is not exported from the module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, detail namespace functions will still generate linker errors if neither inline nor dll-exported.


template <typename Char>
FMT_CONSTEXPR void on_text(const Char*, const Char*) {}
Expand Down Expand Up @@ -1572,7 +1572,7 @@ class tm_writer {
struct chrono_format_checker : null_chrono_spec_handler<chrono_format_checker> {
bool has_precision_integral = false;

FMT_NORETURN void unsupported() { FMT_THROW(format_error("no date")); }
FMT_NORETURN FMT_INLINE void unsupported() { FMT_THROW(format_error("no date")); }

template <typename Char>
FMT_CONSTEXPR void on_text(const Char*, const Char*) {}
Expand Down Expand Up @@ -1693,14 +1693,14 @@ class get_locale {
bool has_locale_ = false;

public:
get_locale(bool localized, locale_ref loc) : has_locale_(localized) {
FMT_INLINE get_locale(bool localized, locale_ref loc) : has_locale_(localized) {
if (localized)
::new (&locale_) std::locale(loc.template get<std::locale>());
}
~get_locale() {
FMT_INLINE ~get_locale() {
if (has_locale_) locale_.~locale();
}
operator const std::locale&() const {
FMT_INLINE operator const std::locale&() const {
return has_locale_ ? locale_ : get_classic_locale();
}
};
Expand Down
30 changes: 15 additions & 15 deletions include/fmt/format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,21 +376,21 @@ class uint128_fallback {
-> uint128_fallback {
return {~n.hi_, ~n.lo_};
}
friend auto operator+(const uint128_fallback& lhs,
const uint128_fallback& rhs) -> uint128_fallback {
FMT_CONSTEXPR friend auto operator+(const uint128_fallback& lhs,
const uint128_fallback& rhs) -> uint128_fallback {
auto result = uint128_fallback(lhs);
result += rhs;
return result;
}
friend auto operator*(const uint128_fallback& lhs, uint32_t rhs)
FMT_CONSTEXPR friend auto operator*(const uint128_fallback& lhs, uint32_t rhs)
-> uint128_fallback {
FMT_ASSERT(lhs.hi_ == 0, "");
uint64_t hi = (lhs.lo_ >> 32) * rhs;
uint64_t lo = (lhs.lo_ & ~uint32_t()) * rhs;
uint64_t new_lo = (hi << 32) + lo;
return {(hi >> 32) + (new_lo < lo ? 1 : 0), new_lo};
}
friend auto operator-(const uint128_fallback& lhs, uint64_t rhs)
FMT_CONSTEXPR friend auto operator-(const uint128_fallback& lhs, uint64_t rhs)
-> uint128_fallback {
return {lhs.hi_ - (lhs.lo_ < rhs ? 1 : 0), lhs.lo_ - rhs};
}
Expand Down Expand Up @@ -964,8 +964,8 @@ class writer {
FILE* file_;

public:
writer(FILE* f) : buf_(nullptr), file_(f) {}
writer(detail::buffer<char>& buf) : buf_(&buf) {}
FMT_INLINE writer(FILE* f) : buf_(nullptr), file_(f) {}
FMT_INLINE writer(detail::buffer<char>& buf) : buf_(&buf) {}

/// Formats `args` according to specifications in `fmt` and writes the
/// output to the file.
Expand All @@ -983,10 +983,10 @@ class string_buffer {
detail::container_buffer<std::string> buf_;

public:
string_buffer() : buf_(str_) {}
FMT_INLINE string_buffer() : buf_(str_) {}

operator writer() { return buf_; }
std::string& str() { return str_; }
FMT_INLINE operator writer() { return buf_; }
FMT_INLINE std::string& str() { return str_; }
};

template <typename T, size_t SIZE, typename Allocator>
Expand Down Expand Up @@ -1416,10 +1416,10 @@ class utf8_to_utf16 {

public:
FMT_API explicit utf8_to_utf16(string_view s);
operator basic_string_view<wchar_t>() const { return {&buffer_[0], size()}; }
auto size() const -> size_t { return buffer_.size() - 1; }
auto c_str() const -> const wchar_t* { return &buffer_[0]; }
auto str() const -> std::wstring { return {&buffer_[0], size()}; }
FMT_INLINE operator basic_string_view<wchar_t>() const { return {&buffer_[0], size()}; }
FMT_INLINE auto size() const -> size_t { return buffer_.size() - 1; }
FMT_INLINE auto c_str() const -> const wchar_t* { return &buffer_[0]; }
FMT_INLINE auto str() const -> std::wstring { return {&buffer_[0], size()}; }
};

enum class to_utf8_error_policy { abort, replace };
Expand Down Expand Up @@ -3925,7 +3925,7 @@ class bytes {
friend struct formatter<bytes>;

public:
explicit bytes(string_view data) : data_(data) {}
FMT_INLINE explicit bytes(string_view data) : data_(data) {}
};

template <> struct formatter<bytes> {
Expand Down Expand Up @@ -4134,7 +4134,7 @@ class format_int {
}

/// Returns the content of the output buffer as an `std::string`.
auto str() const -> std::string { return {str_, size()}; }
FMT_INLINE auto str() const -> std::string { return {str_, size()}; }
};

#define FMT_STRING_IMPL(s, base) \
Expand Down
28 changes: 14 additions & 14 deletions include/fmt/os.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,24 +176,24 @@ class buffered_file {

friend class file;

explicit buffered_file(FILE* f) : file_(f) {}
FMT_INLINE explicit buffered_file(FILE* f) : file_(f) {}

public:
buffered_file(const buffered_file&) = delete;
void operator=(const buffered_file&) = delete;

// Constructs a buffered_file object which doesn't represent any file.
buffered_file() noexcept : file_(nullptr) {}
FMT_INLINE buffered_file() noexcept : file_(nullptr) {}

// Destroys the object closing the file it represents if any.
FMT_API ~buffered_file() noexcept;

public:
buffered_file(buffered_file&& other) noexcept : file_(other.file_) {
FMT_INLINE buffered_file(buffered_file&& other) noexcept : file_(other.file_) {
other.file_ = nullptr;
}

auto operator=(buffered_file&& other) -> buffered_file& {
FMT_INLINE auto operator=(buffered_file&& other) -> buffered_file& {
close();
file_ = other.file_;
other.file_ = nullptr;
Expand All @@ -207,7 +207,7 @@ class buffered_file {
FMT_API void close();

// Returns the pointer to a FILE object representing this file.
auto get() const noexcept -> FILE* { return file_; }
FMT_INLINE auto get() const noexcept -> FILE* { return file_; }

FMT_API auto descriptor() const -> int;

Expand Down Expand Up @@ -248,7 +248,7 @@ class FMT_API file {
};

// Constructs a file object which doesn't represent any file.
file() noexcept : fd_(-1) {}
FMT_INLINE file() noexcept : fd_(-1) {}

// Opens a file and constructs a file object representing this file.
file(cstring_view path, int oflag);
Expand All @@ -257,10 +257,10 @@ class FMT_API file {
file(const file&) = delete;
void operator=(const file&) = delete;

file(file&& other) noexcept : fd_(other.fd_) { other.fd_ = -1; }
FMT_INLINE file(file&& other) noexcept : fd_(other.fd_) { other.fd_ = -1; }

// Move assignment is not noexcept because close may throw.
auto operator=(file&& other) -> file& {
FMT_INLINE auto operator=(file&& other) -> file& {
close();
fd_ = other.fd_;
other.fd_ = -1;
Expand All @@ -271,7 +271,7 @@ class FMT_API file {
~file() noexcept;

// Returns the file descriptor.
auto descriptor() const noexcept -> int { return fd_; }
FMT_INLINE auto descriptor() const noexcept -> int { return fd_; }

// Closes the file.
void close();
Expand Down Expand Up @@ -324,9 +324,9 @@ auto getpagesize() -> long;
namespace detail {

struct buffer_size {
buffer_size() = default;
FMT_CONSTEXPR buffer_size() = default;
size_t value = 0;
auto operator=(size_t val) const -> buffer_size {
FMT_CONSTEXPR auto operator=(size_t val) const -> buffer_size {
auto bs = buffer_size();
bs.value = val;
return bs;
Expand All @@ -337,7 +337,7 @@ struct ostream_params {
int oflag = file::WRONLY | file::CREATE | file::TRUNC;
size_t buffer_size = BUFSIZ > 32768 ? BUFSIZ : 32768;

ostream_params() {}
FMT_CONSTEXPR ostream_params() {}

template <typename... T>
ostream_params(T... params, int new_oflag) : ostream_params(params...) {
Expand Down Expand Up @@ -381,7 +381,7 @@ class FMT_API ostream : private detail::buffer<char> {
return buf;
}

void flush() {
FMT_INLINE void flush() {
if (size() == 0) return;
file_.write(data(), size() * sizeof(data()[0]));
clear();
Expand All @@ -390,7 +390,7 @@ class FMT_API ostream : private detail::buffer<char> {
template <typename... T>
friend auto output_file(cstring_view path, T... params) -> ostream;

void close() {
FMT_INLINE void close() {
flush();
file_.close();
}
Expand Down
6 changes: 3 additions & 3 deletions include/fmt/printf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ template <bool IsSigned> struct int_checker {
unsigned max = to_unsigned(max_value<int>());
return value <= max;
}
static auto fits_in_int(bool) -> bool { return true; }
FMT_INLINE static auto fits_in_int(bool) -> bool { return true; }
};

template <> struct int_checker<true> {
template <typename T> static auto fits_in_int(T value) -> bool {
return value >= (std::numeric_limits<int>::min)() &&
value <= max_value<int>();
}
static auto fits_in_int(int) -> bool { return true; }
FMT_INLINE static auto fits_in_int(int) -> bool { return true; }
};

struct printf_precision_handler {
Expand Down Expand Up @@ -205,7 +205,7 @@ class printf_width_handler {
format_specs& specs_;

public:
explicit printf_width_handler(format_specs& specs) : specs_(specs) {}
FMT_INLINE explicit printf_width_handler(format_specs& specs) : specs_(specs) {}

template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value)>
auto operator()(T value) -> unsigned {
Expand Down
Loading