move-only: Detail_CheckNumFormatSpecifiers and G_TRANSLATION_FUN

This is required for a future commit. Can be reviewed via the git
options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

Also move util::detail::Hex to a proper namespace instead of an inline
namespace so it doesn't conflict with the new util::detail namespace, and
won't create other problems for callers trying to use the inline namespaces.

Also fix a misleading comment in util_string_tests.cpp.

Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>

src/test/util_string_tests.cpp

@@ -16,13 +16,13 @@ BOOST_AUTO_TEST_SUITE(util_string_tests)
 template <unsigned NumArgs>
 inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
 {
-    // This was already executed at compile-time, but is executed again at run-time to avoid -Wunused.
+    // Execute compile-time check again at run-time to get code coverage stats
-    decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt);
+    util::detail::CheckNumFormatSpecifiers<NumArgs>(fmt.fmt);
 }
 template <unsigned WrongNumArgs>
 inline void FailFmtWithError(const char* wrong_fmt, std::string_view error)
 {
-    BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason(error));
+    BOOST_CHECK_EXCEPTION(util::detail::CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*, HasReason{error});
 }
 
 BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)

src/util/strencodings.h

@@ -377,6 +377,24 @@ consteval uint8_t ConstevalHexDigit(const char c)
     throw "Only lowercase hex digits are allowed, for consistency";
 }
 
+namespace detail {
+template <size_t N>
+struct Hex {
+    std::array<std::byte, N / 2> bytes{};
+    consteval Hex(const char (&hex_str)[N])
+        // 2 hex digits required per byte + implicit null terminator
+        requires(N % 2 == 1)
+    {
+        if (hex_str[N - 1]) throw "null terminator required";
+        for (std::size_t i = 0; i < bytes.size(); ++i) {
+            bytes[i] = static_cast<std::byte>(
+                (ConstevalHexDigit(hex_str[2 * i]) << 4) |
+                 ConstevalHexDigit(hex_str[2 * i + 1]));
+        }
+    }
+};
+} // namespace detail
+
 /**
  * ""_hex is a compile-time user-defined literal returning a
  * `std::array<std::byte>`, equivalent to ParseHex(). Variants provided:
@@ -407,25 +425,6 @@ consteval uint8_t ConstevalHexDigit(const char c)
  *   time/runtime barrier.
  */
 inline namespace hex_literals {
-namespace detail {
-
-template <size_t N>
-struct Hex {
-    std::array<std::byte, N / 2> bytes{};
-    consteval Hex(const char (&hex_str)[N])
-        // 2 hex digits required per byte + implicit null terminator
-        requires(N % 2 == 1)
-    {
-        if (hex_str[N - 1]) throw "null terminator required";
-        for (std::size_t i = 0; i < bytes.size(); ++i) {
-            bytes[i] = static_cast<std::byte>(
-                (ConstevalHexDigit(hex_str[2 * i]) << 4) |
-                 ConstevalHexDigit(hex_str[2 * i + 1]));
-        }
-    }
-};
-
-} // namespace detail
 
 template <util::detail::Hex str>
 constexpr auto operator""_hex() { return str.bytes; }

src/util/string.h

@@ -17,6 +17,69 @@
 #include <vector>
 
 namespace util {
+namespace detail {
+template <unsigned num_params>
+constexpr static void CheckNumFormatSpecifiers(const char* str)
+{
+    unsigned count_normal{0}; // Number of "normal" specifiers, like %s
+    unsigned count_pos{0};    // Max number in positional specifier, like %8$s
+    for (auto it{str}; *it != '\0'; ++it) {
+        if (*it != '%' || *++it == '%') continue; // Skip escaped %%
+
+        auto add_arg = [&] {
+            unsigned maybe_num{0};
+            while ('0' <= *it && *it <= '9') {
+                maybe_num *= 10;
+                maybe_num += *it - '0';
+                ++it;
+            }
+
+            if (*it == '$') {
+                ++it;
+                // Positional specifier, like %8$s
+                if (maybe_num == 0) throw "Positional format specifier must have position of at least 1";
+                count_pos = std::max(count_pos, maybe_num);
+            } else {
+                // Non-positional specifier, like %s
+                ++count_normal;
+            }
+        };
+
+        // Increase argument count and consume positional specifier, if present.
+        add_arg();
+
+        // Consume flags.
+        while (*it == '#' || *it == '0' || *it == '-' || *it == ' ' || *it == '+') ++it;
+
+        auto parse_size = [&] {
+            if (*it == '*') {
+                ++it;
+                add_arg();
+            } else {
+                while ('0' <= *it && *it <= '9') ++it;
+            }
+        };
+
+        // Consume dynamic or static width value.
+        parse_size();
+
+        // Consume dynamic or static precision value.
+        if (*it == '.') {
+            ++it;
+            parse_size();
+        }
+
+        if (*it == '\0') throw "Format specifier incorrectly terminated by end of string";
+
+        // Length and type in "[flags][width][.precision][length]type"
+        // is not checked. Parsing continues with the next '%'.
+    }
+    if (count_normal && count_pos) throw "Format specifiers must be all positional or all non-positional!";
+    unsigned count{count_normal | count_pos};
+    if (num_params != count) throw "Format specifier count must match the argument count!";
+}
+} // namespace detail
+
 /**
  * @brief A wrapper for a compile-time partially validated format string
  *
@@ -28,66 +91,7 @@ namespace util {
 template <unsigned num_params>
 struct ConstevalFormatString {
     const char* const fmt;
-    consteval ConstevalFormatString(const char* str) : fmt{str} { Detail_CheckNumFormatSpecifiers(fmt); }
+    consteval ConstevalFormatString(const char* str) : fmt{str} { detail::CheckNumFormatSpecifiers<num_params>(fmt); }
-    constexpr static void Detail_CheckNumFormatSpecifiers(const char* str)
-    {
-        unsigned count_normal{0}; // Number of "normal" specifiers, like %s
-        unsigned count_pos{0};    // Max number in positional specifier, like %8$s
-        for (auto it{str}; *it != '\0'; ++it) {
-            if (*it != '%' || *++it == '%') continue; // Skip escaped %%
-
-            auto add_arg = [&] {
-                unsigned maybe_num{0};
-                while ('0' <= *it && *it <= '9') {
-                    maybe_num *= 10;
-                    maybe_num += *it - '0';
-                    ++it;
-                }
-
-                if (*it == '$') {
-                    ++it;
-                    // Positional specifier, like %8$s
-                    if (maybe_num == 0) throw "Positional format specifier must have position of at least 1";
-                    count_pos = std::max(count_pos, maybe_num);
-                } else {
-                    // Non-positional specifier, like %s
-                    ++count_normal;
-                }
-            };
-
-            // Increase argument count and consume positional specifier, if present.
-            add_arg();
-
-            // Consume flags.
-            while (*it == '#' || *it == '0' || *it == '-' || *it == ' ' || *it == '+') ++it;
-
-            auto parse_size = [&] {
-                if (*it == '*') {
-                    ++it;
-                    add_arg();
-                } else {
-                    while ('0' <= *it && *it <= '9') ++it;
-                }
-            };
-
-            // Consume dynamic or static width value.
-            parse_size();
-
-            // Consume dynamic or static precision value.
-            if (*it == '.') {
-                ++it;
-                parse_size();
-            }
-
-            if (*it == '\0') throw "Format specifier incorrectly terminated by end of string";
-
-            // Length and type in "[flags][width][.precision][length]type"
-            // is not checked. Parsing continues with the next '%'.
-        }
-        if (count_normal && count_pos) throw "Format specifiers must be all positional or all non-positional!";
-        unsigned count{count_normal | count_pos};
-        if (num_params != count) throw "Format specifier count must match the argument count!";
-    }
 };
 
 void ReplaceAll(std::string& in_out, const std::string& search, const std::string& substitute);

src/util/translation.h

@@ -10,6 +10,9 @@
 #include <functional>
 #include <string>
 
+/** Translate a message to the native language of the user. */
+const extern std::function<std::string(const char*)> G_TRANSLATION_FUN;
+
 /**
  * Bilingual messages:
  *   - in GUI: user's native language + untranslated (i.e. English)
@@ -64,9 +67,6 @@ bilingual_str format(const bilingual_str& fmt, const Args&... args)
 }
 } // namespace tinyformat
 
-/** Translate a message to the native language of the user. */
-const extern std::function<std::string(const char*)> G_TRANSLATION_FUN;
-
 struct ConstevalStringLiteral {
     const char* const lit;
     consteval ConstevalStringLiteral(const char* str) : lit{str} {}