Merge bitcoin/bitcoin#30933: test: Prove+document ConstevalFormatString/tinyformat parity

c93bf0e test: Add missing %c character test (Hodlinator)
76cca4a test: Document non-parity between tinyformat and ConstevalFormatstring (Hodlinator)
533013c test: Prove+document ConstevalFormatString/tinyformat parity (Hodlinator)
b81a465 refactor test: Profit from using namespace + using detail function (Hodlinator)

Pull request description:

  Clarifies and puts the extent of parity under test.

  Broken out from #30546 based on bitcoin/bitcoin#30546 (comment) and bitcoin/bitcoin#30546 (comment).

ACKs for top commit:
  maflcko:
    re-ACK c93bf0e 🗜
  l0rinc:
    ACK c93bf0e
  ryanofsky:
    Code review ACK c93bf0e. Just a few cleanups tweaking function declarations and commit comments and consolidating some test cases since last review.

Tree-SHA512: 5ecc893b26cf2761c0009861be392ec4c4fceb0ef95052a2f6f9df76b2e459cfb3f9e257f61be07c3bb2ecc6e525e72c5ca853be1f63b70b52785323d3db6b42

Author: ryanofsky

src/test/util_string_tests.cpp

@@ -8,28 +8,44 @@
 #include <test/util/setup_common.h>
 
 using namespace util;
+using util::detail::CheckNumFormatSpecifiers;
 
 BOOST_AUTO_TEST_SUITE(util_string_tests)
 
+template <unsigned NumArgs>
+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>
-inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
+void PassFmt(ConstevalFormatString<NumArgs> fmt)
 {
     // Execute compile-time check again at run-time to get code coverage stats
-    util::detail::CheckNumFormatSpecifiers<NumArgs>(fmt.fmt);
+    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>
-inline void FailFmtWithError(const char* wrong_fmt, std::string_view error)
+void FailFmtWithError(const char* wrong_fmt, std::string_view error)
 {
-    BOOST_CHECK_EXCEPTION(util::detail::CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*, HasReason{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");
@@ -110,6 +126,24 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
     FailFmtWithError<2>("%1$*2$", err_term);
     FailFmtWithError<2>("%1$.*2$", err_term);
     FailFmtWithError<2>("%1$9.*2$", err_term);
+
+    // Non-parity between tinyformat and ConstevalFormatString.
+    // tinyformat throws but ConstevalFormatString does not.
+    BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<1>{"%n"}, 0), tfm::format_error,
+        HasReason{"tinyformat: %n conversion spec not supported"});
+    BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi"), tfm::format_error,
+        HasReason{"tinyformat: Cannot convert from argument type to integer for use as variable width or precision"});
+    BOOST_CHECK_EXCEPTION(tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi"), tfm::format_error,
+        HasReason{"tinyformat: Cannot convert from argument type to integer for use as variable width or precision"});
+
+    // Ensure that tinyformat throws if format string contains wrong number
+    // of specifiers. PassFmt relies on this to verify tinyformat successfully
+    // formats the strings, and will need to be updated if tinyformat is changed
+    // not to throw on failure.
+    BOOST_CHECK_EXCEPTION(TfmFormatZeroes<2>("%s"), tfm::format_error,
+        HasReason{"tinyformat: Not enough conversion specifiers in format string"});
+    BOOST_CHECK_EXCEPTION(TfmFormatZeroes<1>("%s %s"), tfm::format_error,
+        HasReason{"tinyformat: Too many conversion specifiers in format string"});
 }
 
 BOOST_AUTO_TEST_SUITE_END()

test/lint/run-lint-format-strings.py

@@ -15,6 +15,8 @@
 FALSE_POSITIVES = [
     ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS), COPYRIGHT_HOLDERS_SUBSTITUTION)"),
     ("src/test/translation_tests.cpp", "strprintf(format, arg)"),
+    ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%*s"}, "hi", "hi")'),
+    ("src/test/util_string_tests.cpp", 'tfm::format(ConstevalFormatString<2>{"%.*s"}, "hi", "hi")'),
 ]