Refactor format string checks

include/fmt/core.h

@@ -619,18 +619,30 @@ FMT_TYPE_CONSTANT(const void*, pointer_type);
 constexpr bool is_integral_type(type t) {
   return t > type::none_type && t <= type::last_integer_type;
 }
-
 constexpr bool is_arithmetic_type(type t) {
   return t > type::none_type && t <= type::last_numeric_type;
 }
 
-constexpr auto has_sign(type t) -> bool {
-  return ((0xe2a >> static_cast<int>(t)) & 1) != 0;
+constexpr auto set(type rhs) -> int { return 1 << static_cast<int>(rhs); }
+constexpr auto in(type t, int set) -> bool {
+  return ((set >> static_cast<int>(t)) & 1) != 0;
 }
 
-constexpr auto has_precision(type t) -> bool {
-  return ((0x3e00 >> static_cast<int>(t)) & 1) != 0;
-}
+// Bitsets of types.
+enum {
+  sint_set =
+      set(type::int_type) | set(type::long_long_type) | set(type::int128_type),
+  uint_set = set(type::uint_type) | set(type::ulong_long_type) |
+             set(type::uint128_type),
+  all_int_set = sint_set | uint_set,
+  bool_set = set(type::bool_type),
+  char_set = set(type::char_type),
+  float_set = set(type::float_type) | set(type::double_type) |
+              set(type::long_double_type),
+  string_set = set(type::string_type),
+  cstring_set = set(type::cstring_type),
+  pointer_set = set(type::pointer_type)
+};
 
 FMT_NORETURN FMT_API void throw_format_error(const char* message);
 
@@ -2399,51 +2411,60 @@ FMT_CONSTEXPR auto parse_precision(const Char* begin, const Char* end,
   return parse_dynamic_spec(begin, end, value, ref, ctx);
 }
 
-FMT_CONSTEXPR inline auto parse_presentation_type(char type)
+FMT_CONSTEXPR inline auto do_parse_presentation_type(char c, type t)
     -> presentation_type {
-  switch (type) {
+  using pt = presentation_type;
+  constexpr auto integral_set = all_int_set | bool_set | char_set;
+  switch (c) {
   case 'd':
-    return presentation_type::dec;
+    return in(t, integral_set) ? pt::dec : pt::none;
   case 'o':
-    return presentation_type::oct;
+    return in(t, integral_set) ? pt::oct : pt::none;
   case 'x':
-    return presentation_type::hex_lower;
+    return in(t, integral_set) ? pt::hex_lower : pt::none;
   case 'X':
-    return presentation_type::hex_upper;
+    return in(t, integral_set) ? pt::hex_upper : pt::none;
   case 'b':
-    return presentation_type::bin_lower;
+    return in(t, integral_set) ? pt::bin_lower : pt::none;
   case 'B':
-    return presentation_type::bin_upper;
+    return in(t, integral_set) ? pt::bin_upper : pt::none;
   case 'a':
-    return presentation_type::hexfloat_lower;
+    return in(t, float_set) ? pt::hexfloat_lower : pt::none;
   case 'A':
-    return presentation_type::hexfloat_upper;
+    return in(t, float_set) ? pt::hexfloat_upper : pt::none;
   case 'e':
-    return presentation_type::exp_lower;
+    return in(t, float_set) ? pt::exp_lower : pt::none;
   case 'E':
-    return presentation_type::exp_upper;
+    return in(t, float_set) ? pt::exp_upper : pt::none;
   case 'f':
-    return presentation_type::fixed_lower;
+    return in(t, float_set) ? pt::fixed_lower : pt::none;
   case 'F':
-    return presentation_type::fixed_upper;
+    return in(t, float_set) ? pt::fixed_upper : pt::none;
   case 'g':
-    return presentation_type::general_lower;
+    return in(t, float_set) ? pt::general_lower : pt::none;
   case 'G':
-    return presentation_type::general_upper;
+    return in(t, float_set) ? pt::general_upper : pt::none;
   case 'c':
-    return presentation_type::chr;
+    return in(t, integral_set) ? pt::chr : pt::none;
   case 's':
-    return presentation_type::string;
+    return in(t, bool_set | string_set | cstring_set) ? pt::string : pt::none;
   case 'p':
-    return presentation_type::pointer;
+    return in(t, pointer_set | cstring_set) ? pt::pointer : pt::none;
   case '?':
-    return presentation_type::debug;
+    return in(t, char_set | string_set | cstring_set) ? pt::debug : pt::none;
   default:
-    throw_format_error("invalid format specifier");
-    return presentation_type::none;
+    return pt::none;
   }
 }
 
+FMT_CONSTEXPR inline auto parse_presentation_type(char c, type t)
+    -> presentation_type {
+  auto pt = do_parse_presentation_type(c, t);
+  if (pt == presentation_type::none)
+    throw_format_error("invalid format specifier");
+  return pt;
+}
+
 // Parses standard format specifiers.
 template <typename Char>
 FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
@@ -2451,7 +2472,7 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
     basic_format_parse_context<Char>& ctx, type arg_type) -> const Char* {
   if (1 < end - begin && begin[1] == '}' && is_ascii_letter(*begin) &&
       *begin != 'L') {
-    specs.type = parse_presentation_type(to_ascii(*begin++));
+    specs.type = parse_presentation_type(to_ascii(*begin++), arg_type);
     return begin;
   }
   if (begin == end) return begin;
@@ -2459,7 +2480,7 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
   begin = parse_align(begin, end, specs);
   if (begin == end) return begin;
 
-  if (has_sign(arg_type)) {
+  if (in(arg_type, sint_set | float_set)) {
     switch (to_ascii(*begin)) {
     case '+':
       specs.sign = sign::plus;
@@ -2498,7 +2519,7 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
   begin = parse_dynamic_spec(begin, end, specs.width, specs.width_ref, ctx);
   if (begin == end) return begin;
 
-  if (*begin == '.' && has_precision(arg_type)) {
+  if (*begin == '.' && in(arg_type, float_set | string_set | cstring_set)) {
     begin =
         parse_precision(begin, end, specs.precision, specs.precision_ref, ctx);
     if (begin == end) return begin;
@@ -2509,7 +2530,8 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
     if (++begin == end) return begin;
   }
 
-  if (*begin != '}') specs.type = parse_presentation_type(to_ascii(*begin++));
+  if (*begin != '}')
+    specs.type = parse_presentation_type(to_ascii(*begin++), arg_type);
   return begin;
 }
 
@@ -2626,116 +2648,19 @@ FMT_CONSTEXPR auto parse_format_specs(ParseContext& ctx)
   return f.parse(ctx);
 }
 
-template <typename ErrorHandler>
-FMT_CONSTEXPR void check_int_type_spec(presentation_type type,
-                                       ErrorHandler&& eh) {
-  if (type > presentation_type::bin_upper && type != presentation_type::chr)
-    eh.on_error("invalid format specifier");
-}
-
 // Checks char specs and returns true if the type spec is char (and not int).
-template <typename Char, typename ErrorHandler = error_handler>
-FMT_CONSTEXPR auto check_char_specs(const format_specs<Char>& specs,
-                                    ErrorHandler&& eh = {}) -> bool {
+template <typename Char>
+FMT_CONSTEXPR auto check_char_specs(const format_specs<Char>& specs) -> bool {
   if (specs.type != presentation_type::none &&
       specs.type != presentation_type::chr &&
       specs.type != presentation_type::debug) {
-    check_int_type_spec(specs.type, eh);
     return false;
   }
   if (specs.align == align::numeric || specs.sign != sign::none || specs.alt)
-    eh.on_error("invalid format specifier for char");
+    throw_format_error("invalid format specifier for char");
   return true;
 }
 
-// A floating-point presentation format.
-enum class float_format : unsigned char {
-  general,  // General: exponent notation or fixed point based on magnitude.
-  exp,      // Exponent notation with the default precision of 6, e.g. 1.2e-3.
-  fixed,    // Fixed point with the default precision of 6, e.g. 0.0012.
-  hex
-};
-
-struct float_specs {
-  int precision;
-  float_format format : 8;
-  sign_t sign : 8;
-  bool upper : 1;
-  bool locale : 1;
-  bool binary32 : 1;
-  bool showpoint : 1;
-};
-
-template <typename ErrorHandler = error_handler, typename Char>
-FMT_CONSTEXPR auto parse_float_type_spec(const format_specs<Char>& specs,
-                                         ErrorHandler&& eh = {})
-    -> float_specs {
-  auto result = float_specs();
-  result.showpoint = specs.alt;
-  result.locale = specs.localized;
-  switch (specs.type) {
-  case presentation_type::none:
-    result.format = float_format::general;
-    break;
-  case presentation_type::general_upper:
-    result.upper = true;
-    FMT_FALLTHROUGH;
-  case presentation_type::general_lower:
-    result.format = float_format::general;
-    break;
-  case presentation_type::exp_upper:
-    result.upper = true;
-    FMT_FALLTHROUGH;
-  case presentation_type::exp_lower:
-    result.format = float_format::exp;
-    result.showpoint |= specs.precision != 0;
-    break;
-  case presentation_type::fixed_upper:
-    result.upper = true;
-    FMT_FALLTHROUGH;
-  case presentation_type::fixed_lower:
-    result.format = float_format::fixed;
-    result.showpoint |= specs.precision != 0;
-    break;
-  case presentation_type::hexfloat_upper:
-    result.upper = true;
-    FMT_FALLTHROUGH;
-  case presentation_type::hexfloat_lower:
-    result.format = float_format::hex;
-    break;
-  default:
-    eh.on_error("invalid format specifier");
-    break;
-  }
-  return result;
-}
-
-template <typename ErrorHandler = error_handler>
-FMT_CONSTEXPR auto check_cstring_type_spec(presentation_type type,
-                                           ErrorHandler&& eh = {}) -> bool {
-  if (type == presentation_type::none || type == presentation_type::string ||
-      type == presentation_type::debug)
-    return true;
-  if (type != presentation_type::pointer)
-    eh.on_error("invalid format specifier");
-  return false;
-}
-
-template <typename ErrorHandler = error_handler>
-FMT_CONSTEXPR void check_string_type_spec(presentation_type type,
-                                          ErrorHandler&& eh = {}) {
-  if (type != presentation_type::none && type != presentation_type::string &&
-      type != presentation_type::debug)
-    eh.on_error("invalid format specifier");
-}
-
-template <typename ErrorHandler>
-FMT_CONSTEXPR void check_pointer_type_spec(presentation_type type,
-                                           ErrorHandler&& eh) {
-  if (type != presentation_type::none && type != presentation_type::pointer)
-    eh.on_error("invalid format specifier");
-}
-
 constexpr int invalid_arg_index = -1;
 
 #if FMT_USE_NONTYPE_TEMPLATE_ARGS
@@ -2863,58 +2788,7 @@ struct formatter<T, Char,
     auto type = detail::type_constant<T, Char>::value;
     auto end =
         detail::parse_format_specs(ctx.begin(), ctx.end(), specs_, ctx, type);
-    auto eh = detail::error_handler();
-    switch (type) {
-    case detail::type::none_type:
-      FMT_FALLTHROUGH;
-    case detail::type::custom_type:
-      FMT_ASSERT(false, "invalid argument type");
-      break;
-    case detail::type::bool_type:
-      if (specs_.type == presentation_type::none ||
-          specs_.type == presentation_type::string) {
-        break;
-      }
-      FMT_FALLTHROUGH;
-    case detail::type::int_type:
-    case detail::type::uint_type:
-    case detail::type::long_long_type:
-    case detail::type::ulong_long_type:
-    case detail::type::int128_type:
-    case detail::type::uint128_type:
-      detail::check_int_type_spec(specs_.type, eh);
-      break;
-    case detail::type::char_type:
-      detail::check_char_specs(specs_, eh);
-      break;
-    case detail::type::float_type:
-      if (detail::const_check(FMT_USE_FLOAT))
-        detail::parse_float_type_spec(specs_, eh);
-      else
-        FMT_ASSERT(false, "float support disabled");
-      break;
-    case detail::type::double_type:
-      if (detail::const_check(FMT_USE_DOUBLE))
-        detail::parse_float_type_spec(specs_, eh);
-      else
-        FMT_ASSERT(false, "double support disabled");
-      break;
-    case detail::type::long_double_type:
-      if (detail::const_check(FMT_USE_LONG_DOUBLE))
-        detail::parse_float_type_spec(specs_, eh);
-      else
-        FMT_ASSERT(false, "long double support disabled");
-      break;
-    case detail::type::cstring_type:
-      detail::check_cstring_type_spec(specs_.type, eh);
-      break;
-    case detail::type::string_type:
-      detail::check_string_type_spec(specs_.type, eh);
-      break;
-    case detail::type::pointer_type:
-      detail::check_pointer_type_spec(specs_.type, eh);
-      break;
-    }
+    if (type == detail::type::char_type) detail::check_char_specs(specs_);
     return end;
   }
 

include/fmt/format.h

@@ -2249,14 +2249,13 @@ FMT_CONSTEXPR auto write(OutputIt out,
                          basic_string_view<type_identity_t<Char>> s,
                          const format_specs<Char>& specs, locale_ref)
     -> OutputIt {
-  check_string_type_spec(specs.type);
   return write(out, s, specs);
 }
 template <typename Char, typename OutputIt>
 FMT_CONSTEXPR auto write(OutputIt out, const Char* s,
                          const format_specs<Char>& specs, locale_ref)
     -> OutputIt {
-  return check_cstring_type_spec(specs.type)
+  return specs.type != presentation_type::pointer
              ? write(out, basic_string_view<Char>(s), specs, {})
              : write_ptr<Char>(out, bit_cast<uintptr_t>(s), &specs);
 }
@@ -2283,6 +2282,68 @@ FMT_CONSTEXPR auto write(OutputIt out, T value) -> OutputIt {
   return base_iterator(out, it);
 }
 
+// A floating-point presentation format.
+enum class float_format : unsigned char {
+  general,  // General: exponent notation or fixed point based on magnitude.
+  exp,      // Exponent notation with the default precision of 6, e.g. 1.2e-3.
+  fixed,    // Fixed point with the default precision of 6, e.g. 0.0012.
+  hex
+};
+
+struct float_specs {
+  int precision;
+  float_format format : 8;
+  sign_t sign : 8;
+  bool upper : 1;
+  bool locale : 1;
+  bool binary32 : 1;
+  bool showpoint : 1;
+};
+
+template <typename ErrorHandler = error_handler, typename Char>
+FMT_CONSTEXPR auto parse_float_type_spec(const format_specs<Char>& specs,
+                                         ErrorHandler&& eh = {})
+    -> float_specs {
+  auto result = float_specs();
+  result.showpoint = specs.alt;
+  result.locale = specs.localized;
+  switch (specs.type) {
+  case presentation_type::none:
+    result.format = float_format::general;
+    break;
+  case presentation_type::general_upper:
+    result.upper = true;
+    FMT_FALLTHROUGH;
+  case presentation_type::general_lower:
+    result.format = float_format::general;
+    break;
+  case presentation_type::exp_upper:
+    result.upper = true;
+    FMT_FALLTHROUGH;
+  case presentation_type::exp_lower:
+    result.format = float_format::exp;
+    result.showpoint |= specs.precision != 0;
+    break;
+  case presentation_type::fixed_upper:
+    result.upper = true;
+    FMT_FALLTHROUGH;
+  case presentation_type::fixed_lower:
+    result.format = float_format::fixed;
+    result.showpoint |= specs.precision != 0;
+    break;
+  case presentation_type::hexfloat_upper:
+    result.upper = true;
+    FMT_FALLTHROUGH;
+  case presentation_type::hexfloat_lower:
+    result.format = float_format::hex;
+    break;
+  default:
+    eh.on_error("invalid format specifier");
+    break;
+  }
+  return result;
+}
+
 template <typename Char, typename OutputIt>
 FMT_CONSTEXPR20 auto write_nonfinite(OutputIt out, bool isnan,
                                      format_specs<Char> specs,
@@ -3465,7 +3526,6 @@ template <typename Char, typename OutputIt, typename T,
           FMT_ENABLE_IF(std::is_same<T, void>::value)>
 auto write(OutputIt out, const T* value, const format_specs<Char>& specs = {},
            locale_ref = {}) -> OutputIt {
-  check_pointer_type_spec(specs.type, error_handler());
   return write_ptr<Char>(out, bit_cast<uintptr_t>(value), &specs);
 }
 
@@ -3891,7 +3951,6 @@ template <> struct formatter<bytes> {
   FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
     auto end = parse_format_specs(ctx.begin(), ctx.end(), specs_, ctx,
                                   detail::type::string_type);
-    detail::check_string_type_spec(specs_.type, detail::error_handler());
     return end;
   }
 
@@ -3932,7 +3991,6 @@ template <typename T> struct formatter<group_digits_view<T>> : formatter<T> {
   FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
     auto end = parse_format_specs(ctx.begin(), ctx.end(), specs_, ctx,
                                   detail::type::int_type);
-    detail::check_string_type_spec(specs_.type, detail::error_handler());
     return end;
   }
 

include/fmt/printf.h

@@ -502,7 +502,7 @@ void vprintf(buffer<Char>& buf, basic_string_view<Char> format,
         break;
       }
     }
-    specs.type = parse_presentation_type(type);
+    specs.type = parse_presentation_type(type, arg.type());
     if (specs.type == presentation_type::none)
       throw_format_error("invalid type specifier");
 

test/core-test.cc

@@ -483,35 +483,6 @@ TEST(arg_test, visit_invalid_arg) {
   fmt::visit_format_arg(visitor, arg);
 }
 
-TEST(core_test, has_sign) {
-  using fmt::detail::type;
-  type types_with_sign[] = {type::int_type,    type::long_long_type,
-                            type::int128_type, type::float_type,
-                            type::double_type, type::long_double_type};
-  for (auto t : types_with_sign) EXPECT_TRUE(fmt::detail::has_sign(t));
-  type types_without_sign[] = {
-      type::none_type,    type::uint_type,    type::ulong_long_type,
-      type::uint128_type, type::bool_type,    type::char_type,
-      type::string_type,  type::cstring_type, type::custom_type};
-  for (auto t : types_without_sign) EXPECT_FALSE(fmt::detail::has_sign(t));
-}
-
-TEST(core_test, has_precision) {
-  using fmt::detail::type;
-  type types_with_precision[] = {type::float_type, type::double_type,
-                                 type::long_double_type, type::string_type,
-                                 type::cstring_type};
-  for (auto t : types_with_precision)
-    EXPECT_TRUE(fmt::detail::has_precision(t));
-  type types_without_precision[] = {type::none_type,       type::int_type,
-                                    type::uint_type,       type::long_long_type,
-                                    type::ulong_long_type, type::int128_type,
-                                    type::uint128_type,    type::bool_type,
-                                    type::char_type,       type::custom_type};
-  for (auto t : types_without_precision)
-    EXPECT_FALSE(fmt::detail::has_precision(t));
-}
-
 #if FMT_USE_CONSTEXPR
 
 enum class arg_id_result { none, empty, index, name };
@@ -571,7 +542,8 @@ TEST(core_test, constexpr_parse_format_specs) {
   static_assert(parse_test_specs("{42}").width_ref.val.index == 42, "");
   static_assert(parse_test_specs(".42").precision == 42, "");
   static_assert(parse_test_specs(".{42}").precision_ref.val.index == 42, "");
-  static_assert(parse_test_specs("d").type == fmt::presentation_type::dec, "");
+  static_assert(
+      parse_test_specs("f").type == fmt::presentation_type::fixed_lower, "");
 }
 
 struct test_format_string_handler {

test/xchar-test.cc

@@ -521,7 +521,7 @@ template <class charT> struct formatter<std::complex<double>, charT> {
   FMT_CONSTEXPR typename basic_format_parse_context<charT>::iterator parse(
       basic_format_parse_context<charT>& ctx) {
     auto end = parse_format_specs(ctx.begin(), ctx.end(), specs_, ctx,
-                                  detail::type::string_type);
+                                  detail::type::float_type);
     detail::parse_float_type_spec(specs_, detail::error_handler());
     return end;
   }