Skip to content

Commit

Permalink
[formatv] Leave format parameters unstripped (#112625)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
dwblaikie authored Oct 16, 2024
1 parent 5f9e6c8 commit f796a0c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
7 changes: 3 additions & 4 deletions llvm/lib/Support/FormatVariadic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,20 @@ static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec) {
AlignStyle Where = AlignStyle::Right;
StringRef Options;
unsigned Index = ~0U;
RepString = RepString.trim();
RepString = RepString.ltrim();

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

if (RepString.consume_front(",")) {
if (!consumeFieldLayout(RepString, Where, Align, Pad)) {
assert(false && "Invalid replacement field layout specification!");
return std::nullopt;
}
}
RepString = RepString.trim();
RepString = RepString.ltrim();
if (RepString.consume_front(":")) {
Options = RepString.trim();
Options = RepString;
RepString = StringRef();
}
RepString = RepString.trim();
Expand Down
2 changes: 1 addition & 1 deletion llvm/unittests/Support/FormatVariadicTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ TEST(FormatVariadicTest, ValidReplacementSequence) {
EXPECT_EQ(0u, Replacements[0].Index);
EXPECT_EQ(3u, Replacements[0].Width);
EXPECT_EQ(AlignStyle::Left, Replacements[0].Where);
EXPECT_EQ("foo", Replacements[0].Options);
EXPECT_EQ(" foo ", Replacements[0].Options);

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

0 comments on commit f796a0c

Please sign in to comment.