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

Code before:
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <util/string.h>

#include <boost/test/unit_test.hpp>

using namespace util;

BOOST_AUTO_TEST_SUITE(util_string_tests)

// Helper to allow compile-time sanity checks while providing the number of
// args directly. Normally PassFmt<sizeof...(Args)> would be used.
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.
    decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt);
}
template <unsigned WrongNumArgs>
inline void FailFmtWithError(std::string_view wrong_fmt, std::string_view error)
{
    using ErrType = const char*;
    auto check_throw{[error](const ErrType& str) { return str == error; }};
    BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(wrong_fmt), ErrType, check_throw);
}

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>("%#x");

Review comment (bug):
Unless `HasReason` gets a new ctor that takes non-ref `string` which we can `std::move` into (and it can `move` from internally into the member), we are now allocating extra `string` copies on every `FailFmtWithError`-call.

I prefer going back to `string_view` here.

Yet another possible alternative is to construct `HasReason` copies outside and pass them in. Nice to memory ~bad~but slightly ugly using `HasReason` outside of the Boost macro.
```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 4f9aa197f9..5850102152 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -20,9 +20,9 @@ inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
     decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt);
 }
 template <unsigned WrongNumArgs>
-inline void FailFmtWithError(std::string_view wrong_fmt, std::string error)
+inline void FailFmtWithError(std::string_view wrong_fmt, const HasReason& error)
 {
-    BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason(error));
+    BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(wrong_fmt), const char*, error);
 }
 
 BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
@@ -61,23 +61,23 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
     PassFmt<2>("%2$*3$d");
     PassFmt<1>("%.*f");
 
-    auto err_mix{"Format specifiers must be all positional or all non-positional!"};
+    HasReason err_mix{"Format specifiers must be all positional or all non-positional!"};
     FailFmtWithError<1>("%s%1$s", err_mix);
 
-    auto err_num{"Format specifier count must match the argument count!"};
+    HasReason err_num{"Format specifier count must match the argument count!"};
     FailFmtWithError<1>("", err_num);
     FailFmtWithError<0>("%s", err_num);
     FailFmtWithError<2>("%s", err_num);
     FailFmtWithError<0>("%1$s", err_num);
     FailFmtWithError<2>("%1$s", err_num);
 
-    auto err_0_pos{"Positional format specifier must have position of at least 1"};
+    HasReason err_0_pos{"Positional format specifier must have position of at least 1"};
     FailFmtWithError<1>("%$s", err_0_pos);
     FailFmtWithError<1>("%$", err_0_pos);
     FailFmtWithError<0>("%0$", err_0_pos);
     FailFmtWithError<0>("%0$s", err_0_pos);
 
-    auto err_term{"Format specifier incorrectly terminated by end of string"};
+    HasReason err_term{"Format specifier incorrectly terminated by end of string"};
     FailFmtWithError<1>("%", err_term);
     FailFmtWithError<1>("%1$", err_term);
 }
 ```

Code after:
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <util/string.h>

#include <boost/test/unit_test.hpp>
#include <test/util/setup_common.h>

using namespace util;

BOOST_AUTO_TEST_SUITE(util_string_tests)

// Helper to allow compile-time sanity checks while providing the number of
// args directly. Normally PassFmt<sizeof...(Args)> would be used.
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.
    decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt);
}
template <unsigned WrongNumArgs>
inline void FailFmtWithError(std::string_view wrong_fmt, std::string_view error)
{
    BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(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>("%#x");
    PassFmt<1>("%1$5i");