Skip to content

Commit f796a0c

Browse files
authored
[formatv] Leave format parameters unstripped (#112625)
This is consistent with std::formatv and allows formatters to support a wider variety of use cases (like having a bare string in their formatter if that's useful, etc). Came up in the context of some Carbon diagnostic work here: carbon-language/carbon-lang#4411 (comment)
1 parent 5f9e6c8 commit f796a0c

File tree

2 files changed

+4
-5
lines changed

2 files changed

+4
-5
lines changed

llvm/lib/Support/FormatVariadic.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,21 +64,20 @@ static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec) {
6464
AlignStyle Where = AlignStyle::Right;
6565
StringRef Options;
6666
unsigned Index = ~0U;
67-
RepString = RepString.trim();
67+
RepString = RepString.ltrim();
6868

6969
// If index is not specified, keep it ~0U to indicate unresolved index.
7070
RepString.consumeInteger(0, Index);
71-
RepString = RepString.trim();
7271

7372
if (RepString.consume_front(",")) {
7473
if (!consumeFieldLayout(RepString, Where, Align, Pad)) {
7574
assert(false && "Invalid replacement field layout specification!");
7675
return std::nullopt;
7776
}
7877
}
79-
RepString = RepString.trim();
78+
RepString = RepString.ltrim();
8079
if (RepString.consume_front(":")) {
81-
Options = RepString.trim();
80+
Options = RepString;
8281
RepString = StringRef();
8382
}
8483
RepString = RepString.trim();

llvm/unittests/Support/FormatVariadicTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
150150
EXPECT_EQ(0u, Replacements[0].Index);
151151
EXPECT_EQ(3u, Replacements[0].Width);
152152
EXPECT_EQ(AlignStyle::Left, Replacements[0].Where);
153-
EXPECT_EQ("foo", Replacements[0].Options);
153+
EXPECT_EQ(" foo ", Replacements[0].Options);
154154

155155
// 8. Everything after the first option specifier is part of the style, even
156156
// if it contains another option specifier.

0 commit comments

Comments
 (0)