[Support] Add automatic index assignment in formatv (#107459)

Make index in replacement field optional. It will be automatically
assigned in incremental order by formatv.

Make mixed use of automatic and explicit indices an error that will fail
validation.
Adopt uses of formatv() within FormatVariadic to use automatic index.

Author: jurahul

llvm/include/llvm/Support/FormatVariadic.h

@@ -167,16 +167,20 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
 // Formats textual output.  `Fmt` is a string consisting of one or more
 // replacement sequences with the following grammar:
 //
-// rep_field ::= "{" index ["," layout] [":" format] "}"
+// rep_field ::= "{" [index] ["," layout] [":" format] "}"
 // index     ::= <non-negative integer>
 // layout    ::= [[[char]loc]width]
 // format    ::= <any string not containing "{" or "}">
 // char      ::= <any character except "{" or "}">
 // loc       ::= "-" | "=" | "+"
 // width     ::= <positive integer>
 //
-// index   - A non-negative integer specifying the index of the item in the
-//           parameter pack to print.  Any other value is invalid.
+// index   - An optional non-negative integer specifying the index of the item
+//           in the parameter pack to print. Any other value is invalid. If its
+//           not specified, it will be automatically assigned a value based on
+//           the order of rep_field seen in the format string. Note that mixing
+//           automatic and explicit index in the same call is an error and will
+//           fail validation in assert-enabled builds.
 // layout  - A string controlling how the field is laid out within the available
 //           space.
 // format  - A type-dependent string used to provide additional options to

llvm/lib/Support/FormatVariadic.cpp

@@ -63,25 +63,29 @@ static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec) {
   unsigned Align = 0;
   AlignStyle Where = AlignStyle::Right;
   StringRef Options;
-  unsigned Index = 0;
+  unsigned Index = ~0U;
   RepString = RepString.trim();
-  if (RepString.consumeInteger(0, Index)) {
-    assert(false && "Invalid replacement sequence index!");
-    return std::nullopt;
-  }
+
+  // If index is not specified, keep it ~0U to indicate unresolved index.
+  RepString.consumeInteger(0, Index);
   RepString = RepString.trim();
+
   if (RepString.consume_front(",")) {
-    if (!consumeFieldLayout(RepString, Where, Align, Pad))
+    if (!consumeFieldLayout(RepString, Where, Align, Pad)) {
       assert(false && "Invalid replacement field layout specification!");
+      return std::nullopt;
+    }
   }
   RepString = RepString.trim();
   if (RepString.consume_front(":")) {
     Options = RepString.trim();
     RepString = StringRef();
   }
   RepString = RepString.trim();
-  assert(RepString.empty() &&
-         "Unexpected characters found in replacement string!");
+  if (!RepString.empty()) {
+    assert(0 && "Unexpected characters found in replacement string!");
+    return std::nullopt;
+  }
 
   return ReplacementItem(Spec, Index, Align, Where, Pad, Options);
 }
@@ -139,6 +143,7 @@ SmallVector<ReplacementItem, 2>
 formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
                                        bool Validate) {
   SmallVector<ReplacementItem, 2> Replacements;
+  unsigned NextAutomaticIndex = 0;
 
 #if ENABLE_VALIDATION
   const StringRef SavedFmtStr = Fmt;
@@ -150,6 +155,9 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
     std::tie(I, Fmt) = splitLiteralAndReplacement(Fmt);
     if (!I)
       continue;
+    if (I->Index == ~0U)
+      I->Index = NextAutomaticIndex++;
+
     Replacements.emplace_back(*I);
 #if ENABLE_VALIDATION
     if (I->Type == ReplacementType::Format)
@@ -175,9 +183,8 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
   };
 
   if (NumExpectedArgs != NumArgs) {
-    errs() << formatv(
-        "Expected {0} Args, but got {1} for format string '{2}'\n",
-        NumExpectedArgs, NumArgs, SavedFmtStr);
+    errs() << formatv("Expected {} Args, but got {} for format string '{}'\n",
+                      NumExpectedArgs, NumArgs, SavedFmtStr);
     assert(0 && "Invalid formatv() call");
     return getErrorReplacements("Unexpected number of arguments");
   }
@@ -195,11 +202,21 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
 
   if (Count != NumExpectedArgs) {
     errs() << formatv(
-        "Replacement field indices cannot have holes for format string '{0}'\n",
+        "Replacement field indices cannot have holes for format string '{}'\n",
         SavedFmtStr);
     assert(0 && "Invalid format string");
     return getErrorReplacements("Replacement indices have holes");
   }
+
+  // If we had automatic numbering of replacement indices, verify that all
+  // indices used automatic numbering.
+  if (NextAutomaticIndex != 0 && NextAutomaticIndex != Count) {
+    errs() << formatv(
+        "Cannot mix automatic and explicit indices for format string '{}'\n",
+        SavedFmtStr);
+    assert(0 && "Invalid format string");
+    return getErrorReplacements("Cannot mix automatic and explicit indices");
+  }
 #endif // ENABLE_VALIDATION
   return Replacements;
 }

llvm/unittests/Support/FormatVariadicTest.cpp

@@ -269,14 +269,50 @@ TEST(FormatVariadicTest, MultipleReplacements) {
   EXPECT_EQ(ReplacementType::Literal, Replacements[3].Type);
   EXPECT_EQ("-", Replacements[3].Spec);
 
-  // {2:bar,-3} - Options=bar, Align=-3
+  // {2,-3:bar} - Options=bar, Align=-3
   EXPECT_EQ(ReplacementType::Format, Replacements[4].Type);
   EXPECT_EQ(2u, Replacements[4].Index);
   EXPECT_EQ(3u, Replacements[4].Width);
   EXPECT_EQ(AlignStyle::Left, Replacements[4].Where);
   EXPECT_EQ("bar", Replacements[4].Options);
 }
 
+TEST(FormatVariadicTest, AutomaticIndices) {
+  auto Replacements = parseFormatString("{}");
+  ASSERT_EQ(1u, Replacements.size());
+  EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
+  EXPECT_EQ(0u, Replacements[0].Index);
+
+  Replacements = parseFormatString("{}{}");
+  ASSERT_EQ(2u, Replacements.size());
+  EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(ReplacementType::Format, Replacements[1].Type);
+  EXPECT_EQ(1u, Replacements[1].Index);
+
+  Replacements = parseFormatString("{}{:foo}{,-3:bar}");
+  ASSERT_EQ(3u, Replacements.size());
+  EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(0u, Replacements[0].Width);
+  EXPECT_EQ(AlignStyle::Right, Replacements[0].Where);
+  EXPECT_EQ("", Replacements[0].Options);
+
+  // {:foo} - Options=foo
+  EXPECT_EQ(ReplacementType::Format, Replacements[1].Type);
+  EXPECT_EQ(1u, Replacements[1].Index);
+  EXPECT_EQ(0u, Replacements[1].Width);
+  EXPECT_EQ(AlignStyle::Right, Replacements[1].Where);
+  EXPECT_EQ("foo", Replacements[1].Options);
+
+  // {,-3:bar} - Options=bar, Align=-3
+  EXPECT_EQ(ReplacementType::Format, Replacements[2].Type);
+  EXPECT_EQ(2u, Replacements[2].Index);
+  EXPECT_EQ(3u, Replacements[2].Width);
+  EXPECT_EQ(AlignStyle::Left, Replacements[2].Where);
+  EXPECT_EQ("bar", Replacements[2].Options);
+}
+
 TEST(FormatVariadicTest, FormatNoReplacements) {
   EXPECT_EQ("", formatv("").str());
   EXPECT_EQ("Test", formatv("Test").str());
@@ -291,6 +327,12 @@ TEST(FormatVariadicTest, FormatBasicTypesOneReplacement) {
   EXPECT_EQ("Test3", formatv("{0}", std::string("Test3")).str());
 }
 
+TEST(FormatVariadicTest, FormatAutomaticIndices) {
+  EXPECT_EQ("1", formatv("{}", 1).str());
+  EXPECT_EQ("c1", formatv("{}{}", 'c', 1).str());
+  EXPECT_EQ("c-1rrr-0xFF", formatv("{}-{,r-4}-{:X}", 'c', 1, 255).str());
+}
+
 TEST(FormatVariadicTest, IntegralHexFormatting) {
   // 1. Trivial cases.  Make sure hex is not the default.
   EXPECT_EQ("0", formatv("{0}", 0).str());
@@ -717,13 +759,16 @@ TEST(FormatVariadicTest, Validate) {
   EXPECT_DEATH(formatv("{0}", 1, 2).str(), "Expected 1 Args, but got 2");
   EXPECT_DEATH(formatv("{0} {2}", 1, 2, 3).str(),
                "Replacement field indices cannot have holes");
+  EXPECT_DEATH(formatv("{}{1}", 0, 1).str(),
+               "Cannot mix automatic and explicit indices");
 #else  // GTEST_HAS_DEATH_TEST
   GTEST_SKIP() << "No support for EXPECT_DEATH";
 #endif // GTEST_HAS_DEATH_TEST
 #else  // NDEBUG
   // If asserts are disabled, verify that validation is disabled.
   EXPECT_EQ(formatv("{0}", 1, 2).str(), "1");
   EXPECT_EQ(formatv("{0} {2}", 1, 2, 3).str(), "1 3");
+  EXPECT_EQ(formatv("{}{1}", 0, 1).str(), "01");
 #endif // NDEBUG
 }