Parse printf format specs.

format.cc

@@ -605,7 +605,7 @@ void fmt::BasicWriter<Char>::FormatParser::CheckSign(
 
 template <typename Char>
 void fmt::BasicWriter<Char>::PrintfParser::ParseFlags(
-    FormatSpec &spec, const Char *&s, const ArgInfo &arg) {
+    FormatSpec &spec, const Char *&s) {
   for (;;) {
     switch (*s++) {
       case '-':
@@ -621,9 +621,7 @@ void fmt::BasicWriter<Char>::PrintfParser::ParseFlags(
         spec.flags_ |= SIGN_FLAG;
         break;
       case '#':
-        // TODO: handle floating-point args
+        spec.flags_ |= HASH_FLAG;
-        if (GetIntValue(arg) != 0)
-           spec.flags_ |= HASH_FLAG;
         break;
       default:
         --s;
@@ -632,26 +630,66 @@ void fmt::BasicWriter<Char>::PrintfParser::ParseFlags(
   }
 }
 
+template <typename Char>
+unsigned fmt::BasicWriter<Char>::PrintfParser::ParseHeader(
+  const Char *&s, FormatSpec &spec, const char *&error) {
+  unsigned arg_index = UINT_MAX;
+  Char c = *s;
+  if (c >= '0' && c <= '9') {
+    // Parse an argument index (if followed by '$') or a width possibly
+    // preceded with a '0' flag.
+    unsigned value = internal::ParseNonnegativeInt(s, error);
+    if (*s == '$') {  // value is an argument index
+      ++s;
+      arg_index = value;
+    } else {
+      if (c == '0')
+        spec.fill_ = '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;
+        return arg_index;
+      }
+    }
+  }
+  // Parse flags and width.
+  ParseFlags(spec, s);
+  if (*s >= '0' && *s <= '9') {
+    spec.width_ = internal::ParseNonnegativeInt(s, error);
+  } else if (*s == '*') {
+    ++s;
+    const ArgInfo &arg = HandleArgIndex(UINT_MAX, error);
+    if (arg.type <= LAST_INTEGER_TYPE)
+      spec.width_ = GetIntValue(arg);
+    else if (!error)
+      error = "width is not integer";
+  }
+  return arg_index;
+}
+
 // FIXME: this doesnt' depend on template argument
 template <typename Char>
-unsigned fmt::BasicWriter<Char>::PrintfParser::HandleArgIndex(
+const typename fmt::BasicWriter<Char>::ArgInfo
+  &fmt::BasicWriter<Char>::PrintfParser::HandleArgIndex(
     unsigned arg_index, const char *&error) {
   if (arg_index != UINT_MAX) {
     if (next_arg_index_ <= 0) {
       next_arg_index_ = -1;
-      return arg_index - 1;
+      --arg_index;
-    }
+    } else if (!error) {
-    if (!error)
       error = "cannot switch from automatic to manual argument indexing";
+    }
+  } else if (next_arg_index_ >= 0) {
+    arg_index = next_arg_index_++;
+  } else if (!error) {
+    error = "cannot switch from manual to automatic argument indexing";
   }
-  if (next_arg_index_ >= 0)
+  if (arg_index < num_args_)
-    return next_arg_index_++;
+    return args_[arg_index];
-  // Don't check if the error has already been set because the argument
+  if (!error)
-  // index is semantically the first part of the format string, so
+    error = "argument index is out of range in format";
-  // indexing errors are reported first even though parsing width
+  return DUMMY_ARG;
-  // above can cause another error.
-  error = "cannot switch from manual to automatic argument indexing";
-  return 0;
 }
 
 template <typename Char>
@@ -673,7 +711,7 @@ void fmt::BasicWriter<Char>::PrintfParser::Format(
     }
     writer.buffer_.append(start, s - 1);
 
-    FormatSpec spec(UINT_MAX);
+    FormatSpec spec;
     spec.align_ = ALIGN_RIGHT;
 
     // Reporting errors is delayed till the format specification is
@@ -689,63 +727,18 @@ void fmt::BasicWriter<Char>::PrintfParser::Format(
     const char *error = 0;
 
     c = *s;
-    unsigned arg_index = UINT_MAX;
+    const ArgInfo &arg = HandleArgIndex(ParseHeader(s, spec, error), error);
-    if (c >= '0' && c <= '9') {
+    if (spec.hash_flag() && GetIntValue(arg) == 0)
-      unsigned value = internal::ParseNonnegativeInt(s, error);
+      spec.flags_ &= ~HASH_FLAG;
-      if (*s != '$') {
+    if (spec.fill_ == '0') {
-        if (c == '0')
+      if (arg.type <= LAST_NUMERIC_TYPE)
-          spec.fill_ = '0';
+        spec.align_ = ALIGN_NUMERIC;
-        if (value != 0)
+      else
-          spec.width_ = value;
+        spec.fill_ = ' ';  // Ignore '0' flag for non-numeric types.
-      } else {
-        ++s;
-        arg_index = value;
-      }
-    } else if (c == '*') {
-      ++s;
-      const ArgInfo &arg = args_[HandleArgIndex(UINT_MAX, error)];
-      // TODO: check if arg is integer
-      spec.width_ = GetIntValue(arg);
-    }
-    arg_index = HandleArgIndex(arg_index, error);
-
-    // TODO: move to HandleArgIndex
-    const ArgInfo *arg = 0;
-    if (arg_index < num_args) {
-      arg = &args_[arg_index];
-    } else {
-      arg = &DUMMY_ARG;
-      if (!error)
-        error = "argument index is out of range in format";
-    }
-
-    int precision = -1;
-    switch (spec.width_) {
-    case UINT_MAX: {
-      spec.width_ = 0;
-      ParseFlags(spec, s, *arg);
-
-      // Parse width and zero flag.
-      if (*s >= '0' && *s <= '9') {
-        spec.width_ = internal::ParseNonnegativeInt(s, error);
-      } else if (*s == '*') {
-        ++s;
-        // TODO: parse arg index
-      } else
-        break;
-    }
-    // Fall through.
-    default:
-      if (spec.fill_ == '0') {
-        if (arg->type <= LAST_NUMERIC_TYPE)
-          spec.align_ = ALIGN_NUMERIC;
-        else
-          spec.fill_ = ' ';  // Ignore '0' flag for non-numeric types.
-      }
-      break;
     }
 
     // Parse precision.
+    int precision = -1;
     /*if (*s == '.') {
       ++s;
       precision = 0;
@@ -812,30 +805,30 @@ void fmt::BasicWriter<Char>::PrintfParser::Format(
     start = s;
 
     // Format argument.
-    switch (arg->type) {
+    switch (arg.type) {
     case INT:
-      writer.FormatInt(arg->int_value, spec);
+      writer.FormatInt(arg.int_value, spec);
       break;
     case UINT:
-      writer.FormatInt(arg->uint_value, spec);
+      writer.FormatInt(arg.uint_value, spec);
       break;
     case LONG:
-      writer.FormatInt(arg->long_value, spec);
+      writer.FormatInt(arg.long_value, spec);
       break;
     case ULONG:
-      writer.FormatInt(arg->ulong_value, spec);
+      writer.FormatInt(arg.ulong_value, spec);
       break;
     case LONG_LONG:
-      writer.FormatInt(arg->long_long_value, spec);
+      writer.FormatInt(arg.long_long_value, spec);
       break;
     case ULONG_LONG:
-      writer.FormatInt(arg->ulong_long_value, spec);
+      writer.FormatInt(arg.ulong_long_value, spec);
       break;
     case DOUBLE:
-      writer.FormatDouble(arg->double_value, spec, precision);
+      writer.FormatDouble(arg.double_value, spec, precision);
       break;
     case LONG_DOUBLE:
-      writer.FormatDouble(arg->long_double_value, spec, precision);
+      writer.FormatDouble(arg.long_double_value, spec, precision);
       break;
     case CHAR: {
       if (spec.type_ && spec.type_ != 'c')
@@ -856,14 +849,14 @@ void fmt::BasicWriter<Char>::PrintfParser::Format(
       } else {
         out = writer.GrowBuffer(1);
       }
-      *out = static_cast<Char>(arg->int_value);
+      *out = static_cast<Char>(arg.int_value);
       break;
     }
     case STRING: {
       if (spec.type_ && spec.type_ != 's')
         internal::ReportUnknownType(spec.type_, "string");
-      const Char *str = arg->string.value;
+      const Char *str = arg.string.value;
-      std::size_t size = arg->string.size;
+      std::size_t size = arg.string.size;
       if (size == 0) {
         if (!str)
           throw FormatError("string pointer is null");
@@ -878,12 +871,12 @@ void fmt::BasicWriter<Char>::PrintfParser::Format(
         internal::ReportUnknownType(spec.type_, "pointer");
       spec.flags_= HASH_FLAG;
       spec.type_ = 'x';
-      writer.FormatInt(reinterpret_cast<uintptr_t>(arg->pointer_value), spec);
+      writer.FormatInt(reinterpret_cast<uintptr_t>(arg.pointer_value), spec);
       break;
     case CUSTOM:
       if (spec.type_)
         internal::ReportUnknownType(spec.type_, "object");
-      arg->custom.format(writer, arg->custom.value, spec);
+      arg.custom.format(writer, arg.custom.value, spec);
       break;
     default:
       assert(false);

format.h

@@ -884,7 +884,9 @@ class BasicWriter {
 
   enum Type {
     // Numeric types should go first.
-    INT, UINT, LONG, ULONG, LONG_LONG, ULONG_LONG, DOUBLE, LONG_DOUBLE,
+    INT, UINT, LONG, ULONG, LONG_LONG, ULONG_LONG,
+    LAST_INTEGER_TYPE = ULONG_LONG,
+    DOUBLE, LONG_DOUBLE,
     LAST_NUMERIC_TYPE = LONG_DOUBLE,
     CHAR, STRING, WSTRING, POINTER, CUSTOM
   };
@@ -1043,8 +1045,13 @@ class BasicWriter {
     const ArgInfo *args_;
     int next_arg_index_;
 
-    unsigned HandleArgIndex(unsigned arg_index, const char *&error);
+    void ParseFlags(FormatSpec &spec, const Char *&s);
-    void ParseFlags(FormatSpec &spec, const Char *&s, const ArgInfo &arg);
+
+    // Parses argument index, flags and width and returns the parsed
+    // argument index.
+    unsigned ParseHeader(const Char *&s, FormatSpec &spec, const char *&error);
+
+    const ArgInfo &HandleArgIndex(unsigned arg_index, const char *&error);
 
    public:
     void Format(BasicWriter<Char> &writer,

test/printf-test.cc

@@ -96,7 +96,7 @@ TEST(PrintfTest, SwitchArgIndexing) {
   EXPECT_THROW_MSG(fmt::sprintf("%1$d%", 1, 2),
       FormatError, "invalid format string");
   EXPECT_THROW_MSG(fmt::sprintf(str(Format("%1$d%{}d", BIG_NUM)), 1, 2),
-      FormatError, "cannot switch from manual to automatic argument indexing");
+      FormatError, "number is too big in format");
   EXPECT_THROW_MSG(fmt::sprintf("%1$d%d", 1, 2),
       FormatError, "cannot switch from manual to automatic argument indexing");
 
@@ -109,9 +109,9 @@ TEST(PrintfTest, SwitchArgIndexing) {
 
   // Indexing errors override width errors.
   EXPECT_THROW_MSG(fmt::sprintf(str(Format("%d%1${}d", BIG_NUM)), 1, 2),
-      FormatError, "cannot switch from automatic to manual argument indexing");
+      FormatError, "number is too big in format");
   EXPECT_THROW_MSG(fmt::sprintf(str(Format("%1$d%{}d", BIG_NUM)), 1, 2),
-      FormatError, "cannot switch from manual to automatic argument indexing");
+      FormatError, "number is too big in format");
 }
 
 TEST(PrintfTest, InvalidArgIndex) {
@@ -135,7 +135,6 @@ TEST(PrintfTest, DefaultAlignRight) {
 
 TEST(PrintfTest, Width) {
   EXPECT_PRINTF("  abc", "%5s", "abc");
-  EXPECT_EQ("   42", str(fmt::sprintf("%*d", 5, 42)));
 
   // Width cannot be specified twice.
   EXPECT_THROW_MSG(fmt::sprintf("%5-5d", 42), FormatError,
@@ -147,6 +146,14 @@ TEST(PrintfTest, Width) {
       FormatError, "number is too big in format");
 }
 
+TEST(PrintfTest, DynamicWidth) {
+  EXPECT_EQ("   42", str(fmt::sprintf("%*d", 5, 42)));
+  EXPECT_THROW_MSG(fmt::sprintf("%*d", 5.0, 42), FormatError,
+      "width is not integer");
+  EXPECT_THROW_MSG(fmt::sprintf("%*d"), FormatError,
+      "argument index is out of range in format");
+}
+
 TEST(PrintfTest, ZeroFlag) {
   EXPECT_PRINTF("00042", "%05d", 42);
   EXPECT_PRINTF("-0042", "%05d", -42);