Simplify format string parsing

include/fmt/chrono.h

@@ -2010,11 +2010,7 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
     auto begin = ctx.begin(), end = ctx.end();
     if (begin == end || *begin == '}') return {begin, begin};
 
-    auto align_result = detail::parse_align(begin, end);
-    specs.align = align_result.align;
-    auto fill_size = align_result.end - begin - 1;
-    if (fill_size > 0) specs.fill = {begin, detail::to_unsigned(fill_size)};
-    begin = align_result.end;
+    begin = detail::parse_align(begin, end, specs);
     if (begin == end) return {begin, begin};
 
     begin = detail::parse_dynamic_spec(begin, end, specs.width, width_ref, ctx);

include/fmt/core.h

@@ -592,10 +592,6 @@ enum class type {
   custom_type
 };
 
-constexpr auto has_sign(type t) -> bool {
-  return ((0xe2a >> static_cast<int>(t)) & 1) != 0;
-}
-
 // Maps core type T to the corresponding type enum constant.
 template <typename T, typename Char>
 struct type_constant : std::integral_constant<type, type::custom_type> {};
@@ -628,6 +624,14 @@ 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 has_precision(type t) -> bool {
+  return ((0x3e00 >> static_cast<int>(t)) & 1) != 0;
+}
+
 FMT_NORETURN FMT_API void throw_format_error(const char* message);
 
 struct error_handler {
@@ -2253,15 +2257,10 @@ FMT_CONSTEXPR auto parse_nonnegative_int(const Char*& begin, const Char* end,
              : error_value;
 }
 
-template <typename Char> struct parse_align_result {
-  const Char* end;
-  align_t align;
-};
-
 // Parses [[fill]align].
 template <typename Char>
-FMT_CONSTEXPR auto parse_align(const Char* begin, const Char* end)
-    -> parse_align_result<Char> {
+FMT_CONSTEXPR auto parse_align(const Char* begin, const Char* end,
+                               format_specs<Char>& specs) -> const Char* {
   FMT_ASSERT(begin != end, "");
   auto align = align::none;
   auto p = begin + code_point_length(begin);
@@ -2281,11 +2280,12 @@ FMT_CONSTEXPR auto parse_align(const Char* begin, const Char* end)
     if (align != align::none) {
       if (p != begin) {
         auto c = *begin;
-        if (c == '}') return {begin, align::none};
+        if (c == '}') return begin;
         if (c == '{') {
           throw_format_error("invalid fill character '{'");
-          return {begin, align::none};
+          return begin;
         }
+        specs.fill = {begin, to_unsigned(p - begin)};
         begin = p + 1;
       } else {
         ++begin;
@@ -2296,7 +2296,8 @@ FMT_CONSTEXPR auto parse_align(const Char* begin, const Char* end)
     }
     p = begin;
   }
-  return {begin, align};
+  specs.align = align;
+  return begin;
 }
 
 template <typename Char> constexpr auto is_name_start(Char c) -> bool {
@@ -2392,7 +2393,7 @@ FMT_CONSTEXPR auto parse_precision(const Char* begin, const Char* end,
     -> const Char* {
   ++begin;
   if (begin == end || *begin == '}') {
-    throw_format_error("missing precision");
+    throw_format_error("invalid precision");
     return begin;
   }
   return parse_dynamic_spec(begin, end, value, ref, ctx);
@@ -2443,11 +2444,6 @@ FMT_CONSTEXPR inline auto parse_presentation_type(char type)
   }
 }
 
-FMT_CONSTEXPR inline void require_numeric_argument(type arg_type) {
-  if (!is_arithmetic_type(arg_type))
-    throw_format_error("format specifier requires numeric argument");
-}
-
 // Parses standard format specifiers.
 template <typename Char>
 FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
@@ -2460,16 +2456,10 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
   }
   if (begin == end) return begin;
 
-  auto align = parse_align(begin, end);
-  if (align.align != align::none) {
-    specs.align = align.align;
-    auto fill_size = align.end - begin - 1;
-    if (fill_size > 0) specs.fill = {begin, to_unsigned(fill_size)};
-  }
-  begin = align.end;
+  begin = parse_align(begin, end, specs);
   if (begin == end) return begin;
 
-  if (has_sign(arg_type)) {  // Parse sign.
+  if (has_sign(arg_type)) {
     switch (to_ascii(*begin)) {
     case '+':
       specs.sign = sign::plus;
@@ -2489,15 +2479,14 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
     if (begin == end) return begin;
   }
 
-  if (*begin == '#') {
-    require_numeric_argument(arg_type);
+  if (*begin == '#' && is_arithmetic_type(arg_type)) {
     specs.alt = true;
     if (++begin == end) return begin;
   }
 
-  // Parse zero flag.
-  if (*begin == '0') {
-    require_numeric_argument(arg_type);
+  if (*begin == '0') {  // Parse zero flag.
+    if (!is_arithmetic_type(arg_type))
+      throw_format_error("format specifier requires numeric argument");
     if (specs.align == align::none) {
       // Ignore 0 if align is specified for compatibility with std::format.
       specs.align = align::numeric;
@@ -2509,24 +2498,18 @@ 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;
 
-  // Parse precision.
-  if (*begin == '.') {
+  if (*begin == '.' && has_precision(arg_type)) {
     begin =
         parse_precision(begin, end, specs.precision, specs.precision_ref, ctx);
-    if (is_integral_type(arg_type) || arg_type == type::pointer_type)
-      throw_format_error("precision not allowed for this argument type");
     if (begin == end) return begin;
   }
 
-  if (*begin == 'L') {
-    require_numeric_argument(arg_type);
+  if (*begin == 'L' && is_arithmetic_type(arg_type)) {
     specs.localized = true;
-    ++begin;
+    if (++begin == end) return begin;
   }
 
-  // Parse type.
-  if (begin != end && *begin != '}')
-    specs.type = parse_presentation_type(to_ascii(*begin++));
+  if (*begin != '}') specs.type = parse_presentation_type(to_ascii(*begin++));
   return begin;
 }
 

test/core-test.cc

@@ -489,13 +489,29 @@ TEST(core_test, has_sign) {
                             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::uint_type,    type::ulong_long_type,
-                               type::uint128_type, type::bool_type,
-                               type::char_type,    type::string_type,
-                               type::cstring_type, type::custom_type};
+  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 };

test/format-test.cc

@@ -766,10 +766,10 @@ TEST(format_test, hash_flag) {
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:#}"), 'c'), format_error,
                    "invalid format specifier for char");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:#}"), "abc"), format_error,
-                   "format specifier requires numeric argument");
+                   "invalid format specifier");
   EXPECT_THROW_MSG(
       (void)fmt::format(runtime("{0:#}"), reinterpret_cast<void*>(0x42)),
-      format_error, "format specifier requires numeric argument");
+      format_error, "invalid format specifier");
 }
 
 TEST(format_test, zero_flag) {
@@ -907,54 +907,54 @@ TEST(format_test, precision) {
   char format_str[buffer_size];
   safe_sprintf(format_str, "{0:.%u", UINT_MAX);
   increment(format_str + 4);
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "number is too big");
   size_t size = std::strlen(format_str);
   format_str[size] = '}';
   format_str[size + 1] = 0;
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "number is too big");
 
   safe_sprintf(format_str, "{0:.%u", INT_MAX + 1u);
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "number is too big");
   safe_sprintf(format_str, "{0:.%u}", INT_MAX + 1u);
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "number is too big");
 
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:."), 0), format_error,
-                   "missing precision");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.}"), 0), format_error,
-                   "missing precision");
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:."), 0.0), format_error,
+                   "invalid precision");
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.}"), 0.0), format_error,
+                   "invalid precision");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2"), 0), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2}"), 42), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2f}"), 42), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2}"), 42u), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2f}"), 42u), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2}"), 42l), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2f}"), 42l), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2}"), 42ul), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2f}"), 42ul), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2}"), 42ll), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2f}"), 42ll), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2}"), 42ull), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2f}"), 42ull), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:3.0}"), 'x'), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_EQ("1.2", fmt::format("{0:.2}", 1.2345));
   EXPECT_EQ("1.2", fmt::format("{0:.2}", 1.2345l));
   EXPECT_EQ("1.2e+56", fmt::format("{:.2}", 1.234e56));
@@ -1033,10 +1033,10 @@ TEST(format_test, precision) {
 
   EXPECT_THROW_MSG(
       (void)fmt::format(runtime("{0:.2}"), reinterpret_cast<void*>(0xcafe)),
-      format_error, "precision not allowed for this argument type");
+      format_error, "invalid format specifier");
   EXPECT_THROW_MSG(
       (void)fmt::format(runtime("{0:.2f}"), reinterpret_cast<void*>(0xcafe)),
-      format_error, "precision not allowed for this argument type");
+      format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{:.{}e}"), 42.0,
                                      fmt::detail::max_value<int>()),
                    format_error, "number is too big");
@@ -1071,7 +1071,7 @@ TEST(format_test, runtime_precision) {
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{?}}"), 0.0), format_error,
                    "invalid format string");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}"), 0, 0), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0), format_error,
                    "argument not found");
 
@@ -1098,51 +1098,40 @@ TEST(format_test, runtime_precision) {
                    format_error, "precision is not integer");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42, 2), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"), 42, 2), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42u, 2), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"), 42u, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42l, 2), format_error,
-                   "precision not allowed for this argument type");
+                   "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"), 42l, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42ul, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"), 42ul, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42ll, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"), 42ll, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42ull, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"), 42ull, 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:3.{1}}"), 'x', 0),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_EQ("1.2", fmt::format("{0:.{1}}", 1.2345, 2));
   EXPECT_EQ("1.2", fmt::format("{1:.{0}}", 2, 1.2345l));
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"),
                                      reinterpret_cast<void*>(0xcafe), 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}f}"),
                                      reinterpret_cast<void*>(0xcafe), 2),
-                   format_error,
-                   "precision not allowed for this argument type");
+                   format_error, "invalid format specifier");
 
   EXPECT_EQ("st", fmt::format("{0:.{1}}", "str", 2));
 }

test/ostream-test.cc

@@ -92,7 +92,7 @@ TEST(ostream_test, format_specs) {
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0: }"), test_string()),
                    format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:#}"), test_string()),
-                   format_error, "format specifier requires numeric argument");
+                   format_error, "invalid format specifier");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:05}"), test_string()),
                    format_error, "format specifier requires numeric argument");
   EXPECT_EQ("test         ", fmt::format("{0:13}", test_string("test")));