Skip to content

Commit 82bafe9

Browse files
committed
[Support] Fix bugs in formatv automatic index assignment
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 or explicit more accurate. The existing check fails for format formatv("{}{0}{}", 0, 1) (added as a unit test). Explicitly track if we have seen any explicit and any automatic index instead.
1 parent 36ad072 commit 82bafe9

File tree

2 files changed

+23
-8
lines changed

2 files changed

+23
-8
lines changed

llvm/lib/Support/FormatVariadic.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
144144
bool Validate) {
145145
SmallVector<ReplacementItem, 2> Replacements;
146146
unsigned NextAutomaticIndex = 0;
147+
bool HasExplicitIndex = false;
147148

148149
#if ENABLE_VALIDATION
149150
const StringRef SavedFmtStr = Fmt;
@@ -155,14 +156,17 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
155156
std::tie(I, Fmt) = splitLiteralAndReplacement(Fmt);
156157
if (!I)
157158
continue;
158-
if (I->Index == ~0U)
159-
I->Index = NextAutomaticIndex++;
160-
161-
Replacements.emplace_back(*I);
159+
if (I->Type == ReplacementType::Format) {
160+
if (I->Index == ~0U)
161+
I->Index = NextAutomaticIndex++;
162+
else
163+
HasExplicitIndex = true;
162164
#if ENABLE_VALIDATION
163-
if (I->Type == ReplacementType::Format)
164165
NumExpectedArgs = std::max(NumExpectedArgs, I->Index + 1);
165166
#endif
167+
}
168+
169+
Replacements.emplace_back(*I);
166170
}
167171

168172
#if ENABLE_VALIDATION
@@ -208,9 +212,8 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
208212
return getErrorReplacements("Replacement indices have holes");
209213
}
210214

211-
// If we had automatic numbering of replacement indices, verify that all
212-
// indices used automatic numbering.
213-
if (NextAutomaticIndex != 0 && NextAutomaticIndex != Count) {
215+
// Fail validation if we see both automatic index and explicit index.
216+
if (NextAutomaticIndex != 0 && HasExplicitIndex) {
214217
errs() << formatv(
215218
"Cannot mix automatic and explicit indices for format string '{}'\n",
216219
SavedFmtStr);

llvm/unittests/Support/FormatVariadicTest.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,15 @@ TEST(FormatVariadicTest, AutomaticIndices) {
283283
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
284284
EXPECT_EQ(0u, Replacements[0].Index);
285285

286+
Replacements = parseFormatString("{}bar{}");
287+
ASSERT_EQ(3u, Replacements.size());
288+
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
289+
EXPECT_EQ(0u, Replacements[0].Index);
290+
EXPECT_EQ(ReplacementType::Literal, Replacements[1].Type);
291+
EXPECT_EQ("bar", Replacements[1].Spec);
292+
EXPECT_EQ(ReplacementType::Format, Replacements[2].Type);
293+
EXPECT_EQ(1u, Replacements[2].Index);
294+
286295
Replacements = parseFormatString("{}{}");
287296
ASSERT_EQ(2u, Replacements.size());
288297
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -761,6 +770,8 @@ TEST(FormatVariadicTest, Validate) {
761770
"Replacement field indices cannot have holes");
762771
EXPECT_DEATH(formatv("{}{1}", 0, 1).str(),
763772
"Cannot mix automatic and explicit indices");
773+
EXPECT_DEATH(formatv("{}{0}{}", 0, 1).str(),
774+
"Cannot mix automatic and explicit indices");
764775
#else // GTEST_HAS_DEATH_TEST
765776
GTEST_SKIP() << "No support for EXPECT_DEATH";
766777
#endif // GTEST_HAS_DEATH_TEST
@@ -769,6 +780,7 @@ TEST(FormatVariadicTest, Validate) {
769780
EXPECT_EQ(formatv("{0}", 1, 2).str(), "1");
770781
EXPECT_EQ(formatv("{0} {2}", 1, 2, 3).str(), "1 3");
771782
EXPECT_EQ(formatv("{}{1}", 0, 1).str(), "01");
783+
EXPECT_EQ(formatv("{}{0}{}", 0, 1).str(), "001");
772784
#endif // NDEBUG
773785
}
774786

0 commit comments

Comments
 (0)