Fix fmtlib#2818: diagnose unformattable arguments in unpacked case

timsong-cpp · Mar 18, 2022 · 47d5762 · 47d5762
1 parent acd2ceb
commit 47d5762
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 16 deletions.
diff --git a/include/fmt/core.h b/include/fmt/core.h
@@ -1260,7 +1260,7 @@ template <typename Context> class value {
 };
 
 template <typename Context, typename T>
-FMT_CONSTEXPR auto make_arg(const T& value) -> basic_format_arg<Context>;
+FMT_CONSTEXPR auto make_arg(T&& value) -> basic_format_arg<Context>;
 
 // To minimize the number of types we need to deal with, long is translated
 // either to int or to long long depending on its size.
@@ -1513,7 +1513,7 @@ template <typename Context> class basic_format_arg {
   detail::type type_;
 
   template <typename ContextType, typename T>
-  friend FMT_CONSTEXPR auto detail::make_arg(const T& value)
+  friend FMT_CONSTEXPR auto detail::make_arg(T&& value)
       -> basic_format_arg<ContextType>;
 
   template <typename Visitor, typename Ctx>
@@ -1674,19 +1674,7 @@ constexpr auto encode_types() -> unsigned long long {
 }
 
 template <typename Context, typename T>
-FMT_CONSTEXPR auto make_arg(const T& value) -> basic_format_arg<Context> {
-  basic_format_arg<Context> arg;
-  arg.type_ = mapped_type_constant<T, Context>::value;
-  arg.value_ = arg_mapper<Context>().map(value);
-  return arg;
-}
-
-// The type template parameter is there to avoid an ODR violation when using
-// a fallback formatter in one translation unit and an implicit conversion in
-// another (not recommended).
-template <bool IS_PACKED, typename Context, type, typename T,
-          FMT_ENABLE_IF(IS_PACKED)>
-FMT_CONSTEXPR FMT_INLINE auto make_arg(T&& val) -> value<Context> {
+FMT_CONSTEXPR FMT_INLINE auto make_value(T&& val) -> value<Context> {
   const auto& arg = arg_mapper<Context>().map(std::forward<T>(val));
 
   constexpr bool formattable_char =
@@ -1715,9 +1703,26 @@ FMT_CONSTEXPR FMT_INLINE auto make_arg(T&& val) -> value<Context> {
   return {arg};
 }
 
+template <typename Context, typename T>
+FMT_CONSTEXPR auto make_arg(T&& value) -> basic_format_arg<Context> {
+  basic_format_arg<Context> arg;
+  arg.type_ = mapped_type_constant<T, Context>::value;
+  arg.value_ = make_value<Context>(value);
+  return arg;
+}
+
+// The type template parameter is there to avoid an ODR violation when using
+// a fallback formatter in one translation unit and an implicit conversion in
+// another (not recommended).
+template <bool IS_PACKED, typename Context, type, typename T,
+          FMT_ENABLE_IF(IS_PACKED)>
+FMT_CONSTEXPR FMT_INLINE auto make_arg(T&& val) -> value<Context> {
+  return make_value<Context>(val);
+}
+
 template <bool IS_PACKED, typename Context, type, typename T,
           FMT_ENABLE_IF(!IS_PACKED)>
-inline auto make_arg(const T& value) -> basic_format_arg<Context> {
+FMT_CONSTEXPR inline auto make_arg(T&& value) -> basic_format_arg<Context> {
   return make_arg<Context>(value);
 }
 FMT_END_DETAIL_NAMESPACE

diff --git a/test/compile-error-test/CMakeLists.txt b/test/compile-error-test/CMakeLists.txt
@@ -6,6 +6,8 @@ project(compile-error-test CXX)
 set(fmt_headers "
   #include <fmt/format.h>
   #include <fmt/xchar.h>
+  #include <fmt/ostream.h>
+  #include <iostream>
 ")
 
 set(error_test_names "")
@@ -154,6 +156,16 @@ expect_compile(format-function-error "
   fmt::format(\"{}\", f);
 " ERROR)
 
+# Formatting an unformattable argument should always be a compile time error
+expect_compile(format-lots-of-arguments-with-unformattable "
+  struct E {};
+  fmt::format(\"\", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, E());
+" ERROR)
+expect_compile(format-lots-of-arguments-with-function "
+  void (*f)();
+  fmt::format(\"\", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, f);
+" ERROR)
+
 # Make sure that compiler features detected in the header
 # match the features detected in CMake.
 if (SUPPORTS_USER_DEFINED_LITERALS)
@@ -181,6 +193,20 @@ if (CMAKE_CXX_STANDARD GREATER_EQUAL 20)
       #error
     #endif
   " ERROR)
+  expect_compile(print-string-number-spec-error "
+    #ifdef FMT_HAS_CONSTEVAL
+      fmt::print(\"{:d}\", \"I am not a number\");
+    #else
+      #error
+    #endif
+  " ERROR)
+  expect_compile(print-stream-string-number-spec-error "
+  #ifdef FMT_HAS_CONSTEVAL
+    fmt::print(std::cout, \"{:d}\", \"I am not a number\");
+  #else
+    #error
+  #endif
+  " ERROR)
 
   # Compile-time argument name check
   expect_compile(format-string-name "