Refactor format_specs for #1109 and #940

Refactor `format_specs` and related APIs to support variable-width fill
(#1109), improve naming consistency, remove legacy setters (#940), and
optimize layout.

include/fmt/chrono.h

@@ -715,7 +715,7 @@ struct chrono_formatter {
 template <typename Rep, typename Period, typename Char>
 struct formatter<std::chrono::duration<Rep, Period>, Char> {
  private:
-  align_spec spec;
+  basic_format_specs<Char> spec;
   int precision;
   typedef internal::arg_ref<Char> arg_ref_type;
   arg_ref_type width_ref;
@@ -744,9 +744,9 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
     }
 
     void on_error(const char* msg) { FMT_THROW(format_error(msg)); }
-    void on_fill(Char fill) { f.spec.fill_ = fill; }
-    void on_align(alignment align) { f.spec.align_ = align; }
-    void on_width(unsigned width) { f.spec.width_ = width; }
+    void on_fill(Char fill) { f.spec.fill[0] = fill; }
+    void on_align(align_t align) { f.spec.align = align; }
+    void on_width(unsigned width) { f.spec.width = width; }
     void on_precision(unsigned precision) { f.precision = precision; }
     void end_precision() {}
 
@@ -804,7 +804,7 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
     using range = internal::output_range<decltype(ctx.out()), Char>;
     internal::basic_writer<range> w(range(ctx.out()));
     internal::handle_dynamic_spec<internal::width_checker>(
-        spec.width_, width_ref, ctx, format_str.begin());
+        spec.width, width_ref, ctx, format_str.begin());
     internal::handle_dynamic_spec<internal::precision_checker>(
         precision, precision_ref, ctx, format_str.begin());
     if (begin == end || *begin == '}') {

include/fmt/format-inl.h

@@ -763,7 +763,7 @@ char* sprintf_format(Double value, internal::buffer<char>& buf,
   char format[max_format_size];
   char* format_ptr = format;
   *format_ptr++ = '%';
-  if (spec.has(HASH_FLAG) || !spec.type) *format_ptr++ = '#';
+  if (spec.alt || !spec.type) *format_ptr++ = '#';
   if (spec.precision >= 0) {
     *format_ptr++ = '.';
     *format_ptr++ = '*';

include/fmt/prepare.h

@@ -293,7 +293,7 @@ class prepared_format {
         auto specs = value.spec.parsed_specs;
 
         handle_dynamic_spec<internal::width_checker>(
-            specs.width_, specs.width_ref, ctx, format_view.begin());
+            specs.width, specs.width_ref, ctx, format_view.begin());
         handle_dynamic_spec<internal::precision_checker>(
             specs.precision, specs.precision_ref, ctx, format_view.begin());
 
@@ -330,21 +330,10 @@ class prepared_format {
                             internal::type arg_type) const {
     internal::error_handler h;
     numeric_specs_checker<internal::error_handler> checker(h, arg_type);
-    if (specs.align_ == ALIGN_NUMERIC) {
-      checker.require_numeric_argument();
-    }
-
-    if (specs.has(PLUS_FLAG | MINUS_FLAG | SIGN_FLAG)) {
-      checker.check_sign();
-    }
-
-    if (specs.has(HASH_FLAG)) {
-      checker.require_numeric_argument();
-    }
-
-    if (specs.has_precision()) {
-      checker.check_precision();
-    }
+    if (specs.align == align::numeric) checker.require_numeric_argument();
+    if (specs.sign != sign::none) checker.check_sign();
+    if (specs.alt) checker.require_numeric_argument();
+    if (specs.precision >= 0) checker.check_precision();
   }
 
  private:

include/fmt/printf.h

@@ -160,7 +160,7 @@ template <typename Char> class printf_width_handler {
   unsigned operator()(T value) {
     auto width = static_cast<uint32_or_64_t<T>>(value);
     if (internal::is_negative(value)) {
-      spec_.align_ = ALIGN_LEFT;
+      spec_.align = align::left;
       width = 0 - width;
     }
     unsigned int_max = std::numeric_limits<int>::max();
@@ -248,8 +248,9 @@ class printf_arg_formatter : public internal::arg_formatter_base<Range> {
       format_specs& fmt_spec = *this->spec();
       if (fmt_spec.type && fmt_spec.type != 'c')
         return (*this)(static_cast<int>(value));
-      fmt_spec.flags = 0;
-      fmt_spec.align_ = ALIGN_RIGHT;
+      fmt_spec.sign = sign::none;
+      fmt_spec.alt = false;
+      fmt_spec.align = align::right;
       return base::operator()(value);
     } else {
       return base::operator()(value);
@@ -378,19 +379,19 @@ void basic_printf_context<OutputIt, Char>::parse_flags(format_specs& spec,
   for (; it != end; ++it) {
     switch (*it) {
     case '-':
-      spec.align_ = ALIGN_LEFT;
+      spec.align = align::left;
       break;
     case '+':
-      spec.flags |= SIGN_FLAG | PLUS_FLAG;
+      spec.sign = sign::plus;
       break;
     case '0':
-      spec.fill_ = '0';
+      spec.fill[0] = '0';
       break;
     case ' ':
-      spec.flags |= SIGN_FLAG;
+      spec.sign = sign::space;
       break;
     case '#':
-      spec.flags |= HASH_FLAG;
+      spec.alt = true;
       break;
     default:
       return;
@@ -422,11 +423,11 @@ unsigned basic_printf_context<OutputIt, Char>::parse_header(
       ++it;
       arg_index = value;
     } else {
-      if (c == '0') spec.fill_ = '0';
+      if (c == '0') spec.fill[0] = '0';
       if (value != 0) {
         // Nonzero value means that we parsed width and don't need to
         // parse it or flags again, so return now.
-        spec.width_ = value;
+        spec.width = value;
         return arg_index;
       }
     }
@@ -436,10 +437,10 @@ unsigned basic_printf_context<OutputIt, Char>::parse_header(
   if (it != end) {
     if (*it >= '0' && *it <= '9') {
       internal::error_handler eh;
-      spec.width_ = parse_nonnegative_int(it, end, eh);
+      spec.width = parse_nonnegative_int(it, end, eh);
     } else if (*it == '*') {
       ++it;
-      spec.width_ = visit_format_arg(
+      spec.width = visit_format_arg(
           internal::printf_width_handler<char_type>(spec), get_arg());
     }
   }
@@ -464,7 +465,7 @@ OutputIt basic_printf_context<OutputIt, Char>::format() {
     out = std::copy(start, it - 1, out);
 
     format_specs spec;
-    spec.align_ = ALIGN_RIGHT;
+    spec.align = align::right;
 
     // Parse argument index, flags and width.
     unsigned arg_index = parse_header(it, end, spec);
@@ -486,14 +487,13 @@ OutputIt basic_printf_context<OutputIt, Char>::format() {
     }
 
     format_arg arg = get_arg(arg_index);
-    if (spec.has(HASH_FLAG) && visit_format_arg(internal::is_zero_int(), arg))
-      spec.flags = static_cast<uint_least8_t>(
-          spec.flags & (~internal::to_unsigned<int>(HASH_FLAG)));
-    if (spec.fill_ == '0') {
+    if (spec.alt && visit_format_arg(internal::is_zero_int(), arg))
+      spec.alt = false;
+    if (spec.fill[0] == '0') {
       if (arg.is_arithmetic())
-        spec.align_ = ALIGN_NUMERIC;
+        spec.align = align::numeric;
       else
-        spec.fill_ = ' ';  // Ignore '0' flag for non-numeric types.
+        spec.fill[0] = ' ';  // Ignore '0' flag for non-numeric types.
     }
 
     // Parse length and convert the argument to the required type.

test/format

@@ -690,7 +690,7 @@ struct formatter {
   template <typename FormatContext>
   auto format(const T& val, FormatContext& ctx) -> decltype(ctx.out()) {
     fmt::internal::handle_dynamic_spec<fmt::internal::width_checker>(
-        specs_.width_, specs_.width_ref, ctx, nullptr);
+        specs_.width, specs_.width_ref, ctx, nullptr);
     fmt::internal::handle_dynamic_spec<fmt::internal::precision_checker>(
         specs_.precision, specs_.precision_ref, ctx, nullptr);
     using range_type = fmt::internal::output_range<typename FormatContext::iterator,

test/format-test.cc

@@ -2103,7 +2103,7 @@ struct test_format_specs_handler {
   enum Result { NONE, PLUS, MINUS, SPACE, HASH, ZERO, ERROR };
   Result res = NONE;
 
-  fmt::alignment align_ = fmt::ALIGN_DEFAULT;
+  fmt::align_t align = fmt::align::none;
   char fill = 0;
   unsigned width = 0;
   fmt::internal::arg_ref<char> width_ref;
@@ -2117,7 +2117,7 @@ struct test_format_specs_handler {
   FMT_CONSTEXPR test_format_specs_handler(
       const test_format_specs_handler& other)
       : res(other.res),
-        align_(other.align_),
+        align(other.align),
         fill(other.fill),
         width(other.width),
         width_ref(other.width_ref),
@@ -2125,7 +2125,7 @@ struct test_format_specs_handler {
         precision_ref(other.precision_ref),
         type(other.type) {}
 
-  FMT_CONSTEXPR void on_align(fmt::alignment a) { align_ = a; }
+  FMT_CONSTEXPR void on_align(fmt::align_t a) { align = a; }
   FMT_CONSTEXPR void on_fill(char f) { fill = f; }
   FMT_CONSTEXPR void on_plus() { res = PLUS; }
   FMT_CONSTEXPR void on_minus() { res = MINUS; }
@@ -2159,7 +2159,7 @@ FMT_CONSTEXPR test_format_specs_handler parse_test_specs(const char (&s)[N]) {
 
 TEST(FormatTest, ConstexprParseFormatSpecs) {
   typedef test_format_specs_handler handler;
-  static_assert(parse_test_specs("<").align_ == fmt::ALIGN_LEFT, "");
+  static_assert(parse_test_specs("<").align == fmt::align::left, "");
   static_assert(parse_test_specs("*^").fill == '*', "");
   static_assert(parse_test_specs("+").res == handler::PLUS, "");
   static_assert(parse_test_specs("-").res == handler::MINUS, "");
@@ -2216,16 +2216,16 @@ FMT_CONSTEXPR fmt::format_specs parse_specs(const char (&s)[N]) {
 }
 
 TEST(FormatTest, ConstexprSpecsHandler) {
-  static_assert(parse_specs("<").align() == fmt::ALIGN_LEFT, "");
-  static_assert(parse_specs("*^").fill() == '*', "");
-  static_assert(parse_specs("+").has(fmt::PLUS_FLAG), "");
-  static_assert(parse_specs("-").has(fmt::MINUS_FLAG), "");
-  static_assert(parse_specs(" ").has(fmt::SIGN_FLAG), "");
-  static_assert(parse_specs("#").has(fmt::HASH_FLAG), "");
-  static_assert(parse_specs("0").align() == fmt::ALIGN_NUMERIC, "");
-  static_assert(parse_specs("42").width() == 42, "");
-  static_assert(parse_specs("{}").width() == 11, "");
-  static_assert(parse_specs("{22}").width() == 22, "");
+  static_assert(parse_specs("<").align == fmt::align::left, "");
+  static_assert(parse_specs("*^").fill[0] == '*', "");
+  static_assert(parse_specs("+").sign == fmt::sign::plus, "");
+  static_assert(parse_specs("-").sign == fmt::sign::minus, "");
+  static_assert(parse_specs(" ").sign == fmt::sign::space, "");
+  static_assert(parse_specs("#").alt, "");
+  static_assert(parse_specs("0").align == fmt::align::numeric, "");
+  static_assert(parse_specs("42").width == 42, "");
+  static_assert(parse_specs("{}").width == 11, "");
+  static_assert(parse_specs("{22}").width == 22, "");
   static_assert(parse_specs(".42").precision == 42, "");
   static_assert(parse_specs(".{}").precision == 11, "");
   static_assert(parse_specs(".{22}").precision == 22, "");
@@ -2243,14 +2243,14 @@ FMT_CONSTEXPR fmt::internal::dynamic_format_specs<char> parse_dynamic_specs(
 }
 
 TEST(FormatTest, ConstexprDynamicSpecsHandler) {
-  static_assert(parse_dynamic_specs("<").align() == fmt::ALIGN_LEFT, "");
-  static_assert(parse_dynamic_specs("*^").fill() == '*', "");
-  static_assert(parse_dynamic_specs("+").has(fmt::PLUS_FLAG), "");
-  static_assert(parse_dynamic_specs("-").has(fmt::MINUS_FLAG), "");
-  static_assert(parse_dynamic_specs(" ").has(fmt::SIGN_FLAG), "");
-  static_assert(parse_dynamic_specs("#").has(fmt::HASH_FLAG), "");
-  static_assert(parse_dynamic_specs("0").align() == fmt::ALIGN_NUMERIC, "");
-  static_assert(parse_dynamic_specs("42").width() == 42, "");
+  static_assert(parse_dynamic_specs("<").align == fmt::align::left, "");
+  static_assert(parse_dynamic_specs("*^").fill[0] == '*', "");
+  static_assert(parse_dynamic_specs("+").sign == fmt::sign::plus, "");
+  static_assert(parse_dynamic_specs("-").sign == fmt::sign::minus, "");
+  static_assert(parse_dynamic_specs(" ").sign == fmt::sign::space, "");
+  static_assert(parse_dynamic_specs("#").alt, "");
+  static_assert(parse_dynamic_specs("0").align == fmt::align::numeric, "");
+  static_assert(parse_dynamic_specs("42").width == 42, "");
   static_assert(parse_dynamic_specs("{}").width_ref.val.index == 11, "");
   static_assert(parse_dynamic_specs("{42}").width_ref.val.index == 42, "");
   static_assert(parse_dynamic_specs(".42").precision == 42, "");
@@ -2269,7 +2269,7 @@ FMT_CONSTEXPR test_format_specs_handler check_specs(const char (&s)[N]) {
 
 TEST(FormatTest, ConstexprSpecsChecker) {
   typedef test_format_specs_handler handler;
-  static_assert(check_specs("<").align_ == fmt::ALIGN_LEFT, "");
+  static_assert(check_specs("<").align == fmt::align::left, "");
   static_assert(check_specs("*^").fill == '*', "");
   static_assert(check_specs("+").res == handler::PLUS, "");
   static_assert(check_specs("-").res == handler::MINUS, "");

test/prepare-test.cc

@@ -71,12 +71,12 @@ bool operator==(const format_part<char>::specification& lhs,
   } break;
   }
 
-  return std::tie(lhs.parsed_specs.width_, lhs.parsed_specs.fill_,
-                  lhs.parsed_specs.align_, lhs.parsed_specs.precision,
-                  lhs.parsed_specs.flags, lhs.parsed_specs.type) ==
-         std::tie(rhs.parsed_specs.width_, rhs.parsed_specs.fill_,
-                  rhs.parsed_specs.align_, rhs.parsed_specs.precision,
-                  rhs.parsed_specs.flags, rhs.parsed_specs.type);
+  return std::tie(lhs.parsed_specs.width, lhs.parsed_specs.fill[0],
+                  lhs.parsed_specs.align, lhs.parsed_specs.precision,
+                  lhs.parsed_specs.sign, lhs.parsed_specs.type) ==
+         std::tie(rhs.parsed_specs.width, rhs.parsed_specs.fill[0],
+                  rhs.parsed_specs.align, rhs.parsed_specs.precision,
+                  rhs.parsed_specs.sign, rhs.parsed_specs.type);
 }
 
 bool operator!=(const format_part<char>::specification& lhs,
@@ -352,8 +352,8 @@ TEST(
   const auto last_part = format_part(0u);
   format_part::specification expected_specification(0u);
   fmt::internal::dynamic_format_specs<char> specs{};
-  specs.align_ = fmt::alignment::ALIGN_LEFT;
-  specs.width_ = 10;
+  specs.align = fmt::align::left;
+  specs.width = 10;
   expected_specification.parsed_specs = specs;
 
   auto expected_substitution_part = format_part(expected_specification);
@@ -384,8 +384,8 @@ TEST(
   const auto last_part = format_part(format_part::named_argument_id(arg_id));
   format_part::specification expected_specification(arg_id);
   fmt::internal::dynamic_format_specs<char> specs{};
-  specs.align_ = fmt::alignment::ALIGN_LEFT;
-  specs.width_ = 10;
+  specs.align = fmt::align::left;
+  specs.width = 10;
   expected_specification.parsed_specs = specs;
 
   auto expected_substitution_part = format_part(expected_specification);