Refactor presentation types

CMakeLists.txt

@@ -151,7 +151,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
       -Wcast-align
       -Wctor-dtor-privacy -Wdisabled-optimization
       -Winvalid-pch -Woverloaded-virtual
-      -Wconversion -Wswitch-enum -Wundef
+      -Wconversion -Wundef
       -Wno-ctor-dtor-privacy -Wno-format-nonliteral)
   if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.6)
       set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS}

include/fmt/chrono.h

@@ -904,7 +904,8 @@ template <typename Char, typename Rep, typename OutputIt,
 OutputIt format_duration_value(OutputIt out, Rep val, int precision) {
   auto specs = basic_format_specs<Char>();
   specs.precision = precision;
-  specs.type = precision > 0 ? 'f' : 'g';
+  specs.type = precision > 0 ? presentation_type::fixed_lower
+                             : presentation_type::general_lower;
   return write<Char>(out, val, specs);
 }
 

include/fmt/core.h

@@ -1949,11 +1949,33 @@ template <typename Char> struct fill_t {
 };
 FMT_END_DETAIL_NAMESPACE
 
+enum class presentation_type : unsigned char {
+  none,
+  // Integer types should go first,
+  dec,             // 'd'
+  oct,             // 'o'
+  hex_lower,       // 'x'
+  hex_upper,       // 'X'
+  bin_lower,       // 'b'
+  bin_upper,       // 'B'
+  hexfloat_lower,  // 'a'
+  hexfloat_upper,  // 'A'
+  exp_lower,       // 'e'
+  exp_upper,       // 'E'
+  fixed_lower,     // 'f'
+  fixed_upper,     // 'F'
+  general_lower,   // 'g'
+  general_upper,   // 'G'
+  chr,             // 'c'
+  string,          // 's'
+  pointer          // 'p'
+};
+
 // Format specifiers for built-in and string types.
 template <typename Char> struct basic_format_specs {
   int width;
   int precision;
-  char type;
+  presentation_type type;
   align_t align : 4;
   sign_t sign : 3;
   bool alt : 1;  // Alternate form ('#').
@@ -1963,7 +1985,7 @@ template <typename Char> struct basic_format_specs {
   constexpr basic_format_specs()
       : width(0),
         precision(-1),
-        type(0),
+        type(presentation_type::none),
         align(align::none),
         sign(sign::none),
         alt(false),
@@ -2043,9 +2065,7 @@ template <typename Char> class specs_setter {
   }
   FMT_CONSTEXPR void end_precision() {}
 
-  FMT_CONSTEXPR void on_type(Char type) {
+  FMT_CONSTEXPR void on_type(presentation_type type) { specs_.type = type; }
-    specs_.type = static_cast<char>(type);
-  }
 };
 
 // Format spec handler that saves references to arguments representing dynamic
@@ -2327,6 +2347,48 @@ FMT_CONSTEXPR auto parse_precision(const Char* begin, const Char* end,
   return begin;
 }
 
+template <typename Char>
+FMT_CONSTEXPR auto parse_presentation_type(Char type) -> presentation_type {
+  switch (to_ascii(type)) {
+  case 'd':
+    return presentation_type::dec;
+  case 'o':
+    return presentation_type::oct;
+  case 'x':
+    return presentation_type::hex_lower;
+  case 'X':
+    return presentation_type::hex_upper;
+  case 'b':
+    return presentation_type::bin_lower;
+  case 'B':
+    return presentation_type::bin_upper;
+  case 'a':
+    return presentation_type::hexfloat_lower;
+  case 'A':
+    return presentation_type::hexfloat_upper;
+  case 'e':
+    return presentation_type::exp_lower;
+  case 'E':
+    return presentation_type::exp_upper;
+  case 'f':
+    return presentation_type::fixed_lower;
+  case 'F':
+    return presentation_type::fixed_upper;
+  case 'g':
+    return presentation_type::general_lower;
+  case 'G':
+    return presentation_type::general_upper;
+  case 'c':
+    return presentation_type::chr;
+  case 's':
+    return presentation_type::string;
+  case 'p':
+    return presentation_type::pointer;
+  default:
+    return presentation_type::none;
+  }
+}
+
 // Parses standard format specifiers and sends notifications about parsed
 // components to handler.
 template <typename Char, typename SpecHandler>
@@ -2336,7 +2398,10 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(const Char* begin,
     -> const Char* {
   if (begin + 1 < end && begin[1] == '}' && is_ascii_letter(*begin) &&
       *begin != 'L') {
-    handler.on_type(*begin++);
+    presentation_type type = parse_presentation_type(*begin++);
+    if (type == presentation_type::none)
+      handler.on_error("invalid type specifier");
+    handler.on_type(type);
     return begin;
   }
 
@@ -2390,7 +2455,12 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(const Char* begin,
   }
 
   // Parse type.
-  if (begin != end && *begin != '}') handler.on_type(*begin++);
+  if (begin != end && *begin != '}') {
+    presentation_type type = parse_presentation_type(*begin++);
+    if (type == presentation_type::none)
+      handler.on_error("invalid type specifier");
+    handler.on_type(type);
+  }
   return begin;
 }
 
@@ -2532,28 +2602,18 @@ class compile_parse_context
 };
 
 template <typename ErrorHandler>
-FMT_CONSTEXPR void check_int_type_spec(char spec, ErrorHandler&& eh) {
+FMT_CONSTEXPR void check_int_type_spec(presentation_type type,
-  switch (spec) {
+                                       ErrorHandler&& eh) {
-  case 0:
+  if (type > presentation_type::bin_upper && type != presentation_type::chr)
-  case 'd':
-  case 'x':
-  case 'X':
-  case 'b':
-  case 'B':
-  case 'o':
-  case 'c':
-    break;
-  default:
     eh.on_error("invalid type specifier");
-    break;
-  }
 }
 
 // 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 basic_format_specs<Char>& specs,
                                     ErrorHandler&& eh = {}) -> bool {
-  if (specs.type && specs.type != 'c') {
+  if (specs.type != presentation_type::none &&
+      specs.type != presentation_type::chr) {
     check_int_type_spec(specs.type, eh);
     return false;
   }
@@ -2589,33 +2649,33 @@ FMT_CONSTEXPR auto parse_float_type_spec(const basic_format_specs<Char>& specs,
   result.showpoint = specs.alt;
   result.locale = specs.localized;
   switch (specs.type) {
-  case 0:
+  case presentation_type::none:
     result.format = float_format::general;
     break;
-  case 'G':
+  case presentation_type::general_upper:
     result.upper = true;
     FMT_FALLTHROUGH;
-  case 'g':
+  case presentation_type::general_lower:
     result.format = float_format::general;
     break;
-  case 'E':
+  case presentation_type::exp_upper:
     result.upper = true;
     FMT_FALLTHROUGH;
-  case 'e':
+  case presentation_type::exp_lower:
     result.format = float_format::exp;
     result.showpoint |= specs.precision != 0;
     break;
-  case 'F':
+  case presentation_type::fixed_upper:
     result.upper = true;
     FMT_FALLTHROUGH;
-  case 'f':
+  case presentation_type::fixed_lower:
     result.format = float_format::fixed;
     result.showpoint |= specs.precision != 0;
     break;
-  case 'A':
+  case presentation_type::hexfloat_upper:
     result.upper = true;
     FMT_FALLTHROUGH;
-  case 'a':
+  case presentation_type::hexfloat_lower:
     result.format = float_format::hex;
     break;
   default:
@@ -2625,22 +2685,27 @@ FMT_CONSTEXPR auto parse_float_type_spec(const basic_format_specs<Char>& specs,
   return result;
 }
 
-template <typename Char, typename ErrorHandler = error_handler>
+template <typename ErrorHandler = error_handler>
-FMT_CONSTEXPR auto check_cstring_type_spec(Char spec, ErrorHandler&& eh = {})
+FMT_CONSTEXPR auto check_cstring_type_spec(presentation_type type,
-    -> bool {
+                                           ErrorHandler&& eh = {}) -> bool {
-  if (spec == 0 || spec == 's') return true;
+  if (type == presentation_type::none || type == presentation_type::string)
-  if (spec != 'p') eh.on_error("invalid type specifier");
+    return true;
+  if (type != presentation_type::pointer) eh.on_error("invalid type specifier");
   return false;
 }
 
-template <typename Char, typename ErrorHandler = error_handler>
+template <typename ErrorHandler = error_handler>
-FMT_CONSTEXPR void check_string_type_spec(Char spec, ErrorHandler&& eh = {}) {
+FMT_CONSTEXPR void check_string_type_spec(presentation_type type,
-  if (spec != 0 && spec != 's') eh.on_error("invalid type specifier");
+                                          ErrorHandler&& eh = {}) {
+  if (type != presentation_type::none && type != presentation_type::string)
+    eh.on_error("invalid type specifier");
 }
 
-template <typename Char, typename ErrorHandler>
+template <typename ErrorHandler>
-FMT_CONSTEXPR void check_pointer_type_spec(Char spec, ErrorHandler&& eh) {
+FMT_CONSTEXPR void check_pointer_type_spec(presentation_type type,
-  if (spec != 0 && spec != 'p') eh.on_error("invalid type specifier");
+                                           ErrorHandler&& eh) {
+  if (type != presentation_type::none && type != presentation_type::pointer)
+    eh.on_error("invalid type specifier");
 }
 
 // A parse_format_specs handler that checks if specifiers are consistent with
@@ -2818,7 +2883,10 @@ struct formatter<T, Char,
       FMT_ASSERT(false, "invalid argument type");
       break;
     case detail::type::bool_type:
-      if (!specs_.type || specs_.type == 's') break;
+      if (specs_.type == presentation_type::none ||
+          specs_.type == presentation_type::string) {
+        break;
+      }
       FMT_FALLTHROUGH;
     case detail::type::int_type:
     case detail::type::uint_type:

include/fmt/format.h

@@ -1558,10 +1558,10 @@ FMT_CONSTEXPR FMT_INLINE auto write_int(OutputIt out, write_int_arg<T> arg,
   static_assert(std::is_same<T, uint32_or_64_or_128_t<T>>::value, "");
   auto abs_value = arg.abs_value;
   auto prefix = arg.prefix;
-  auto utype = static_cast<unsigned>(specs.type);
+  auto pres_type = static_cast<presentation_type>(specs.type);
-  switch (specs.type) {
+  switch (pres_type) {
-  case 0:
+  case presentation_type::none:
-  case 'd': {
+  case presentation_type::dec: {
     if (specs.localized &&
         write_int_localized(out, static_cast<uint64_or_128_t<T>>(abs_value),
                             prefix, specs, loc)) {
@@ -1573,38 +1573,40 @@ FMT_CONSTEXPR FMT_INLINE auto write_int(OutputIt out, write_int_arg<T> arg,
           return format_decimal<Char>(it, abs_value, num_digits).end;
         });
   }
-  case 'x':
+  case presentation_type::hex_lower:
-  case 'X': {
+  case presentation_type::hex_upper: {
-    if (specs.alt) prefix_append(prefix, (utype << 8) | '0');
+    bool upper = pres_type == presentation_type::hex_upper;
-    bool upper = specs.type != 'x';
+    if (specs.alt)
+      prefix_append(prefix, unsigned(upper ? 'X' : 'x') << 8 | '0');
     int num_digits = count_digits<4>(abs_value);
     return write_int(
         out, num_digits, prefix, specs, [=](reserve_iterator<OutputIt> it) {
           return format_uint<4, Char>(it, abs_value, num_digits, upper);
         });
   }
-  case 'b':
+  case presentation_type::bin_lower:
-  case 'B': {
+  case presentation_type::bin_upper: {
-    if (specs.alt) prefix_append(prefix, (utype << 8) | '0');
+    bool upper = pres_type == presentation_type::bin_upper;
+    if (specs.alt)
+      prefix_append(prefix, unsigned(upper ? 'B' : 'b') << 8 | '0');
     int num_digits = count_digits<1>(abs_value);
     return write_int(out, num_digits, prefix, specs,
                      [=](reserve_iterator<OutputIt> it) {
                        return format_uint<1, Char>(it, abs_value, num_digits);
                      });
   }
-  case 'o': {
+  case presentation_type::oct: {
     int num_digits = count_digits<3>(abs_value);
-    if (specs.alt && specs.precision <= num_digits && abs_value != 0) {
+    // Octal prefix '0' is counted as a digit, so only add it if precision
-      // Octal prefix '0' is counted as a digit, so only add it if precision
+    // is not greater than the number of digits.
-      // is not greater than the number of digits.
+    if (specs.alt && specs.precision <= num_digits && abs_value != 0)
       prefix_append(prefix, '0');
-    }
     return write_int(out, num_digits, prefix, specs,
                      [=](reserve_iterator<OutputIt> it) {
                        return format_uint<3, Char>(it, abs_value, num_digits);
                      });
   }
-  case 'c':
+  case presentation_type::chr:
     return write_char(out, static_cast<Char>(abs_value), specs);
   default:
     FMT_THROW(format_error("invalid type specifier"));
@@ -1924,7 +1926,9 @@ auto write(OutputIt out, T value, basic_format_specs<Char> specs,
     return write_bytes<align::right>(out, {buffer.data(), buffer.size()},
                                      specs);
   }
-  int precision = specs.precision >= 0 || !specs.type ? specs.precision : 6;
+  int precision = specs.precision >= 0 || specs.type == presentation_type::none
+                      ? specs.precision
+                      : 6;
   if (fspecs.format == float_format::exp) {
     if (precision == max_value<int>())
       FMT_THROW(format_error("number is too big"));
@@ -2032,7 +2036,8 @@ template <typename Char, typename OutputIt, typename T,
 FMT_CONSTEXPR auto write(OutputIt out, T value,
                          const basic_format_specs<Char>& specs = {},
                          locale_ref = {}) -> OutputIt {
-  return specs.type && specs.type != 's'
+  return specs.type != presentation_type::none &&
+                 specs.type != presentation_type::string
              ? write(out, value ? 1 : 0, specs, {})
              : write_bytes(out, value ? "true" : "false", specs);
 }

include/fmt/printf.h

@@ -233,7 +233,7 @@ class printf_arg_formatter : public arg_formatter<Char> {
 
   OutputIt write_null_pointer(bool is_string = false) {
     auto s = this->specs;
-    s.type = 0;
+    s.type = presentation_type::none;
     return write_bytes(this->out, is_string ? "(null)" : "(nil)", s);
   }
 
@@ -249,8 +249,10 @@ class printf_arg_formatter : public arg_formatter<Char> {
     // std::is_same instead.
     if (std::is_same<T, Char>::value) {
       format_specs fmt_specs = this->specs;
-      if (fmt_specs.type && fmt_specs.type != 'c')
+      if (fmt_specs.type != presentation_type::none &&
+          fmt_specs.type != presentation_type::chr) {
         return (*this)(static_cast<int>(value));
+      }
       fmt_specs.sign = sign::none;
       fmt_specs.alt = false;
       fmt_specs.fill[0] = ' ';  // Ignore '0' flag for char types.
@@ -271,13 +273,13 @@ class printf_arg_formatter : public arg_formatter<Char> {
   /** Formats a null-terminated C string. */
   OutputIt operator()(const char* value) {
     if (value) return base::operator()(value);
-    return write_null_pointer(this->specs.type != 'p');
+    return write_null_pointer(this->specs.type != presentation_type::pointer);
   }
 
   /** Formats a null-terminated wide C string. */
   OutputIt operator()(const wchar_t* value) {
     if (value) return base::operator()(value);
-    return write_null_pointer(this->specs.type != 'p');
+    return write_null_pointer(this->specs.type != presentation_type::pointer);
   }
 
   OutputIt operator()(basic_string_view<Char> value) {
@@ -490,13 +492,13 @@ void vprintf(buffer<Char>& buf, basic_string_view<Char> format,
 
     // Parse type.
     if (it == end) FMT_THROW(format_error("invalid format string"));
-    specs.type = static_cast<char>(*it++);
+    char type = static_cast<char>(*it++);
     if (arg.is_integral()) {
       // Normalize type.
-      switch (specs.type) {
+      switch (type) {
       case 'i':
       case 'u':
-        specs.type = 'd';
+        type = 'd';
         break;
       case 'c':
         visit_format_arg(
@@ -505,6 +507,9 @@ void vprintf(buffer<Char>& buf, basic_string_view<Char> format,
         break;
       }
     }
+    specs.type = parse_presentation_type(type);
+    if (specs.type == presentation_type::none)
+      parse_ctx.on_error("invalid type specifier");
 
     start = it;
 

test/core-test.cc

@@ -524,7 +524,7 @@ struct test_format_specs_handler {
   fmt::detail::arg_ref<char> width_ref;
   int precision = 0;
   fmt::detail::arg_ref<char> precision_ref;
-  char type = 0;
+  fmt::presentation_type type = fmt::presentation_type::none;
 
   // Workaround for MSVC2017 bug that results in "expression did not evaluate
   // to a constant" with compiler-generated copy ctor.
@@ -550,14 +550,14 @@ struct test_format_specs_handler {
   constexpr void on_dynamic_precision(string_view) {}
 
   constexpr void end_precision() {}
-  constexpr void on_type(char t) { type = t; }
+  constexpr void on_type(fmt::presentation_type t) { type = t; }
   constexpr void on_error(const char*) { res = error; }
 };
 
 template <size_t N>
 constexpr test_format_specs_handler parse_test_specs(const char (&s)[N]) {
   auto h = test_format_specs_handler();
-  fmt::detail::parse_format_specs(s, s + N, h);
+  fmt::detail::parse_format_specs(s, s + N - 1, h);
   return h;
 }
 
@@ -575,7 +575,7 @@ 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 == 'd', "");
+  static_assert(parse_test_specs("d").type == fmt::presentation_type::dec, "");
   static_assert(parse_test_specs("{<").res == handler::error, "");
 }
 
@@ -597,7 +597,7 @@ constexpr fmt::detail::dynamic_format_specs<char> parse_dynamic_specs(
   auto specs = fmt::detail::dynamic_format_specs<char>();
   auto ctx = test_parse_context();
   auto h = fmt::detail::dynamic_specs_handler<test_parse_context>(specs, ctx);
-  parse_format_specs(s, s + N, h);
+  parse_format_specs(s, s + N - 1, h);
   return specs;
 }
 
@@ -615,14 +615,15 @@ TEST(format_test, constexpr_dynamic_specs_handler) {
   static_assert(parse_dynamic_specs(".42").precision == 42, "");
   static_assert(parse_dynamic_specs(".{}").precision_ref.val.index == 11, "");
   static_assert(parse_dynamic_specs(".{42}").precision_ref.val.index == 42, "");
-  static_assert(parse_dynamic_specs("d").type == 'd', "");
+  static_assert(parse_dynamic_specs("d").type == fmt::presentation_type::dec,
+                "");
 }
 
 template <size_t N>
 constexpr test_format_specs_handler check_specs(const char (&s)[N]) {
   fmt::detail::specs_checker<test_format_specs_handler> checker(
       test_format_specs_handler(), fmt::detail::type::double_type);
-  parse_format_specs(s, s + N, checker);
+  parse_format_specs(s, s + N - 1, checker);
   return checker;
 }
 
@@ -639,7 +640,7 @@ TEST(format_test, constexpr_specs_checker) {
   static_assert(check_specs("{42}").width_ref.val.index == 42, "");
   static_assert(check_specs(".42").precision == 42, "");
   static_assert(check_specs(".{42}").precision_ref.val.index == 42, "");
-  static_assert(check_specs("d").type == 'd', "");
+  static_assert(check_specs("d").type == fmt::presentation_type::dec, "");
   static_assert(check_specs("{<").res == handler::error, "");
 }
 

test/format-test.cc

@@ -555,7 +555,7 @@ TEST(format_test, fill) {
             fmt::format(string_view("{:\0>4}", 6), '*'));
   EXPECT_EQ("жж42", fmt::format("{0:ж>4}", 42));
   EXPECT_THROW_MSG(fmt::format(runtime("{:\x80\x80\x80\x80\x80>}"), 0),
-                   format_error, "missing '}' in format string");
+                   format_error, "invalid type specifier");
 }
 
 TEST(format_test, plus_sign) {
@@ -1037,7 +1037,7 @@ void check_unknown_types(const T& value, const char* types, const char*) {
 
 TEST(format_test, format_int) {
   EXPECT_THROW_MSG(fmt::format(runtime("{0:v"), 42), format_error,
-                   "missing '}' in format string");
+                   "invalid type specifier");
   check_unknown_types(42, "bBdoxXnLc", "integer");
   EXPECT_EQ("x", fmt::format("{:c}", static_cast<int>('x')));
 }

test/fuzzing/named-arg.cc

@@ -29,7 +29,8 @@ void invoke_fmt(const uint8_t* data, size_t size, unsigned arg_name_size) {
         fmt::format(format_str.get(), fmt::arg(arg_name.data(), value));
 #else
     fmt::memory_buffer out;
-    fmt::format_to(out, format_str.get(), fmt::arg(arg_name.data(), value));
+    fmt::format_to(std::back_inserter(out), format_str.get(),
+                   fmt::arg(arg_name.data(), value));
 #endif
   } catch (std::exception&) {
   }

test/xchar-test.cc

@@ -409,7 +409,7 @@ template <class charT> struct formatter<std::complex<double>, charT> {
         specs_.precision, specs_.precision_ref, ctx);
     auto specs = std::string();
     if (specs_.precision > 0) specs = fmt::format(".{}", specs_.precision);
-    if (specs_.type) specs += specs_.type;
+    if (specs_.type == presentation_type::fixed_lower) specs += 'f';
     auto real = fmt::format(ctx.locale().template get<std::locale>(),
                             fmt::runtime("{:" + specs + "}"), c.real());
     auto imag = fmt::format(ctx.locale().template get<std::locale>(),