[clang] [lldb] [llvm] [mlir] [Support] Validate number of arguments passed to formatv() (PR #105745)
Rahul Joshi via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 28 11:40:18 PDT 2024
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/105745
>From d805dcdc44d22bb21d086b186cc7f644323f68ef Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Thu, 22 Aug 2024 08:47:02 -0700
Subject: [PATCH] [Support] Detect invalid formatv() calls
- Detect formatv() calls where the number of replacement parameters
expected after parsing the format string does not match the number
provides in the formatv() call.
- assert() in debug builds, and fail the formatv() call in release
builds by just emitting an error message in the stream.
---
.../Checkers/CheckPlacementNew.cpp | 2 +-
.../Checkers/StdLibraryFunctionsChecker.cpp | 2 +-
.../Plugins/SymbolFile/DWARF/DWARFUnit.cpp | 2 +-
llvm/benchmarks/CMakeLists.txt | 1 +
llvm/benchmarks/FormatVariadicBM.cpp | 53 ++++++++++++
llvm/include/llvm/Support/FormatVariadic.h | 44 ++++++----
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 3 +-
llvm/lib/Support/FormatVariadic.cpp | 84 ++++++++++++++++---
.../tools/llvm-pdbutil/ExplainOutputStyle.cpp | 4 +-
llvm/unittests/Support/FormatVariadicTest.cpp | 68 +++++++++------
llvm/utils/TableGen/IntrinsicEmitter.cpp | 4 +-
.../mlir-linalg-ods-yaml-gen.cpp | 4 +-
.../tools/mlir-tblgen/LLVMIRConversionGen.cpp | 2 +-
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 13 ++-
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 14 ++--
mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp | 3 +-
16 files changed, 218 insertions(+), 85 deletions(-)
create mode 100644 llvm/benchmarks/FormatVariadicBM.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
index 99e11a15c08dc2..1b89951397cfb1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -131,7 +131,7 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
"Storage provided to placement new is only {0} bytes, "
"whereas the allocated array type requires more space for "
"internal needs",
- SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
+ SizeOfPlaceCI->getValue()));
else
Msg = std::string(llvm::formatv(
"Storage provided to placement new is only {0} bytes, "
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 8f4bd17afc8581..60c035612dcd44 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1401,7 +1401,7 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
ErrnoNote =
llvm::formatv("After calling '{0}' {1}", FunctionName, ErrnoNote);
} else {
- CaseNote = llvm::formatv(Case.getNote().str().c_str(), FunctionName);
+ CaseNote = llvm::formatvv(Case.getNote().str().c_str(), FunctionName);
}
const SVal RV = Call.getReturnValue();
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index d1d91134b6237c..0eb882b0e7d4f5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -655,7 +655,7 @@ DWARFUnit::GetDIE(dw_offset_t die_offset) {
if (!ContainsDIEOffset(die_offset)) {
GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
- "GetDIE for DIE {0:x16} is outside of its CU {0:x16}", die_offset,
+ "GetDIE for DIE {0:x16} is outside of its CU {1:x16}", die_offset,
GetOffset());
return DWARFDIE(); // Not found
}
diff --git a/llvm/benchmarks/CMakeLists.txt b/llvm/benchmarks/CMakeLists.txt
index 713d4ccd3c5975..e3366e6f3ffe19 100644
--- a/llvm/benchmarks/CMakeLists.txt
+++ b/llvm/benchmarks/CMakeLists.txt
@@ -5,3 +5,4 @@ set(LLVM_LINK_COMPONENTS
add_benchmark(DummyYAML DummyYAML.cpp PARTIAL_SOURCES_INTENDED)
add_benchmark(xxhash xxhash.cpp PARTIAL_SOURCES_INTENDED)
add_benchmark(GetIntrinsicForClangBuiltin GetIntrinsicForClangBuiltin.cpp PARTIAL_SOURCES_INTENDED)
+add_benchmark(FormatVariadicBM FormatVariadicBM.cpp PARTIAL_SOURCES_INTENDED)
diff --git a/llvm/benchmarks/FormatVariadicBM.cpp b/llvm/benchmarks/FormatVariadicBM.cpp
new file mode 100644
index 00000000000000..e9916344f9bb67
--- /dev/null
+++ b/llvm/benchmarks/FormatVariadicBM.cpp
@@ -0,0 +1,53 @@
+#include "benchmark/benchmark.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <algorithm>
+#include <string>
+#include <vector>
+
+using namespace llvm;
+using namespace std;
+
+// Generate a list of format strings that have `NumReplacements` replacements.
+static vector<string> getFormatStrings(int NumReplacements) {
+ vector<string> Components;
+ for (int I = 0; I < NumReplacements; I++)
+ Components.push_back("{" + to_string(I) + "}");
+ // Intersperse these with some other literal text (_).
+ const string_view Literal = "____";
+ for (char C : Literal)
+ Components.push_back(string(1, C));
+
+ vector<string> Formats;
+ do {
+ string Concat;
+ for (const string &C : Components)
+ Concat += C;
+ Formats.emplace_back(Concat);
+ } while (next_permutation(Components.begin(), Components.end()));
+ return Formats;
+}
+
+static const vector<vector<string>> Formats = {
+ getFormatStrings(1), getFormatStrings(2), getFormatStrings(3),
+ getFormatStrings(4), getFormatStrings(5),
+};
+
+// Benchmark formatv() for a variety of format strings and 1-5 replacements.
+static void BM_FormatVariadic(benchmark::State &state) {
+ for (auto _ : state) {
+ for (const string &Fmt : Formats[0])
+ formatv(Fmt.c_str(), 1).str();
+ for (const string &Fmt : Formats[1])
+ formatv(Fmt.c_str(), 1, 2).str();
+ for (const string &Fmt : Formats[2])
+ formatv(Fmt.c_str(), 1, 2, 3).str();
+ for (const string &Fmt : Formats[3])
+ formatv(Fmt.c_str(), 1, 2, 3, 4).str();
+ for (const string &Fmt : Formats[4])
+ formatv(Fmt.c_str(), 1, 2, 3, 4, 5).str();
+ }
+}
+
+BENCHMARK(BM_FormatVariadic);
+
+BENCHMARK_MAIN();
diff --git a/llvm/include/llvm/Support/FormatVariadic.h b/llvm/include/llvm/Support/FormatVariadic.h
index 595f2cf559a428..59360751e026ff 100644
--- a/llvm/include/llvm/Support/FormatVariadic.h
+++ b/llvm/include/llvm/Support/FormatVariadic.h
@@ -67,23 +67,20 @@ class formatv_object_base {
protected:
StringRef Fmt;
ArrayRef<support::detail::format_adapter *> Adapters;
-
- static bool consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
- size_t &Align, char &Pad);
-
- static std::pair<ReplacementItem, StringRef>
- splitLiteralAndReplacement(StringRef Fmt);
+ bool Validate;
formatv_object_base(StringRef Fmt,
- ArrayRef<support::detail::format_adapter *> Adapters)
- : Fmt(Fmt), Adapters(Adapters) {}
+ ArrayRef<support::detail::format_adapter *> Adapters,
+ bool Validate)
+ : Fmt(Fmt), Adapters(Adapters), Validate(Validate) {}
formatv_object_base(formatv_object_base const &rhs) = delete;
formatv_object_base(formatv_object_base &&rhs) = default;
public:
void format(raw_ostream &S) const {
- for (auto &R : parseFormatString(Fmt)) {
+ const auto Replacements = parseFormatString(Fmt, Adapters.size(), Validate);
+ for (const auto &R : Replacements) {
if (R.Type == ReplacementType::Empty)
continue;
if (R.Type == ReplacementType::Literal) {
@@ -101,9 +98,10 @@ class formatv_object_base {
Align.format(S, R.Options);
}
}
- static SmallVector<ReplacementItem, 2> parseFormatString(StringRef Fmt);
- static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec);
+ // Parse and optionally validate format string (in debug builds).
+ static SmallVector<ReplacementItem, 2>
+ parseFormatString(StringRef Fmt, size_t NumArgs, bool Validate);
std::string str() const {
std::string Result;
@@ -149,8 +147,8 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
};
public:
- formatv_object(StringRef Fmt, Tuple &&Params)
- : formatv_object_base(Fmt, ParameterPointers),
+ formatv_object(StringRef Fmt, Tuple &&Params, bool Validate)
+ : formatv_object_base(Fmt, ParameterPointers, Validate),
Parameters(std::move(Params)) {
ParameterPointers = std::apply(create_adapters(), Parameters);
}
@@ -247,15 +245,27 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
// assertion. Otherwise, it will try to do something reasonable, but in general
// the details of what that is are undefined.
//
+
+// formatv() with validation enable/disable controlled by the first argument.
template <typename... Ts>
-inline auto formatv(const char *Fmt, Ts &&...Vals)
+inline auto formatv(bool Validate, const char *Fmt, Ts &&...Vals)
-> formatv_object<decltype(std::make_tuple(
support::detail::build_format_adapter(std::forward<Ts>(Vals))...))> {
using ParamTuple = decltype(std::make_tuple(
support::detail::build_format_adapter(std::forward<Ts>(Vals))...));
- return formatv_object<ParamTuple>(
- Fmt, std::make_tuple(support::detail::build_format_adapter(
- std::forward<Ts>(Vals))...));
+ auto Params = std::make_tuple(
+ support::detail::build_format_adapter(std::forward<Ts>(Vals))...);
+ return formatv_object<ParamTuple>(Fmt, std::move(Params), Validate);
+}
+
+// formatv() with validation enabled.
+template <typename... Ts> inline auto formatv(const char *Fmt, Ts &&...Vals) {
+ return formatv<Ts...>(true, Fmt, std::forward<Ts>(Vals)...);
+}
+
+// formatvv() has validation disabled.
+template <typename... Ts> inline auto formatvv(const char *Fmt, Ts &&...Vals) {
+ return formatv<Ts...>(false, Fmt, std::forward<Ts>(Vals)...);
}
} // end namespace llvm
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 77e8ece9439cf9..eb2751ab30ac50 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1518,8 +1518,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
error() << formatv("NameIndex @ {0:x}: Indexing multiple compile units "
"and abbreviation {1:x} has no DW_IDX_compile_unit "
"or DW_IDX_type_unit attribute.\n",
- NI.getUnitOffset(), Abbrev.Code,
- dwarf::DW_IDX_compile_unit);
+ NI.getUnitOffset(), Abbrev.Code);
});
++NumErrors;
}
diff --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp
index e25d036cdf1e8c..5bea91229c20d8 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -25,8 +25,8 @@ static std::optional<AlignStyle> translateLocChar(char C) {
LLVM_BUILTIN_UNREACHABLE;
}
-bool formatv_object_base::consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
- size_t &Align, char &Pad) {
+static bool consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
+ size_t &Align, char &Pad) {
Where = AlignStyle::Right;
Align = 0;
Pad = ' ';
@@ -35,8 +35,7 @@ bool formatv_object_base::consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
if (Spec.size() > 1) {
// A maximum of 2 characters at the beginning can be used for something
- // other
- // than the width.
+ // other than the width.
// If Spec[1] is a loc char, then Spec[0] is a pad char and Spec[2:...]
// contains the width.
// Otherwise, if Spec[0] is a loc char, then Spec[1:...] contains the width.
@@ -55,8 +54,7 @@ bool formatv_object_base::consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
return !Failed;
}
-std::optional<ReplacementItem>
-formatv_object_base::parseReplacementItem(StringRef Spec) {
+static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec) {
StringRef RepString = Spec.trim("{}");
// If the replacement sequence does not start with a non-negative integer,
@@ -82,15 +80,14 @@ formatv_object_base::parseReplacementItem(StringRef Spec) {
RepString = StringRef();
}
RepString = RepString.trim();
- if (!RepString.empty()) {
- assert(false && "Unexpected characters found in replacement string!");
- }
+ assert(RepString.empty() &&
+ "Unexpected characters found in replacement string!");
return ReplacementItem{Spec, Index, Align, Where, Pad, Options};
}
-std::pair<ReplacementItem, StringRef>
-formatv_object_base::splitLiteralAndReplacement(StringRef Fmt) {
+static std::pair<ReplacementItem, StringRef>
+splitLiteralAndReplacement(StringRef Fmt) {
while (!Fmt.empty()) {
// Everything up until the first brace is a literal.
if (Fmt.front() != '{') {
@@ -143,15 +140,76 @@ formatv_object_base::splitLiteralAndReplacement(StringRef Fmt) {
return std::make_pair(ReplacementItem{Fmt}, StringRef());
}
+#ifndef NDEBUG
+#define ENABLE_VALIDATION 1
+#else
+#define ENABLE_VALIDATION 1 // Convienently enable validation in release mode.
+#endif
+
SmallVector<ReplacementItem, 2>
-formatv_object_base::parseFormatString(StringRef Fmt) {
+formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs,
+ bool Validate) {
SmallVector<ReplacementItem, 2> Replacements;
- ReplacementItem I;
+
+#if ENABLE_VALIDATION
+ const StringRef SavedFmtStr = Fmt;
+ size_t NumExpectedArgs = 0;
+#endif
+
while (!Fmt.empty()) {
+ ReplacementItem I;
std::tie(I, Fmt) = splitLiteralAndReplacement(Fmt);
if (I.Type != ReplacementType::Empty)
Replacements.push_back(I);
+#if ENABLE_VALIDATION
+ if (I.Type == ReplacementType::Format)
+ NumExpectedArgs = std::max(NumExpectedArgs, I.Index + 1);
+#endif
+ }
+
+#if ENABLE_VALIDATION
+ if (!Validate)
+ return Replacements;
+
+ // Perform additional validation. Verify that the number of arguments matches
+ // the number of replacement indices and that there are no holes in the
+ // replacement indexes.
+
+ // When validation fails, return an array of replacement items that
+ // will print an error message as the outout of this formatv().
+ auto getErrorReplacements = [SavedFmtStr](StringLiteral ErrorMsg) {
+ return SmallVector<ReplacementItem, 2>{
+ ReplacementItem("Invalid formatv() call: "), ReplacementItem(ErrorMsg),
+ ReplacementItem(" for format string: "), ReplacementItem(SavedFmtStr)};
+ };
+
+ if (NumExpectedArgs != NumArgs) {
+ errs() << formatv(
+ "Expected {0} Args, but got {1} for format string '{2}'\n",
+ NumExpectedArgs, NumArgs, SavedFmtStr);
+ assert(0 && "Invalid formatv() call");
+ return getErrorReplacements("Unexpected number of arguments");
+ }
+
+ // Find the number of unique indices seen. All replacement indices
+ // are < NumExpectedArgs.
+ SmallVector<bool> Indices(NumExpectedArgs);
+ size_t Count = 0;
+ for (const ReplacementItem &I : Replacements) {
+ if (I.Type != ReplacementType::Format || Indices[I.Index])
+ continue;
+ Indices[I.Index] = true;
+ ++Count;
+ }
+
+ if (Count != NumExpectedArgs) {
+ errs() << formatv(
+ "Replacement field indices cannot have holes for format string '{0}'\n",
+ SavedFmtStr);
+ assert(0 && "Invalid format string");
+ return getErrorReplacements("Replacement indices have holes");
}
+#endif // ENABLE_VALIDATION
return Replacements;
}
diff --git a/llvm/tools/llvm-pdbutil/ExplainOutputStyle.cpp b/llvm/tools/llvm-pdbutil/ExplainOutputStyle.cpp
index ce9d659335b30b..200e9037de3cbe 100644
--- a/llvm/tools/llvm-pdbutil/ExplainOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbutil/ExplainOutputStyle.cpp
@@ -139,8 +139,8 @@ bool ExplainOutputStyle::explainPdbBlockStatus() {
FileOffset, File.pdb().getFileSize());
return false;
}
- P.formatLine("Block:Offset = {2:X-}:{1:X-4}.", FileOffset, pdbBlockOffset(),
- pdbBlockIndex());
+ P.formatLine("Block:Offset = {0:X-}:{1:X-4}.", pdbBlockIndex(),
+ pdbBlockOffset());
bool IsFree = File.pdb().getMsfLayout().FreePageMap[pdbBlockIndex()];
P.formatLine("Address is in block {0} ({1}allocated).", pdbBlockIndex(),
diff --git a/llvm/unittests/Support/FormatVariadicTest.cpp b/llvm/unittests/Support/FormatVariadicTest.cpp
index a78b25c53d7e43..4f77b2d2210ca0 100644
--- a/llvm/unittests/Support/FormatVariadicTest.cpp
+++ b/llvm/unittests/Support/FormatVariadicTest.cpp
@@ -9,9 +9,11 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatAdapters.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
using namespace llvm;
+using ::testing::HasSubstr;
// Compile-time tests templates in the detail namespace.
namespace {
@@ -35,14 +37,19 @@ struct NoFormat {};
static_assert(uses_missing_provider<NoFormat>::value, "");
}
+// Helper to parse format string with no validation.
+static SmallVector<ReplacementItem, 2> parseFormatString(StringRef Fmt) {
+ return formatv_object_base::parseFormatString(Fmt, 0, false);
+}
+
TEST(FormatVariadicTest, EmptyFormatString) {
- auto Replacements = formatv_object_base::parseFormatString("");
+ auto Replacements = parseFormatString("");
EXPECT_EQ(0U, Replacements.size());
}
TEST(FormatVariadicTest, NoReplacements) {
const StringRef kFormatString = "This is a test";
- auto Replacements = formatv_object_base::parseFormatString(kFormatString);
+ auto Replacements = parseFormatString(kFormatString);
ASSERT_EQ(1U, Replacements.size());
EXPECT_EQ(kFormatString, Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
@@ -50,25 +57,25 @@ TEST(FormatVariadicTest, NoReplacements) {
TEST(FormatVariadicTest, EscapedBrace) {
// {{ should be replaced with {
- auto Replacements = formatv_object_base::parseFormatString("{{");
+ auto Replacements = parseFormatString("{{");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("{", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
// An even number N of braces should be replaced with N/2 braces.
- Replacements = formatv_object_base::parseFormatString("{{{{{{");
+ Replacements = parseFormatString("{{{{{{");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("{{{", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
// } does not require doubling up.
- Replacements = formatv_object_base::parseFormatString("}");
+ Replacements = parseFormatString("}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("}", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
// } does not require doubling up.
- Replacements = formatv_object_base::parseFormatString("}}}");
+ Replacements = parseFormatString("}}}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("}}}", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
@@ -76,14 +83,14 @@ TEST(FormatVariadicTest, EscapedBrace) {
TEST(FormatVariadicTest, ValidReplacementSequence) {
// 1. Simple replacement - parameter index only
- auto Replacements = formatv_object_base::parseFormatString("{0}");
+ auto Replacements = parseFormatString("{0}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
EXPECT_EQ(0u, Replacements[0].Align);
EXPECT_EQ("", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{1}");
+ Replacements = parseFormatString("{1}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(1u, Replacements[0].Index);
@@ -92,7 +99,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 2. Parameter index with right alignment
- Replacements = formatv_object_base::parseFormatString("{0,3}");
+ Replacements = parseFormatString("{0,3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -101,7 +108,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 3. And left alignment
- Replacements = formatv_object_base::parseFormatString("{0,-3}");
+ Replacements = parseFormatString("{0,-3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -110,7 +117,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 4. And center alignment
- Replacements = formatv_object_base::parseFormatString("{0,=3}");
+ Replacements = parseFormatString("{0,=3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -119,7 +126,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 4. Parameter index with option string
- Replacements = formatv_object_base::parseFormatString("{0:foo}");
+ Replacements = parseFormatString("{0:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -128,7 +135,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("foo", Replacements[0].Options);
// 5. Parameter index with alignment before option string
- Replacements = formatv_object_base::parseFormatString("{0,-3:foo}");
+ Replacements = parseFormatString("{0,-3:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -137,7 +144,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("foo", Replacements[0].Options);
// 7. Parameter indices, options, and alignment can all have whitespace.
- Replacements = formatv_object_base::parseFormatString("{ 0, -3 : foo }");
+ Replacements = parseFormatString("{ 0, -3 : foo }");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -147,7 +154,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
// 8. Everything after the first option specifier is part of the style, even
// if it contains another option specifier.
- Replacements = formatv_object_base::parseFormatString("{0:0:1}");
+ Replacements = parseFormatString("{0:0:1}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("0:0:1", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -157,7 +164,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("0:1", Replacements[0].Options);
// 9. Custom padding character
- Replacements = formatv_object_base::parseFormatString("{0,p+4:foo}");
+ Replacements = parseFormatString("{0,p+4:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("0,p+4:foo", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -168,7 +175,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("foo", Replacements[0].Options);
// Format string special characters are allowed as padding character
- Replacements = formatv_object_base::parseFormatString("{0,-+4:foo}");
+ Replacements = parseFormatString("{0,-+4:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("0,-+4:foo", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -178,7 +185,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('-', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{0,+-4:foo}");
+ Replacements = parseFormatString("{0,+-4:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("0,+-4:foo", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -188,7 +195,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('+', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{0,==4:foo}");
+ Replacements = parseFormatString("{0,==4:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("0,==4:foo", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -198,7 +205,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('=', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{0,:=4:foo}");
+ Replacements = parseFormatString("{0,:=4:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("0,:=4:foo", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -211,7 +218,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
TEST(FormatVariadicTest, DefaultReplacementValues) {
// 2. If options string is missing, it defaults to empty.
- auto Replacements = formatv_object_base::parseFormatString("{0,3}");
+ auto Replacements = parseFormatString("{0,3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -219,7 +226,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
EXPECT_EQ("", Replacements[0].Options);
// Including if the colon is present but contains no text.
- Replacements = formatv_object_base::parseFormatString("{0,3:}");
+ Replacements = parseFormatString("{0,3:}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -227,7 +234,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
EXPECT_EQ("", Replacements[0].Options);
// 3. If alignment is missing, it defaults to 0, right, space
- Replacements = formatv_object_base::parseFormatString("{0:foo}");
+ Replacements = parseFormatString("{0:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(AlignStyle::Right, Replacements[0].Where);
@@ -238,8 +245,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
}
TEST(FormatVariadicTest, MultipleReplacements) {
- auto Replacements =
- formatv_object_base::parseFormatString("{0} {1:foo}-{2,-3:bar}");
+ auto Replacements = parseFormatString("{0} {1:foo}-{2,-3:bar}");
ASSERT_EQ(5u, Replacements.size());
// {0}
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
@@ -500,7 +506,7 @@ struct format_tuple {
explicit format_tuple(const char *Fmt) : Fmt(Fmt) {}
template <typename... Ts> auto operator()(Ts &&... Values) const {
- return formatv(Fmt, std::forward<Ts>(Values)...);
+ return formatvv(Fmt, std::forward<Ts>(Values)...);
}
};
@@ -704,6 +710,16 @@ TEST(FormatVariadicTest, FormatFilterRange) {
EXPECT_EQ("1, 2, 3", formatv("{0}", Range).str());
}
+#ifdef NDEBUG // Disable the test in debug builds where it will assert.
+TEST(FormatVariadicTest, Validate) {
+ std::string Str = formatv("{0}", 1, 2).str();
+ EXPECT_THAT(Str, HasSubstr("Unexpected number of arguments"));
+
+ Str = formatv("{0} {2}", 1, 2, 3).str();
+ EXPECT_THAT(Str, HasSubstr("eplacement indices have holes"));
+}
+#endif // NDEBUG
+
namespace {
enum class Base { First };
diff --git a/llvm/utils/TableGen/IntrinsicEmitter.cpp b/llvm/utils/TableGen/IntrinsicEmitter.cpp
index 4c211cdca84c5f..af8ff889918428 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -735,8 +735,8 @@ Intrinsic::getIntrinsicFor{1}Builtin(StringRef TargetPrefix,
const auto &[Map, CommonPrefix] = Entry;
if (TargetPrefix.empty())
continue;
- OS << formatv(R"( {{"{0}", {0}Names, "{2}"},)", TargetPrefix,
- TargetPrefix, CommonPrefix)
+ OS << formatv(R"( {{"{0}", {0}Names, "{1}"},)", TargetPrefix,
+ CommonPrefix)
<< "\n";
}
OS << " };\n";
diff --git a/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp b/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
index 7311cdd39d0755..a00f12661f7120 100644
--- a/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
+++ b/mlir/tools/mlir-linalg-ods-gen/mlir-linalg-ods-yaml-gen.cpp
@@ -634,7 +634,7 @@ ArrayAttr {0}::getIndexingMaps() {{
MLIRContext *context = getContext();
auto symbolBindings = getSymbolBindings(*this);
SmallVector<AffineMap> maps;
- {2}
+ {1}
cached = Builder(context).getAffineMapArrayAttr(maps);
getOperation()->setAttr(memoizeAttr, cached);
return cached;
@@ -929,7 +929,7 @@ exprs.push_back(getAffineConstantExpr(cst{1}, context));
// TODO: This needs to be memoized and/or converted to non-parser based
// C++ codegen prior to real use.
os << llvm::formatv(structuredOpIndexingMapsFormat, className,
- dimIdentsStr, interleaveToString(stmts, "\n "));
+ interleaveToString(stmts, "\n "));
}
} else {
os << llvm::formatv(rankPolyStructuredOpIndexingMapsFormat, className);
diff --git a/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp b/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
index ebadfe4499a54d..5560298831865f 100644
--- a/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
+++ b/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
@@ -519,7 +519,7 @@ static void emitOneCEnumFromConversion(const llvm::Record *record,
os << formatv(
"inline LLVM_ATTRIBUTE_UNUSED {0}::{1} convert{1}FromLLVM(int64_t "
"value) {{\n",
- cppNamespace, cppClassName, llvmClass);
+ cppNamespace, cppClassName);
os << " switch (value) {\n";
for (const auto &enumerant : enumAttr.getAllCases()) {
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 66dbb16760ebb0..572c1545b43fcb 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1378,7 +1378,7 @@ void OpEmitter::genPropertiesSupport() {
::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) -> ::mlir::LogicalResult {{
{0}
};
- {2};
+ {1};
)decl";
const char *attrGetNoDefaultFmt = R"decl(;
if (attr && ::mlir::failed(setFromAttr(prop.{0}, attr, emitError)))
@@ -1419,7 +1419,7 @@ void OpEmitter::genPropertiesSupport() {
&fctx.addSubst("_attr", propertyAttr)
.addSubst("_storage", propertyStorage)
.addSubst("_diag", propertyDiag)),
- name, getAttr);
+ getAttr);
if (prop.hasStorageTypeValueOverride()) {
setPropMethod << formatv(attrGetDefaultFmt, name,
prop.getStorageTypeValueOverride());
@@ -2768,11 +2768,10 @@ void OpEmitter::genInferredTypeCollectiveParamBuilder() {
<< "u && \"mismatched number of return types\");";
body << "\n " << builderOpState << ".addTypes(inferredReturnTypes);";
- body << formatv(R"(
- } else {{
+ body << R"(
+ } else {
::llvm::report_fatal_error("Failed to infer result type(s).");
- })",
- opClass.getClassName(), builderOpState);
+ })";
}
void OpEmitter::genUseOperandAsResultTypeSeparateParamBuilder() {
@@ -3882,7 +3881,7 @@ void OpEmitter::genSuccessorVerifier(MethodBody &body) {
auto getSuccessor =
formatv(successor.isVariadic() ? "{0}()" : getSingleSuccessor,
- successor.name, it.index())
+ successor.name)
.str();
auto constraintFn =
staticVerifierEmitter.getSuccessorConstraintFn(successor.constraint);
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 9a95f495b77658..6558e76dc6cb0b 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1122,7 +1122,7 @@ static void genCustomDirectiveParser(CustomDirective *dir, MethodBody &body,
" {0}Operands.append(subRange.begin(), subRange.end());\n"
" {0}OperandGroupSizes.push_back(subRange.size());\n"
" }\n",
- var->name, var->constraint.getVariadicOfVariadicSegmentSizeAttr());
+ var->name);
}
} else if (auto *dir = dyn_cast<TypeDirective>(param)) {
ArgumentLengthKind lengthKind;
@@ -1575,9 +1575,7 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
ArgumentLengthKind lengthKind = getArgumentLengthKind(operand->getVar());
StringRef name = operand->getVar()->name;
if (lengthKind == ArgumentLengthKind::VariadicOfVariadic)
- body << llvm::formatv(
- variadicOfVariadicOperandParserCode, name,
- operand->getVar()->constraint.getVariadicOfVariadicSegmentSizeAttr());
+ body << llvm::formatv(variadicOfVariadicOperandParserCode, name);
else if (lengthKind == ArgumentLengthKind::Variadic)
body << llvm::formatv(variadicOperandParserCode, name);
else if (lengthKind == ArgumentLengthKind::Optional)
@@ -1656,12 +1654,12 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
dir->shouldBeQualified() ? qualifiedTypeParserCode : typeParserCode;
TypeSwitch<FormatElement *>(dir->getArg())
.Case<OperandVariable, ResultVariable>([&](auto operand) {
- body << formatv(parserCode,
- operand->getVar()->constraint.getCppType(),
- listName);
+ body << formatvv(parserCode,
+ operand->getVar()->constraint.getCppType(),
+ listName);
})
.Default([&](auto operand) {
- body << formatv(parserCode, "::mlir::Type", listName);
+ body << formatvv(parserCode, "::mlir::Type", listName);
});
}
} else if (auto *dir = dyn_cast<FunctionalTypeDirective>(element)) {
diff --git a/mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp b/mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
index 9aeb14d14eeca5..ec211ad3519ce3 100644
--- a/mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
+++ b/mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp
@@ -780,8 +780,7 @@ static void emitSerializationFunction(const Record *attrClass,
os << formatv(" (void)emitDebugLine(functionBody, {0}.getLoc());\n",
opVar);
os << formatv(" (void)encodeInstructionInto("
- "functionBody, spirv::Opcode::{1}, {2});\n",
- op.getQualCppClassName(),
+ "functionBody, spirv::Opcode::{0}, {1});\n",
record->getValueAsString("spirvOpName"), operands);
}
More information about the cfe-commits
mailing list