[clang] [llvm] [Support] Validate number of arguments passed to formatv() (PR #105745)
Rahul Joshi via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 26 17:58:25 PDT 2024
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/105745
>From 7b8c1794d4f2510502349d1151c8266c8b234ac0 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 1/2] [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.
---
llvm/include/llvm/Support/FormatVariadic.h | 21 ++++-
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 3 +-
llvm/lib/Support/FormatVariadic.cpp | 10 ++-
llvm/unittests/Support/FormatVariadicTest.cpp | 79 +++++++++++++------
4 files changed, 81 insertions(+), 32 deletions(-)
diff --git a/llvm/include/llvm/Support/FormatVariadic.h b/llvm/include/llvm/Support/FormatVariadic.h
index 595f2cf559a428..c0c931b3541040 100644
--- a/llvm/include/llvm/Support/FormatVariadic.h
+++ b/llvm/include/llvm/Support/FormatVariadic.h
@@ -83,7 +83,20 @@ class formatv_object_base {
public:
void format(raw_ostream &S) const {
- for (auto &R : parseFormatString(Fmt)) {
+ const auto [Replacements, NumExpectedParams] = parseFormatString(Fmt);
+ // Fail formatv() call if the number of replacement parameters provided
+ // does not match the expected number after parsing the format string.
+ // Assert in debug builds.
+ assert(NumExpectedParams == Adapters.size() &&
+ "Mismatch between replacement parameters expected and provided");
+ if (NumExpectedParams != Adapters.size()) {
+ S << "formatv() error: " << NumExpectedParams
+ << " replacement parameters expected, but " << Adapters.size()
+ << " provided";
+ return;
+ }
+
+ for (const auto &R : Replacements) {
if (R.Type == ReplacementType::Empty)
continue;
if (R.Type == ReplacementType::Literal) {
@@ -101,7 +114,11 @@ class formatv_object_base {
Align.format(S, R.Options);
}
}
- static SmallVector<ReplacementItem, 2> parseFormatString(StringRef Fmt);
+
+ // Parse format string and return the array of replacement items as well as
+ // the number of values we expect to be supplied to the formatv() call.
+ static std::pair<SmallVector<ReplacementItem, 2>, size_t>
+ parseFormatString(StringRef Fmt);
static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec);
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..c355aa519b8c53 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -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.
@@ -143,16 +142,19 @@ formatv_object_base::splitLiteralAndReplacement(StringRef Fmt) {
return std::make_pair(ReplacementItem{Fmt}, StringRef());
}
-SmallVector<ReplacementItem, 2>
+std::pair<SmallVector<ReplacementItem, 2>, size_t>
formatv_object_base::parseFormatString(StringRef Fmt) {
SmallVector<ReplacementItem, 2> Replacements;
+ size_t NumExpectedParams = 0;
ReplacementItem I;
while (!Fmt.empty()) {
std::tie(I, Fmt) = splitLiteralAndReplacement(Fmt);
if (I.Type != ReplacementType::Empty)
Replacements.push_back(I);
+ if (I.Type == ReplacementType::Format)
+ NumExpectedParams = std::max(NumExpectedParams, I.Index + 1);
}
- return Replacements;
+ return {Replacements, NumExpectedParams};
}
void support::detail::format_adapter::anchor() {}
diff --git a/llvm/unittests/Support/FormatVariadicTest.cpp b/llvm/unittests/Support/FormatVariadicTest.cpp
index a78b25c53d7e43..b94c432f73b41a 100644
--- a/llvm/unittests/Support/FormatVariadicTest.cpp
+++ b/llvm/unittests/Support/FormatVariadicTest.cpp
@@ -36,13 +36,15 @@ static_assert(uses_missing_provider<NoFormat>::value, "");
}
TEST(FormatVariadicTest, EmptyFormatString) {
- auto Replacements = formatv_object_base::parseFormatString("");
+ auto [Replacements, NumVals] = formatv_object_base::parseFormatString("");
EXPECT_EQ(0U, Replacements.size());
+ EXPECT_EQ(0U, NumVals);
}
TEST(FormatVariadicTest, NoReplacements) {
const StringRef kFormatString = "This is a test";
- auto Replacements = formatv_object_base::parseFormatString(kFormatString);
+ auto [Replacements, NumVals] =
+ formatv_object_base::parseFormatString(kFormatString);
ASSERT_EQ(1U, Replacements.size());
EXPECT_EQ(kFormatString, Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
@@ -50,25 +52,27 @@ TEST(FormatVariadicTest, NoReplacements) {
TEST(FormatVariadicTest, EscapedBrace) {
// {{ should be replaced with {
- auto Replacements = formatv_object_base::parseFormatString("{{");
+ auto [Replacements, NumVals] = formatv_object_base::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("{{{{{{");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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("}");
+ std::tie(Replacements, NumVals) = formatv_object_base::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("}}}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("}}}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("}}}", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
@@ -76,14 +80,15 @@ TEST(FormatVariadicTest, EscapedBrace) {
TEST(FormatVariadicTest, ValidReplacementSequence) {
// 1. Simple replacement - parameter index only
- auto Replacements = formatv_object_base::parseFormatString("{0}");
+ auto [Replacements, NumVals] = formatv_object_base::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}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{1}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(1u, Replacements[0].Index);
@@ -92,7 +97,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 2. Parameter index with right alignment
- Replacements = formatv_object_base::parseFormatString("{0,3}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0,3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -101,7 +107,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 3. And left alignment
- Replacements = formatv_object_base::parseFormatString("{0,-3}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0,-3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -110,7 +117,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 4. And center alignment
- Replacements = formatv_object_base::parseFormatString("{0,=3}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0,=3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -119,7 +127,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 4. Parameter index with option string
- Replacements = formatv_object_base::parseFormatString("{0:foo}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -128,7 +137,8 @@ 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}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0,-3:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -137,7 +147,8 @@ 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 }");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{ 0, -3 : foo }");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -147,7 +158,8 @@ 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}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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 +169,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("0:1", Replacements[0].Options);
// 9. Custom padding character
- Replacements = formatv_object_base::parseFormatString("{0,p+4:foo}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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 +181,8 @@ 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}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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 +192,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('-', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{0,+-4:foo}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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 +203,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('+', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{0,==4:foo}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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 +214,8 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('=', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- Replacements = formatv_object_base::parseFormatString("{0,:=4:foo}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::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 +228,8 @@ 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, NumVals] =
+ formatv_object_base::parseFormatString("{0,3}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -219,7 +237,8 @@ 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:}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0,3:}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(0u, Replacements[0].Index);
@@ -227,7 +246,8 @@ 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}");
+ std::tie(Replacements, NumVals) =
+ formatv_object_base::parseFormatString("{0:foo}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
EXPECT_EQ(AlignStyle::Right, Replacements[0].Where);
@@ -238,7 +258,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
}
TEST(FormatVariadicTest, MultipleReplacements) {
- auto Replacements =
+ auto [Replacements, NumVals] =
formatv_object_base::parseFormatString("{0} {1:foo}-{2,-3:bar}");
ASSERT_EQ(5u, Replacements.size());
// {0}
@@ -698,6 +718,17 @@ TEST(FormatVariadicTest, FormatError) {
EXPECT_FALSE(E1.isA<StringError>()); // consumed
}
+TEST(FormatVariadicTest, FormatErrorNumArgMismatch) {
+#ifndef NDEBUG
+ EXPECT_EQ(
+ formatv("{0}", 0, 1).str(),
+ "formatv() error: 1 replacement parameters expected, but 2 provided");
+ EXPECT_EQ(
+ formatv("{0} {4}", 0, 1).str(),
+ "formatv() error: 5 replacement parameters expected, but 2 provided");
+#endif
+}
+
TEST(FormatVariadicTest, FormatFilterRange) {
std::vector<int> Vec{0, 1, 2};
auto Range = map_range(Vec, [](int V) { return V + 1; });
>From fd4beebda138da0b736c4c07af9724459680e1e5 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Thu, 22 Aug 2024 14:48:53 -0700
Subject: [PATCH 2/2] [Support] Validate number of arguments passed to
formatv()
- Change formatv() to validate that the number of arguments passed matches
number of replacement parameters in the format string.
- When the format string is a literal, this failure can be easily fixed,
but when its dynamically generated, it may not be as straightforward.
- So changed existing formatv() to only accept literal strings as a
format string and enable argument count validation for that case only.
- Add a `formatvv` variant that allows non-literal format strings and
disables argument count verification.
---
.../Checkers/CheckPlacementNew.cpp | 2 +-
llvm/include/llvm/Support/FormatVariadic.h | 65 +++----
.../Orc/CompileOnDemandLayer.cpp | 8 +-
llvm/lib/Support/FormatVariadic.cpp | 170 ++++++++++--------
llvm/unittests/Support/FormatVariadicTest.cpp | 146 ++++++++-------
llvm/utils/TableGen/IntrinsicEmitter.cpp | 4 +-
6 files changed, 227 insertions(+), 168 deletions(-)
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/llvm/include/llvm/Support/FormatVariadic.h b/llvm/include/llvm/Support/FormatVariadic.h
index c0c931b3541040..b42e24646b31b5 100644
--- a/llvm/include/llvm/Support/FormatVariadic.h
+++ b/llvm/include/llvm/Support/FormatVariadic.h
@@ -66,35 +66,22 @@ struct ReplacementItem {
class formatv_object_base {
protected:
StringRef Fmt;
+ bool Validate;
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);
-
- formatv_object_base(StringRef Fmt,
+ formatv_object_base(StringRef Fmt, bool Validate,
ArrayRef<support::detail::format_adapter *> Adapters)
- : Fmt(Fmt), Adapters(Adapters) {}
+ : Fmt(Fmt), Validate(Validate), Adapters(Adapters) {}
formatv_object_base(formatv_object_base const &rhs) = delete;
formatv_object_base(formatv_object_base &&rhs) = default;
public:
void format(raw_ostream &S) const {
- const auto [Replacements, NumExpectedParams] = parseFormatString(Fmt);
- // Fail formatv() call if the number of replacement parameters provided
- // does not match the expected number after parsing the format string.
- // Assert in debug builds.
- assert(NumExpectedParams == Adapters.size() &&
- "Mismatch between replacement parameters expected and provided");
- if (NumExpectedParams != Adapters.size()) {
- S << "formatv() error: " << NumExpectedParams
- << " replacement parameters expected, but " << Adapters.size()
- << " provided";
+ const auto [Replacements, IsValid] =
+ parseFormatString(S, Fmt, Adapters.size(), Validate);
+ if (!IsValid)
return;
- }
for (const auto &R : Replacements) {
if (R.Type == ReplacementType::Empty)
@@ -115,12 +102,12 @@ class formatv_object_base {
}
}
- // Parse format string and return the array of replacement items as well as
- // the number of values we expect to be supplied to the formatv() call.
- static std::pair<SmallVector<ReplacementItem, 2>, size_t>
- parseFormatString(StringRef Fmt);
-
- static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec);
+ // Parse format string and return the array of replacement items. If there is
+ // an error in the format string, return false for the second member of the
+ // pair, and print the error message to `S`.
+ static std::pair<SmallVector<ReplacementItem, 2>, bool>
+ parseFormatString(raw_ostream &S, StringRef Fmt, size_t NumArgs,
+ bool Validate);
std::string str() const {
std::string Result;
@@ -166,8 +153,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, bool ValidateNumArgs, Tuple &&Params)
+ : formatv_object_base(Fmt, ValidateNumArgs, ParameterPointers),
Parameters(std::move(Params)) {
ParameterPointers = std::apply(create_adapters(), Parameters);
}
@@ -191,7 +178,7 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
//
// rep_field ::= "{" index ["," layout] [":" format] "}"
// index ::= <non-negative integer>
-// layout ::= [[[char]loc]width]
+// layout ::= [[[pad]loc]width]
// format ::= <any string not containing "{" or "}">
// char ::= <any character except "{" or "}">
// loc ::= "-" | "=" | "+"
@@ -204,7 +191,7 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
// format - A type-dependent string used to provide additional options to
// the formatting operation. Refer to the documentation of the
// various individual format providers for per-type options.
-// char - The padding character. Defaults to ' ' (space). Only valid if
+// pad - The padding character. Defaults to ' ' (space). Only valid if
// `loc` is also specified.
// loc - Where to print the formatted text within the field. Only valid if
// `width` is also specified.
@@ -264,6 +251,8 @@ 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 enabled.
template <typename... Ts>
inline auto formatv(const char *Fmt, Ts &&...Vals)
-> formatv_object<decltype(std::make_tuple(
@@ -271,8 +260,22 @@ inline auto formatv(const char *Fmt, 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))...));
+ Fmt, /*Validate=*/true,
+ std::make_tuple(
+ support::detail::build_format_adapter(std::forward<Ts>(Vals))...));
+}
+
+// formatvv() perform no argument/format string validation.
+template <typename... Ts>
+inline auto formatvv(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, /*Validate=*/false,
+ std::make_tuple(
+ support::detail::build_format_adapter(std::forward<Ts>(Vals))...));
}
} // end namespace llvm
diff --git a/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp b/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp
index 6448adaa0ceb36..49f239b73e8a01 100644
--- a/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp
@@ -348,10 +348,10 @@ void CompileOnDemandLayer::emitPartition(
HC = hash_combine(HC, hash_combine_range(GVName.begin(), GVName.end()));
}
raw_string_ostream(SubModuleName)
- << ".submodule."
- << formatv(sizeof(size_t) == 8 ? "{0:x16}" : "{0:x8}",
- static_cast<size_t>(HC))
- << ".ll";
+ << ".submodule."
+ << formatv(sizeof(size_t) == 8 ? "{0:x16}" : "{0:x8}",
+ static_cast<size_t>(HC))
+ << ".ll";
}
// Extract the requested partiton (plus any necessary aliases) and
diff --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp
index c355aa519b8c53..2378c8c0385e40 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -6,8 +6,10 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/ADT/SmallSet.h"
#include <cassert>
#include <optional>
+#include <variant>
using namespace llvm;
@@ -25,8 +27,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 = ' ';
@@ -54,8 +56,8 @@ bool formatv_object_base::consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
return !Failed;
}
-std::optional<ReplacementItem>
-formatv_object_base::parseReplacementItem(StringRef Spec) {
+static std::variant<ReplacementItem, StringRef>
+parseReplacementItem(StringRef Spec) {
StringRef RepString = Spec.trim("{}");
// If the replacement sequence does not start with a non-negative integer,
@@ -66,14 +68,12 @@ formatv_object_base::parseReplacementItem(StringRef Spec) {
StringRef Options;
size_t Index = 0;
RepString = RepString.trim();
- if (RepString.consumeInteger(0, Index)) {
- assert(false && "Invalid replacement sequence index!");
- return ReplacementItem{};
- }
+ if (RepString.consumeInteger(0, Index))
+ return "Invalid replacement sequence index!";
RepString = RepString.trim();
if (RepString.consume_front(",")) {
if (!consumeFieldLayout(RepString, Where, Align, Pad))
- assert(false && "Invalid replacement field layout specification!");
+ return "Invalid replacement field layout specification!";
}
RepString = RepString.trim();
if (RepString.consume_front(":")) {
@@ -81,80 +81,110 @@ formatv_object_base::parseReplacementItem(StringRef Spec) {
RepString = StringRef();
}
RepString = RepString.trim();
- if (!RepString.empty()) {
- assert(false && "Unexpected characters found in replacement string!");
- }
-
+ if (!RepString.empty())
+ return "Unexpected character found in replacement string!";
return ReplacementItem{Spec, Index, Align, Where, Pad, Options};
}
-std::pair<ReplacementItem, StringRef>
-formatv_object_base::splitLiteralAndReplacement(StringRef Fmt) {
- while (!Fmt.empty()) {
- // Everything up until the first brace is a literal.
- if (Fmt.front() != '{') {
- std::size_t BO = Fmt.find_first_of('{');
- return std::make_pair(ReplacementItem{Fmt.substr(0, BO)}, Fmt.substr(BO));
- }
-
- StringRef Braces = Fmt.take_while([](char C) { return C == '{'; });
- // If there is more than one brace, then some of them are escaped. Treat
- // these as replacements.
- if (Braces.size() > 1) {
- size_t NumEscapedBraces = Braces.size() / 2;
- StringRef Middle = Fmt.take_front(NumEscapedBraces);
- StringRef Right = Fmt.drop_front(NumEscapedBraces * 2);
- return std::make_pair(ReplacementItem{Middle}, Right);
- }
- // An unterminated open brace is undefined. Assert to indicate that this is
- // undefined and that we consider it an error. When asserts are disabled,
- // build a replacement item with an error message.
- std::size_t BC = Fmt.find_first_of('}');
- if (BC == StringRef::npos) {
- assert(
- false &&
- "Unterminated brace sequence. Escape with {{ for a literal brace.");
- return std::make_pair(
- ReplacementItem{"Unterminated brace sequence. Escape with {{ for a "
- "literal brace."},
- StringRef());
- }
-
- // Even if there is a closing brace, if there is another open brace before
- // this closing brace, treat this portion as literal, and try again with the
- // next one.
- std::size_t BO2 = Fmt.find_first_of('{', 1);
- if (BO2 < BC)
- return std::make_pair(ReplacementItem{Fmt.substr(0, BO2)},
- Fmt.substr(BO2));
-
- StringRef Spec = Fmt.slice(1, BC);
- StringRef Right = Fmt.substr(BC + 1);
-
- auto RI = parseReplacementItem(Spec);
- if (RI)
- return std::make_pair(*RI, Right);
+static std::variant<std::pair<ReplacementItem, StringRef>, StringRef>
+splitLiteralAndReplacement(StringRef Fmt) {
+ // Everything up until the first brace is a literal.
+ if (Fmt.front() != '{') {
+ std::size_t BO = Fmt.find_first_of('{');
+ return std::make_pair(ReplacementItem(Fmt.substr(0, BO)), Fmt.substr(BO));
+ }
- // If there was an error parsing the replacement item, treat it as an
- // invalid replacement spec, and just continue.
- Fmt = Fmt.drop_front(BC + 1);
+ StringRef Braces = Fmt.take_while([](char C) { return C == '{'; });
+ // If there is more than one brace, then some of them are escaped. Treat
+ // these as replacements.
+ if (Braces.size() > 1) {
+ size_t NumEscapedBraces = Braces.size() / 2;
+ StringRef Middle = Fmt.take_front(NumEscapedBraces);
+ StringRef Right = Fmt.drop_front(NumEscapedBraces * 2);
+ return std::make_pair(ReplacementItem(Middle), Right);
}
- return std::make_pair(ReplacementItem{Fmt}, StringRef());
+ // An unterminated open brace is undefined. Assert to indicate that this is
+ // undefined and that we consider it an error. When asserts are disabled,
+ // build a replacement item with an error message.
+ std::size_t BC = Fmt.find_first_of('}');
+ if (BC == StringRef::npos)
+ return "Unterminated brace sequence. Escape with {{ for a literal brace.";
+
+ // Even if there is a closing brace, if there is another open brace before
+ // this closing brace, treat this portion as literal, and try again with the
+ // next one.
+ std::size_t BO2 = Fmt.find_first_of('{', 1);
+ if (BO2 < BC)
+ return std::make_pair(ReplacementItem{Fmt.substr(0, BO2)}, Fmt.substr(BO2));
+
+ StringRef Spec = Fmt.slice(1, BC);
+ StringRef Right = Fmt.substr(BC + 1);
+
+ auto RI = parseReplacementItem(Spec);
+ if (const StringRef *ErrMsg = std::get_if<1>(&RI))
+ return *ErrMsg;
+
+ return std::make_pair(std::get<0>(RI), Right);
}
-std::pair<SmallVector<ReplacementItem, 2>, size_t>
-formatv_object_base::parseFormatString(StringRef Fmt) {
+std::pair<SmallVector<ReplacementItem, 2>, bool>
+formatv_object_base::parseFormatString(raw_ostream &S, const StringRef Fmt,
+ size_t NumArgs, bool Validate) {
SmallVector<ReplacementItem, 2> Replacements;
- size_t NumExpectedParams = 0;
ReplacementItem I;
- while (!Fmt.empty()) {
- std::tie(I, Fmt) = splitLiteralAndReplacement(Fmt);
+ size_t NumExpectedArgs = 0;
+
+ // Make a copy for pasring as it updates it.
+ StringRef ParseFmt = Fmt;
+ while (!ParseFmt.empty()) {
+ auto RI = splitLiteralAndReplacement(ParseFmt);
+ if (const StringRef *ErrMsg = std::get_if<1>(&RI)) {
+ // If there was an error parsing the format string, write the error to the
+ // stream, and return false as second member of the pair.
+ errs() << "Invalid format string: " << Fmt << "\n";
+ assert(0 && "Invalid format string");
+ S << *ErrMsg;
+ return {{}, false};
+ }
+ std::tie(I, ParseFmt) = std::get<0>(RI);
if (I.Type != ReplacementType::Empty)
Replacements.push_back(I);
if (I.Type == ReplacementType::Format)
- NumExpectedParams = std::max(NumExpectedParams, I.Index + 1);
+ NumExpectedArgs = std::max(NumExpectedArgs, I.Index + 1);
}
- return {Replacements, NumExpectedParams};
+ if (!Validate)
+ return {Replacements, true};
+
+ // 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.
+ if (NumExpectedArgs != NumArgs) {
+ errs() << "Expected " << NumExpectedArgs << " Args, but got " << NumArgs
+ << " for format string '" << Fmt;
+ assert(0 && "Invalid formatv() call");
+ S << "Expected " << NumExpectedArgs << " Args, but got " << NumArgs
+ << " for format string '" << Fmt << "'\n";
+ return {{}, false};
+ }
+
+ SmallSet<size_t, 2> Indices;
+ for (const ReplacementItem &R : Replacements) {
+ if (R.Type != ReplacementType::Format)
+ continue;
+ Indices.insert(R.Index);
+ }
+
+ if (Indices.size() != NumExpectedArgs) {
+ errs() << "Invalid format string: Replacement field indices "
+ "cannot have holes for format string '"
+ << Fmt << "'\n";
+ assert(0 && "Invalid format string");
+ S << "Replacement field indices cannot have holes for format string '"
+ << Fmt << "'\n";
+ return {{}, false};
+ }
+
+ return {Replacements, true};
}
void support::detail::format_adapter::anchor() {}
diff --git a/llvm/unittests/Support/FormatVariadicTest.cpp b/llvm/unittests/Support/FormatVariadicTest.cpp
index b94c432f73b41a..23399babe7337e 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,16 +37,38 @@ struct NoFormat {};
static_assert(uses_missing_provider<NoFormat>::value, "");
}
+// Helper to parse format string with no validation.
+static SmallVector<ReplacementItem, 2> parseFormatString(StringRef Fmt) {
+ std::string Error;
+ raw_string_ostream ErrorStream(Error);
+ auto [Replacements, IsValid] =
+ formatv_object_base::parseFormatString(ErrorStream, Fmt, 0, false);
+ ErrorStream.flush();
+ assert(IsValid && Error.empty());
+ return Replacements;
+}
+
+#if NDEBUG
+// Helper for parse format string with errors.
+static std::string parseFormatStringError(StringRef Fmt) {
+ std::string Error;
+ raw_string_ostream ErrorStream(Error);
+ auto [Replacements, IsValid] =
+ formatv_object_base::parseFormatString(ErrorStream, Fmt, 0, false);
+ ErrorStream.flush();
+ assert(!IsValid && Replacements.empty());
+ return Error;
+}
+#endif
+
TEST(FormatVariadicTest, EmptyFormatString) {
- auto [Replacements, NumVals] = formatv_object_base::parseFormatString("");
+ auto Replacements = parseFormatString("");
EXPECT_EQ(0U, Replacements.size());
- EXPECT_EQ(0U, NumVals);
}
TEST(FormatVariadicTest, NoReplacements) {
const StringRef kFormatString = "This is a test";
- auto [Replacements, NumVals] =
- 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);
@@ -52,27 +76,25 @@ TEST(FormatVariadicTest, NoReplacements) {
TEST(FormatVariadicTest, EscapedBrace) {
// {{ should be replaced with {
- auto [Replacements, NumVals] = 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.
- std::tie(Replacements, NumVals) =
- 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.
- std::tie(Replacements, NumVals) = 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.
- std::tie(Replacements, NumVals) =
- formatv_object_base::parseFormatString("}}}");
+ Replacements = parseFormatString("}}}");
ASSERT_EQ(1u, Replacements.size());
EXPECT_EQ("}}}", Replacements[0].Spec);
EXPECT_EQ(ReplacementType::Literal, Replacements[0].Type);
@@ -80,15 +102,14 @@ TEST(FormatVariadicTest, EscapedBrace) {
TEST(FormatVariadicTest, ValidReplacementSequence) {
// 1. Simple replacement - parameter index only
- auto [Replacements, NumVals] = 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);
- std::tie(Replacements, NumVals) =
- 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);
@@ -97,8 +118,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 2. Parameter index with right alignment
- std::tie(Replacements, NumVals) =
- 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);
@@ -107,8 +127,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 3. And left alignment
- std::tie(Replacements, NumVals) =
- 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);
@@ -117,8 +136,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 4. And center alignment
- std::tie(Replacements, NumVals) =
- 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);
@@ -127,8 +145,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("", Replacements[0].Options);
// 4. Parameter index with option string
- std::tie(Replacements, NumVals) =
- 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);
@@ -137,8 +154,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("foo", Replacements[0].Options);
// 5. Parameter index with alignment before option string
- std::tie(Replacements, NumVals) =
- 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,8 +163,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("foo", Replacements[0].Options);
// 7. Parameter indices, options, and alignment can all have whitespace.
- std::tie(Replacements, NumVals) =
- 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);
@@ -158,8 +173,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
// 8. Everything after the first option specifier is part of the style, even
// if it contains another option specifier.
- std::tie(Replacements, NumVals) =
- 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);
@@ -169,8 +183,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("0:1", Replacements[0].Options);
// 9. Custom padding character
- std::tie(Replacements, NumVals) =
- 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);
@@ -181,8 +194,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ("foo", Replacements[0].Options);
// Format string special characters are allowed as padding character
- std::tie(Replacements, NumVals) =
- 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);
@@ -192,8 +204,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('-', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- std::tie(Replacements, NumVals) =
- 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);
@@ -203,8 +214,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('+', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- std::tie(Replacements, NumVals) =
- 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);
@@ -214,8 +224,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ('=', Replacements[0].Pad);
EXPECT_EQ("foo", Replacements[0].Options);
- std::tie(Replacements, NumVals) =
- 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);
@@ -228,8 +237,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
TEST(FormatVariadicTest, DefaultReplacementValues) {
// 2. If options string is missing, it defaults to empty.
- auto [Replacements, NumVals] =
- 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);
@@ -237,8 +245,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
EXPECT_EQ("", Replacements[0].Options);
// Including if the colon is present but contains no text.
- std::tie(Replacements, NumVals) =
- 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);
@@ -246,8 +253,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
EXPECT_EQ("", Replacements[0].Options);
// 3. If alignment is missing, it defaults to 0, right, space
- std::tie(Replacements, NumVals) =
- 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);
@@ -258,8 +264,7 @@ TEST(FormatVariadicTest, DefaultReplacementValues) {
}
TEST(FormatVariadicTest, MultipleReplacements) {
- auto [Replacements, NumVals] =
- 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);
@@ -291,6 +296,28 @@ TEST(FormatVariadicTest, MultipleReplacements) {
EXPECT_EQ("bar", Replacements[4].Options);
}
+#ifdef NDEBUG // Disable the test in debug builds where it will assert.
+TEST(FormatVariadicTest, FormatStringError) {
+ // Missing index.
+ std::string Error = parseFormatStringError("{}");
+ EXPECT_THAT(Error, HasSubstr("Invalid replacement sequence index"));
+
+ // Invalid character after layout.
+ Error = parseFormatStringError("{0, +h}");
+ EXPECT_THAT(Error,
+ HasSubstr("Invalid replacement field layout specification"));
+
+ // Invalid character after the alignment.
+ Error = parseFormatStringError("{0, +20,}");
+ EXPECT_THAT(Error,
+ HasSubstr("Unexpected character found in replacement string"));
+
+ // Un-escaped {
+ Error = parseFormatStringError("{");
+ EXPECT_THAT(Error, HasSubstr("Unterminated brace sequence"));
+}
+#endif // NDEBUG
+
TEST(FormatVariadicTest, FormatNoReplacements) {
EXPECT_EQ("", formatv("").str());
EXPECT_EQ("Test", formatv("Test").str());
@@ -520,7 +547,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)...);
}
};
@@ -536,12 +563,12 @@ TEST(FormatVariadicTest, BigTest) {
.08215f, (void *)nullptr, 0, 6.62E-34, -908234908423,
908234908422234, std::numeric_limits<double>::infinity(), 0x0)};
// Test long string formatting with many edge cases combined.
- const char *Intro =
+ const char Intro[] =
"There are {{{0}} items in the tuple, and {{{1}} tuple(s) in the array.";
- const char *Header =
+ const char Header[] =
"{0,6}|{1,8}|{2,=10}|{3,=10}|{4,=13}|{5,7}|{6,7}|{7,10}|{8,"
"-7}|{9,10}|{10,16}|{11,17}|{12,6}|{13,4}";
- const char *Line =
+ const char Line[] =
"{0,6}|{1,8:X}|{2,=10}|{3,=10:5}|{4,=13}|{5,7:3}|{6,7:P2}|{7,"
"10:X8}|{8,-7:N}|{9,10:E4}|{10,16:N}|{11,17:D}|{12,6}|{13,"
"4:X}";
@@ -718,23 +745,22 @@ TEST(FormatVariadicTest, FormatError) {
EXPECT_FALSE(E1.isA<StringError>()); // consumed
}
-TEST(FormatVariadicTest, FormatErrorNumArgMismatch) {
-#ifndef NDEBUG
- EXPECT_EQ(
- formatv("{0}", 0, 1).str(),
- "formatv() error: 1 replacement parameters expected, but 2 provided");
- EXPECT_EQ(
- formatv("{0} {4}", 0, 1).str(),
- "formatv() error: 5 replacement parameters expected, but 2 provided");
-#endif
-}
-
TEST(FormatVariadicTest, FormatFilterRange) {
std::vector<int> Vec{0, 1, 2};
auto Range = map_range(Vec, [](int V) { return V + 1; });
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("Expected 1 Args, but got 2"));
+
+ Str = formatv("{0} {2}", 1, 2, 3).str();
+ EXPECT_THAT(Str, HasSubstr("eplacement field indices cannot 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";
More information about the cfe-commits
mailing list