[llvm] d5d6b44 - [Support] Add automatic index assignment in formatv (#107459)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 12 04:38:40 PDT 2024
Author: Rahul Joshi
Date: 2024-09-12T04:38:36-07:00
New Revision: d5d6b447840f80a78047cf5ba769e4a09b44b83e
URL: https://github.com/llvm/llvm-project/commit/d5d6b447840f80a78047cf5ba769e4a09b44b83e
DIFF: https://github.com/llvm/llvm-project/commit/d5d6b447840f80a78047cf5ba769e4a09b44b83e.diff
LOG: [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.
Added:
Modified:
llvm/include/llvm/Support/FormatVariadic.h
llvm/lib/Support/FormatVariadic.cpp
llvm/unittests/Support/FormatVariadicTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/FormatVariadic.h b/llvm/include/llvm/Support/FormatVariadic.h
index 005d26f02d8fd9..d0e647e1403bdf 100644
--- a/llvm/include/llvm/Support/FormatVariadic.h
+++ b/llvm/include/llvm/Support/FormatVariadic.h
@@ -167,7 +167,7 @@ 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 "}">
@@ -175,8 +175,12 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
// 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
diff --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp
index 9056466190284b..3240b1c0f4e413 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -63,16 +63,18 @@ 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(":")) {
@@ -80,8 +82,10 @@ static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec) {
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;
}
diff --git a/llvm/unittests/Support/FormatVariadicTest.cpp b/llvm/unittests/Support/FormatVariadicTest.cpp
index 4f3d1791c0018d..68938480ecfe20 100644
--- a/llvm/unittests/Support/FormatVariadicTest.cpp
+++ b/llvm/unittests/Support/FormatVariadicTest.cpp
@@ -269,7 +269,7 @@ 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);
@@ -277,6 +277,42 @@ TEST(FormatVariadicTest, MultipleReplacements) {
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,6 +759,8 @@ 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
@@ -724,6 +768,7 @@ TEST(FormatVariadicTest, Validate) {
// 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
}
More information about the llvm-commits
mailing list