[llvm] [formatv] Leave format parameters unstripped (PR #112625)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 16 15:27:11 PDT 2024


https://github.com/dwblaikie created https://github.com/llvm/llvm-project/pull/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: https://github.com/carbon-language/carbon-lang/pull/4411#discussion_r1803688859

>From bef17395484ef73e14a14ab7500e4dcd7cfe7d9a Mon Sep 17 00:00:00 2001
From: David Blaikie <dblaikie at gmail.com>
Date: Wed, 16 Oct 2024 22:07:31 +0000
Subject: [PATCH] [formatv] Leave format parameters unstripped

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).
---
 llvm/lib/Support/FormatVariadic.cpp           | 7 +++----
 llvm/unittests/Support/FormatVariadicTest.cpp | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp
index 7eb10887940513..f3e8d0a7fe6f33 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -64,11 +64,10 @@ 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)) {
@@ -76,9 +75,9 @@ static std::optional<ReplacementItem> parseReplacementItem(StringRef Spec) {
       return std::nullopt;
     }
   }
-  RepString = RepString.trim();
+  RepString = RepString.ltrim();
   if (RepString.consume_front(":")) {
-    Options = RepString.trim();
+    Options = RepString;
     RepString = StringRef();
   }
   RepString = RepString.trim();
diff --git a/llvm/unittests/Support/FormatVariadicTest.cpp b/llvm/unittests/Support/FormatVariadicTest.cpp
index e745f99a5a6c4e..03102c96d46629 100644
--- a/llvm/unittests/Support/FormatVariadicTest.cpp
+++ b/llvm/unittests/Support/FormatVariadicTest.cpp
@@ -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.



More information about the llvm-commits mailing list