[llvm] 2f7ffba - [Support] Fix bugs in formatv automatic index assignment (#108384)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 16 21:48:09 PDT 2024
Author: Rahul Joshi
Date: 2024-09-16T21:48:05-07:00
New Revision: 2f7ffbaad3e7bd14b7a82d4887ef1640272ba1be
URL: https://github.com/llvm/llvm-project/commit/2f7ffbaad3e7bd14b7a82d4887ef1640272ba1be
DIFF: https://github.com/llvm/llvm-project/commit/2f7ffbaad3e7bd14b7a82d4887ef1640272ba1be.diff
LOG: [Support] Fix bugs in formatv automatic index assignment (#108384)
Fix bugs found when actually trying to use formatv() automatic index
assignment in IntrinsicEmitter.cpp:
- Assign automatic index only for `ReplacementType::Format`.
- Make the check for all replacement indices being either automatic orexplicit more accurate.
The existing check fails for formatv("{}{0}{}", 0, 1) (added as a unit test). Explicitly track if we
have seen any explicit and any automatic index instead.
Added:
Modified:
llvm/lib/Support/FormatVariadic.cpp
llvm/unittests/Support/FormatVariadicTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp
index 3240b1c0f4e413..7eb10887940513 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -148,6 +148,7 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
#if ENABLE_VALIDATION
const StringRef SavedFmtStr = Fmt;
unsigned NumExpectedArgs = 0;
+ bool HasExplicitIndex = false;
#endif
while (!Fmt.empty()) {
@@ -155,14 +156,17 @@ 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 (I->Type == ReplacementType::Format) {
+ if (I->Index == ~0U)
+ I->Index = NextAutomaticIndex++;
#if ENABLE_VALIDATION
- if (I->Type == ReplacementType::Format)
+ else
+ HasExplicitIndex = true;
NumExpectedArgs = std::max(NumExpectedArgs, I->Index + 1);
#endif
+ }
+
+ Replacements.emplace_back(*I);
}
#if ENABLE_VALIDATION
@@ -208,9 +212,8 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
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) {
+ // Fail validation if we see both automatic index and explicit index.
+ if (NextAutomaticIndex != 0 && HasExplicitIndex) {
errs() << formatv(
"Cannot mix automatic and explicit indices for format string '{}'\n",
SavedFmtStr);
diff --git a/llvm/unittests/Support/FormatVariadicTest.cpp b/llvm/unittests/Support/FormatVariadicTest.cpp
index 8e326e1f2b4396..e745f99a5a6c4e 100644
--- a/llvm/unittests/Support/FormatVariadicTest.cpp
+++ b/llvm/unittests/Support/FormatVariadicTest.cpp
@@ -283,6 +283,15 @@ TEST(FormatVariadicTest, AutomaticIndices) {
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
+ Replacements = parseFormatString("{}bar{}");
+ ASSERT_EQ(3u, Replacements.size());
+ EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
+ EXPECT_EQ(0u, Replacements[0].Index);
+ EXPECT_EQ(ReplacementType::Literal, Replacements[1].Type);
+ EXPECT_EQ("bar", Replacements[1].Spec);
+ EXPECT_EQ(ReplacementType::Format, Replacements[2].Type);
+ EXPECT_EQ(1u, Replacements[2].Index);
+
Replacements = parseFormatString("{}{}");
ASSERT_EQ(2u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -760,6 +769,8 @@ TEST(FormatVariadicTest, Validate) {
"Replacement field indices cannot have holes");
EXPECT_DEATH(formatv("{}{1}", 0, 1).str(),
"Cannot mix automatic and explicit indices");
+ EXPECT_DEATH(formatv("{}{0}{}", 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
@@ -768,6 +779,7 @@ TEST(FormatVariadicTest, Validate) {
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");
+ EXPECT_EQ(formatv("{}{0}{}", 0, 1).str(), "001");
#endif // NDEBUG
}
More information about the llvm-commits
mailing list