Fix padding with custom character in formatv

[mkoeppen]

Summary

The format string for formatv allows to specify a custom padding character instead of the default space.
This custom character was parsed correctly, but not passed on to the formatter.

[zturner]

Sorry this slipped by me. Lgtm and thanks for the fix!

[mkoeppen]

Thanks for the review! In case this patch is accepted: I do not have commit access.

[mkoeppen]

ping :)

include/llvm/Support/FormatVariadic.h

for (auto &R : Replacements) {
       }
       if (R.Index >= Adapters.size()) {
         S << R.Spec;
         continue;
       }
 
       auto W = Adapters[R.Index];
 
-      FmtAlign Align(*W, R.Where, R.Align);
+      FmtAlign Align(*W, R.Where, R.Align, R.Pad);
       Align.format(S, R.Options);
     }
   }
   static std::vector<ReplacementItem> parseFormatString(StringRef Fmt);
 
   static Optional<ReplacementItem> parseReplacementItem(StringRef Spec);
 
   std::string str() const {

unittests/Support/FormatVariadicTest.cpp

TEST(FormatVariadicTest, ValidReplacementSequence) {
   Replacements = 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);
   EXPECT_EQ(0u, Replacements[0].Index);
   EXPECT_EQ(0u, Replacements[0].Align);
   EXPECT_EQ(AlignStyle::Right, Replacements[0].Where);
   EXPECT_EQ("0:1", Replacements[0].Options);
+
+  // 9. Custom padding character
+  Replacements = 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);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(4u, Replacements[0].Align);
+  EXPECT_EQ(AlignStyle::Right, Replacements[0].Where);
+  EXPECT_EQ('p', Replacements[0].Pad);
+  EXPECT_EQ("foo", Replacements[0].Options);
+
+  // Format string special characters are allowed as padding character
+  Replacements = 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);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(4u, Replacements[0].Align);
+  EXPECT_EQ(AlignStyle::Right, Replacements[0].Where);
+  EXPECT_EQ('-', Replacements[0].Pad);
+  EXPECT_EQ("foo", Replacements[0].Options);
+
+  Replacements = 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);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(4u, Replacements[0].Align);
+  EXPECT_EQ(AlignStyle::Left, Replacements[0].Where);
+  EXPECT_EQ('+', Replacements[0].Pad);
+  EXPECT_EQ("foo", Replacements[0].Options);
+
+  Replacements = 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);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(4u, Replacements[0].Align);
+  EXPECT_EQ(AlignStyle::Center, Replacements[0].Where);
+  EXPECT_EQ('=', Replacements[0].Pad);
+  EXPECT_EQ("foo", Replacements[0].Options);
+
+  Replacements = 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);
+  EXPECT_EQ(0u, Replacements[0].Index);
+  EXPECT_EQ(4u, Replacements[0].Align);
+  EXPECT_EQ(AlignStyle::Center, Replacements[0].Where);
+  EXPECT_EQ(':', Replacements[0].Pad);
+  EXPECT_EQ("foo", Replacements[0].Options);
 }
 
 TEST(FormatVariadicTest, DefaultReplacementValues) {
   // 2. If options string is missing, it defaults to empty.
   auto Replacements = formatv_object_base::parseFormatString("{0,3}");
   ASSERT_EQ(1u, Replacements.size());
   EXPECT_EQ(ReplacementType::Format, Replacements[0].Type);
   EXPECT_EQ(0u, Replacements[0].Index);
TEST(FormatVariadicTest, DoubleFormatting) {
 
   // 2. With padding and precision.
   EXPECT_EQ("   0.000", formatv("{0,8:F3}", 0.0).str());
   EXPECT_EQ("  -1.100", formatv("{0,8:F3}", -1.1).str());
   EXPECT_EQ("1234.568", formatv("{0,8:F3}", 1234.5678).str());
   EXPECT_EQ("  -0.001", formatv("{0,8:F3}", -.0012345678).str());
 }
 
+TEST(FormatVariadicTest, CustomPaddingCharacter) {
+  // 1. Padding with custom character
+  EXPECT_EQ("==123", formatv("{0,=+5}", 123).str());
+  EXPECT_EQ("=123=", formatv("{0,==5}", 123).str());
+  EXPECT_EQ("123==", formatv("{0,=-5}", 123).str());
+
+  // 2. Combined with zero padding
+  EXPECT_EQ("=00123=", formatv("{0,==7:5}", 123).str());
+}
+
 struct format_tuple {
   const char *Fmt;
   explicit format_tuple(const char *Fmt) : Fmt(Fmt) {}
 
   template <typename... Ts>
   auto operator()(Ts &&... Values) const
       -> decltype(formatv(Fmt, std::forward<Ts>(Values)...)) {
     return formatv(Fmt, std::forward<Ts>(Values)...);