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

Add UDL as replacement for FMT_COMPILE #2043

Merged
merged 6 commits into from
Dec 7, 2020
Merged

Add UDL as replacement for FMT_COMPILE #2043

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
506230b
add _cf literal as replacement for FMT_COMPILE
alexezeder Dec 1, 2020
8c8b1cc
apply requested changes
alexezeder Dec 2, 2020
855e488
add empty format string into CompileTest::CompileFormatStringLiteral
alexezeder Dec 2, 2020
6ea2ca8
fix for empty format string
alexezeder Dec 2, 2020
b81bf00
update FMT_USE_NONTYPE_TEMPLATE_PARAMETERS
alexezeder Dec 7, 2020
e7d035d
remove deduction guide for `fixed_string`
alexezeder Dec 7, 2020
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
38 changes: 38 additions & 0 deletions include/fmt/compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@

#include "format.h"

#if !defined(FMT_USE_NONTYPE_TEMPLATE_PARAMETERS) && \
defined(__cpp_nontype_template_parameter_class) && \
(!FMT_GCC_VERSION || FMT_GCC_VERSION >= 903)
# define FMT_USE_NONTYPE_TEMPLATE_PARAMETERS
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make FMT_USE_NONTYPE_TEMPLATE_PARAMETERS defined to 1/0 for consistency with other similar macros and to make it possible to disable the feature. See e.g.

fmt/include/fmt/core.h

Lines 178 to 185 in 33f9a6d

#ifndef FMT_USE_INLINE_NAMESPACES
# if FMT_HAS_FEATURE(cxx_inline_namespaces) || FMT_GCC_VERSION >= 404 || \
(FMT_MSC_VER >= 1900 && !_MANAGED)
# define FMT_USE_INLINE_NAMESPACES 1
# else
# define FMT_USE_INLINE_NAMESPACES 0
# endif
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


FMT_BEGIN_NAMESPACE
namespace detail {

Expand All @@ -37,6 +43,27 @@ struct is_compiled_string : std::is_base_of<compiled_string, S> {};
*/
#define FMT_COMPILE(s) FMT_STRING_IMPL(s, fmt::detail::compiled_string)

#ifdef FMT_USE_NONTYPE_TEMPLATE_PARAMETERS
template <typename Char, size_t N> struct fixed_string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is fixed_string actually needed? Can we directly use an array instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like yes, it's actually needed. The raw C-style array should be wrapped in some type, and this wrapper-type should also have a constructor with const Char (&str)[N] as an argument. Of course, I can be wrong here, but all my attempts ended up with nothing working.

But I realized that the deduction guide for fixed_string can be removed now because it holds a null-terminated string.

constexpr fixed_string(const Char (&str)[N]) {
copy_str<Char, const Char*, Char*>(static_cast<const Char*>(str), str + N,
data);
}
Char data[N]{};
Copy link
Contributor

Choose a reason for hiding this comment

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

{} seems unnecessary, please remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's either {} here or : data{} initialization in constructor for GCC 9.3, otherwise:

<source>:8:13: error: member 'fmt::v7::detail::fixed_string<char, 2>::data' must be initialized by mem-initializer in 'constexpr' constructor
<source>:12:8: note: declared here
   12 |   Char data[N];
      |        ^~~~

};

template <typename Char, size_t N>
fixed_string(const Char (&str)[N]) -> fixed_string<Char, N>;

template <typename Char, size_t N, fixed_string<Char, N> Str>
struct udl_compiled_string : compiled_string {
using char_type = Char;
constexpr operator basic_string_view<char_type>() const {
return {Str.data, N - 1};
}
};
#endif

template <typename T, typename... Tail>
const T& first(const T& value, const Tail&...) {
return value;
Expand Down Expand Up @@ -698,6 +725,17 @@ size_t formatted_size(const CompiledFormat& cf, const Args&... args) {
return format_to(detail::counting_iterator(), cf, args...).count();
}

#ifdef FMT_USE_NONTYPE_TEMPLATE_PARAMETERS
inline namespace literals {
template <detail::fixed_string Str>
constexpr detail::udl_compiled_string<remove_cvref_t<decltype(Str.data[0])>,
sizeof(Str.data), Str>
operator""_cf() {
return {};
}
} // namespace literals
#endif

FMT_END_NAMESPACE

#endif // FMT_COMPILE_H_
8 changes: 8 additions & 0 deletions test/compile-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ TEST(CompileTest, Empty) {
}
#endif

#ifdef FMT_USE_NONTYPE_TEMPLATE_PARAMETERS
TEST(CompileTest, CompileFormatStringLiteral) {
using namespace fmt::literals;
EXPECT_EQ("", fmt::format(""_cf));
EXPECT_EQ("42", fmt::format("{}"_cf, 42));
}
#endif

#if __cplusplus >= 202002L
template <size_t max_string_length> struct test_string {
template <typename T> constexpr bool operator==(const T& rhs) const noexcept {
Expand Down