Repository: bitcoin/bitcoin
File: src/test/util_string_tests.cpp (C++)

Code before:
template <unsigned NumArgs>
inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
{
    // Execute compile-time check again at run-time to get code coverage stats
    util::detail::CheckNumFormatSpecifiers<NumArgs>(fmt.fmt);
}
template <unsigned WrongNumArgs>
inline void FailFmtWithError(const char* wrong_fmt, std::string_view error)
{
    BOOST_CHECK_EXCEPTION(util::detail::CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*, HasReason{error});
}

BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
{
    PassFmt<0>("");
    PassFmt<0>("%%");
    PassFmt<1>("%s");
    PassFmt<0>("%%s");
    PassFmt<0>("s%%");
    PassFmt<1>("%%%s");
    PassFmt<1>("%s%%");
    PassFmt<0>(" 1$s");
    PassFmt<1>("%1$s");
    PassFmt<1>("%1$s%1$s");
    PassFmt<2>("%2$s");
    PassFmt<2>("%2$s 4$s %2$s");
    PassFmt<129>("%129$s 999$s %2$s");
    PassFmt<1>("%02d");
    PassFmt<1>("%+2s");
    PassFmt<1>("%.6i");
    PassFmt<1>("%5.2f");
    PassFmt<1>("%5.f");
    PassFmt<1>("%.f");
    PassFmt<1>("%#x");
    PassFmt<1>("%1$5i");
    PassFmt<1>("%1$-5i");
    PassFmt<1>("%1$.5i");
    // tinyformat accepts almost any "type" spec, even '%', or '_', or '\n'.
    PassFmt<1>("%123%");
    PassFmt<1>("%123%s");
    PassFmt<1>("%_");
    PassFmt<1>("%\n");

    PassFmt<2>("%*c");
    PassFmt<2>("%+*c");
    PassFmt<2>("%.*f");
    PassFmt<3>("%*.*f");
    PassFmt<3>("%2$*3$d");
    PassFmt<3>("%2$*3$.9d");
    PassFmt<3>("%2$.*3$d");
    PassFmt<3>("%2$9.*3$d");

Review comment (bug):
In commit "test: Prove+document ConstevalFormatString/tinyformat parity" (a911efc6e868ab1a23d05ee8ab3e94f7ecb89e2b)

The name `TfmF` doesn't have any clear meaning to me, and usage of the function is also complicated by the need to use tuple_cat everywhere. I think the code would be clearer if the function were less generic and did not require passing a tuple. Maybe would suggest:

<details><summary>diff</summary>
<p>

```diff
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -10,12 +10,12 @@ using namespace util;
 
 BOOST_AUTO_TEST_SUITE(util_string_tests)
 
-template <typename... Tt>
-void TfmF(const char* fmt, const std::tuple<Tt...>& t)
+template <unsigned NumArgs>
+std::string FormatZeroes(const char* fmt)
 {
-    std::apply([fmt](const Tt&... ta){
-        tfm::format(fmt, ta...);
-    }, t);
+    return std::apply([fmt](auto... args) {
+        return tfm::format(fmt, args...);
+    }, std::tuple_cat(std::array<int, NumArgs>{}));
 }
 
 // Helper to allow compile-time sanity checks while providing the number of
@@ -27,9 +27,11 @@ inline void PassFmt(ConstevalFormatString<NumArgs> fmt)
     decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt);
 
     // Prove parity with tinyformat
-    TfmF(fmt.fmt, std::tuple_cat(std::array<int, NumArgs>{}));
+    FormatZeroes<NumArgs>(fmt.fmt);
+
+    // Make sure tinyformat would throw if an argument is missing
     if constexpr (NumArgs > 0) {
-        BOOST_CHECK_THROW(TfmF(fmt.fmt, std::tuple_cat(std::array<int, NumArgs - 1>{})), tfm::format_error);
+        BOOST_CHECK_THROW(FormatZeroes<NumArgs - 1>(fmt.fmt), tfm::format_error);
     }
 }
 template <unsigned WrongNumArgs, unsigned CorrectArgs>
@@ -37,8 +39,8 @@ inline void PassFmtIncorrect(ConstevalFormatString<WrongNumArgs> fmt)
 {
     // Disprove parity with tinyformat
     static_assert(WrongNumArgs != CorrectArgs);
-    TfmF(fmt.fmt, std::tuple_cat(std::array<int, CorrectArgs>{}));
-    BOOST_CHECK_THROW(TfmF(fmt.fmt, std::tuple_cat(std::array<int, WrongNumArgs>{})), tfm::format_error);
+    FormatZeroes<CorrectArgs>(fmt.fmt);
+    BOOST_CHECK_THROW(FormatZeroes<WrongNumArgs>(fmt.fmt), tfm::format_error);
 }
 template <unsigned WrongNumArgs>
 inline void FailFmtWithError(std::string_view wrong_fmt, std::string_view error)
```
</p>
</details>

Code after:
void TfmFormatZeroes(const std::string& fmt)
{
    std::apply([&](auto... args) {
        (void)tfm::format(fmt, args...);
    }, std::array<int, NumArgs>{});
}

// Helper to allow compile-time sanity checks while providing the number of
// args directly. Normally PassFmt<sizeof...(Args)> would be used.
template <unsigned NumArgs>
void PassFmt(ConstevalFormatString<NumArgs> fmt)
{
    // Execute compile-time check again at run-time to get code coverage stats
    BOOST_CHECK_NO_THROW(CheckNumFormatSpecifiers<NumArgs>(fmt.fmt));

    // If ConstevalFormatString didn't throw above, make sure tinyformat doesn't
    // throw either for the same format string and parameter count combination.
    // Proves that we have some extent of protection from runtime errors
    // (tinyformat may still throw for some type mismatches).
    BOOST_CHECK_NO_THROW(TfmFormatZeroes<NumArgs>(fmt.fmt));
}
template <unsigned WrongNumArgs>
void FailFmtWithError(const char* wrong_fmt, std::string_view error)
{
    BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*, HasReason{error});
}

BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
{
    PassFmt<0>("");
    PassFmt<0>("%%");
    PassFmt<1>("%s");
    PassFmt<1>("%c");
    PassFmt<0>("%%s");
    PassFmt<0>("s%%");
    PassFmt<1>("%%%s");
    PassFmt<1>("%s%%");
    PassFmt<0>(" 1$s");
    PassFmt<1>("%1$s");
    PassFmt<1>("%1$s%1$s");
    PassFmt<2>("%2$s");
    PassFmt<2>("%2$s 4$s %2$s");
    PassFmt<129>("%129$s 999$s %2$s");
    PassFmt<1>("%02d");
    PassFmt<1>("%+2s");
    PassFmt<1>("%.6i");
    PassFmt<1>("%5.2f");
    PassFmt<1>("%5.f");
    PassFmt<1>("%.f");
    PassFmt<1>("%#x");
    PassFmt<1>("%1$5i");