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.

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

fix and improve module #3386

Merged
merged 1 commit into from
Apr 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions include/fmt/args.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class dynamic_arg_list {
}
};
} // namespace detail
FMT_BEGIN_EXPORT
Copy link
Contributor

Choose a reason for hiding this comment

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

We only have one class template here so I suggest applying FMT_MODULE_EXPORT to it.


/**
\rst
Expand Down Expand Up @@ -229,6 +230,7 @@ class dynamic_format_arg_store
}
};

FMT_END_EXPORT
FMT_END_NAMESPACE

#endif // FMT_ARGS_H_
2 changes: 1 addition & 1 deletion include/fmt/chrono.h
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,7 @@ struct chrono_format_checker : null_chrono_spec_handler<chrono_format_checker> {
FMT_CONSTEXPR void on_duration_unit() {}
};

template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value)>
template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value && has_isfinite<T>::value)>
vitaut marked this conversation as resolved.
Show resolved Hide resolved
inline bool isfinite(T) {
return true;
}
Expand Down
16 changes: 13 additions & 3 deletions include/fmt/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ FMT_CONSTEXPR inline auto is_utf8() -> bool {
compiled with a different ``-std`` option than the client code (which is not
recommended).
*/
FMT_MODULE_EXPORT
template <typename Char> class basic_string_view {
private:
const Char* data_;
Expand Down Expand Up @@ -496,9 +497,11 @@ template <typename Char> class basic_string_view {
}
};

FMT_MODULE_EXPORT
using string_view = basic_string_view<char>;

/** Specifies if ``T`` is a character type. Can be specialized by users. */
FMT_MODULE_EXPORT
template <typename T> struct is_char : std::false_type {};
template <> struct is_char<char> : std::true_type {};
vitaut marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -646,6 +649,7 @@ template <typename S> using char_t = typename detail::char_t_impl<S>::type;
You can use the ``format_parse_context`` type alias for ``char`` instead.
\endrst
*/
FMT_MODULE_EXPORT
template <typename Char> class basic_format_parse_context {
private:
basic_string_view<Char> format_str_;
Expand Down Expand Up @@ -711,6 +715,7 @@ template <typename Char> class basic_format_parse_context {
FMT_CONSTEXPR void check_dynamic_spec(int arg_id);
};

FMT_MODULE_EXPORT
using format_parse_context = basic_format_parse_context<char>;

namespace detail {
Expand Down Expand Up @@ -775,11 +780,12 @@ FMT_CONSTEXPR void basic_format_parse_context<Char>::check_dynamic_spec(
}
}

template <typename Context> class basic_format_arg;
template <typename Context> class basic_format_args;
template <typename Context> class dynamic_format_arg_store;
FMT_MODULE_EXPORT template <typename Context> class basic_format_arg;
FMT_MODULE_EXPORT template <typename Context> class basic_format_args;
FMT_MODULE_EXPORT template <typename Context> class dynamic_format_arg_store;
Comment on lines +783 to +785
Copy link
Contributor

Choose a reason for hiding this comment

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

These are just forward declarations. Do we still need to export them if the main templates are exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Names must be declared export when introduced into a namespace [module.interface]/6. The compiler will then collect all semantic properties that pertain to that entity that is referenced by the name. This collection continues throughout the whole module purview up to the end of the TU or the start of the private module fragment, whichever comes first.

In this case, the (forward) declarations introduce the names. So they need to be marked as exported at this very place in the source text.

This reminds me to remove the export declarations from the definitions of the primary templates 🤦‍♂️


// A formatter for objects of type T.
FMT_MODULE_EXPORT
template <typename T, typename Char = char, typename Enable = void>
struct formatter {
// A deleted default constructor indicates a disabled formatter.
Expand Down Expand Up @@ -1476,6 +1482,7 @@ enum { max_packed_args = 62 / packed_arg_bits };
enum : unsigned long long { is_unpacked_bit = 1ULL << 63 };
enum : unsigned long long { has_named_args_bit = 1ULL << 62 };
} // namespace detail
FMT_BEGIN_EXPORT

// An output iterator that appends to a buffer.
// It is used to reduce symbol sizes for the common case.
Expand Down Expand Up @@ -1594,6 +1601,7 @@ FMT_CONSTEXPR FMT_INLINE auto visit_format_arg(
return vis(monostate());
}

FMT_END_EXPORT
namespace detail {

template <typename Char, typename InputIt>
Expand Down Expand Up @@ -1710,6 +1718,7 @@ FMT_CONSTEXPR inline auto make_arg(T&& value) -> basic_format_arg<Context> {
return make_arg<Context>(value);
}
} // namespace detail
FMT_BEGIN_EXPORT

// Formatting context.
template <typename OutputIt, typename Char> class basic_format_context {
Expand Down Expand Up @@ -1998,6 +2007,7 @@ enum type FMT_ENUM_UNDERLYING_TYPE(unsigned char){none, minus, plus, space};
}
using sign_t = sign::type;

FMT_END_EXPORT
namespace detail {

// Workaround an array initialization issue in gcc 4.8.
Expand Down
9 changes: 7 additions & 2 deletions include/fmt/std.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ inline void write_escaped_path<std::filesystem::path::value_type>(

} // namespace detail

FMT_MODULE_EXPORT
template <typename Char>
struct formatter<std::filesystem::path, Char>
: formatter<basic_string_view<Char>> {
Expand All @@ -95,12 +96,14 @@ FMT_END_NAMESPACE
#endif

FMT_BEGIN_NAMESPACE
FMT_MODULE_EXPORT
template <typename Char>
struct formatter<std::thread::id, Char> : basic_ostream_formatter<Char> {};
FMT_END_NAMESPACE

#ifdef __cpp_lib_optional
FMT_BEGIN_NAMESPACE
FMT_MODULE_EXPORT
template <typename T, typename Char>
struct formatter<std::optional<T>, Char,
std::enable_if_t<is_formattable<T, Char>::value>> {
Expand Down Expand Up @@ -144,6 +147,7 @@ FMT_END_NAMESPACE

#ifdef __cpp_lib_variant
FMT_BEGIN_NAMESPACE
FMT_MODULE_EXPORT
template <typename Char> struct formatter<std::monostate, Char> {
template <typename ParseContext>
FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
Expand Down Expand Up @@ -192,7 +196,6 @@ auto write_variant_alternative(OutputIt out, const T& v) -> OutputIt {
}

} // namespace detail

template <typename T> struct is_variant_like {
static constexpr const bool value = detail::is_variant_like_<T>::value;
};
Expand All @@ -202,6 +205,7 @@ template <typename T, typename C> struct is_variant_formattable {
detail::is_variant_formattable_<T, C>::value;
};

FMT_MODULE_EXPORT
template <typename Variant, typename Char>
struct formatter<
Variant, Char,
Expand Down Expand Up @@ -235,7 +239,7 @@ FMT_END_NAMESPACE
#endif // __cpp_lib_variant

FMT_BEGIN_NAMESPACE

FMT_MODULE_EXPORT
template <typename Char> struct formatter<std::error_code, Char> {
template <typename ParseContext>
FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
Expand All @@ -253,6 +257,7 @@ template <typename Char> struct formatter<std::error_code, Char> {
}
};

FMT_MODULE_EXPORT
template <typename T, typename Char>
struct formatter<
T, Char,
Expand Down
4 changes: 2 additions & 2 deletions include/fmt/xchar.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ inline auto vformat_to(
auto&& buf = detail::get_buffer<Char>(out);
vformat_to(buf, detail::to_string_view(format_str), args,
detail::locale_ref(loc));
return detail::get_iterator(buf);
return detail::get_iterator(buf, out);
}

template <
Expand All @@ -181,7 +181,7 @@ template <
inline auto format_to(OutputIt out, const Locale& loc, const S& format_str,
Args&&... args) ->
typename std::enable_if<enable, OutputIt>::type {
return vformat_to(out, loc, to_string_view(format_str),
return vformat_to(out, loc, detail::to_string_view(format_str),
fmt::make_format_args<buffer_context<Char>>(args...));
}

Expand Down
7 changes: 7 additions & 0 deletions src/fmt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,25 @@ module;
#include <cstring>
#include <ctime>
#include <exception>
#include <filesystem>
#include <fstream>
#include <functional>
#include <iterator>
#include <limits>
#include <locale>
#include <memory>
#include <optional>
#include <ostream>
#include <stdexcept>
#include <string>
#include <string_view>
#include <system_error>
#include <thread>
#include <type_traits>
#include <utility>
#include <variant>
#include <vector>
#include <version>

#if _MSC_VER
# include <intrin.h>
Expand Down Expand Up @@ -75,6 +81,7 @@ export module fmt;
#include "fmt/os.h"
#include "fmt/printf.h"
#include "fmt/xchar.h"
#include "fmt/std.h"

// gcc doesn't yet implement private module fragments
#if !FMT_GCC_VERSION
Expand Down