Fixes issue where AbslStringify() breaks formatting with %d for enums

PiperOrigin-RevId: 493682437 Change-Id: I30f2ac36b998b86c24fe7513cd952b860560a66e
2026-06-04 12:07:05 +08:00 · 2022-12-07 12:49:08 -08:00
parent e9787e7d1d
commit 9bff2a9302
3 changed files with 170 additions and 47 deletions
--- a/absl/strings/internal/str_format/arg.cc
+++ b/absl/strings/internal/str_format/arg.cc
@@ -296,6 +296,37 @@ constexpr auto ConvertV(T) {
  return FormatConversionCharInternal::u;
 }

+template <typename T>
+bool ConvertFloatArg(T v, FormatConversionSpecImpl conv, FormatSinkImpl *sink) {
+  if (conv.conversion_char() == FormatConversionCharInternal::v) {
+    conv.set_conversion_char(FormatConversionCharInternal::g);
+  }
+
+  return FormatConversionCharIsFloat(conv.conversion_char()) &&
+         ConvertFloatImpl(v, conv, sink);
+}
+
+inline bool ConvertStringArg(string_view v, const FormatConversionSpecImpl conv,
+                             FormatSinkImpl *sink) {
+  if (conv.is_basic()) {
+    sink->Append(v);
+    return true;
+  }
+  return sink->PutPaddedString(v, conv.width(), conv.precision(),
+                               conv.has_left_flag());
+}
+
+}  // namespace
+
+bool ConvertBoolArg(bool v, FormatSinkImpl *sink) {
+  if (v) {
+    sink->Append("true");
+  } else {
+    sink->Append("false");
+  }
+  return true;
+}
+
 template <typename T>
 bool ConvertIntArg(T v, FormatConversionSpecImpl conv, FormatSinkImpl *sink) {
  using U = typename MakeUnsigned<T>::type;
@@ -354,36 +385,37 @@ bool ConvertIntArg(T v, FormatConversionSpecImpl conv, FormatSinkImpl *sink) {
  return ConvertIntImplInnerSlow(as_digits, conv, sink);
 }

-template <typename T>
-bool ConvertFloatArg(T v, FormatConversionSpecImpl conv, FormatSinkImpl *sink) {
-  if (conv.conversion_char() == FormatConversionCharInternal::v) {
-    conv.set_conversion_char(FormatConversionCharInternal::g);
-  }
-
-  return FormatConversionCharIsFloat(conv.conversion_char()) &&
-         ConvertFloatImpl(v, conv, sink);
-}
-
-inline bool ConvertStringArg(string_view v, const FormatConversionSpecImpl conv,
-                             FormatSinkImpl *sink) {
-  if (conv.is_basic()) {
-    sink->Append(v);
-    return true;
-  }
-  return sink->PutPaddedString(v, conv.width(), conv.precision(),
-                               conv.has_left_flag());
-}
-
-}  // namespace
-
-bool ConvertBoolArg(bool v, FormatSinkImpl *sink) {
-  if (v) {
-    sink->Append("true");
-  } else {
-    sink->Append("false");
-  }
-  return true;
-}
+template bool ConvertIntArg<char>(char v, FormatConversionSpecImpl conv,
+                                  FormatSinkImpl *sink);
+template bool ConvertIntArg<signed char>(signed char v,
+                                         FormatConversionSpecImpl conv,
+                                         FormatSinkImpl *sink);
+template bool ConvertIntArg<unsigned char>(unsigned char v,
+                                           FormatConversionSpecImpl conv,
+                                           FormatSinkImpl *sink);
+template bool ConvertIntArg<short>(short v,  // NOLINT
+                                   FormatConversionSpecImpl conv,
+                                   FormatSinkImpl *sink);
+template bool ConvertIntArg<unsigned short>(unsigned short v,  // NOLINT
+                                            FormatConversionSpecImpl conv,
+                                            FormatSinkImpl *sink);
+template bool ConvertIntArg<int>(int v, FormatConversionSpecImpl conv,
+                                 FormatSinkImpl *sink);
+template bool ConvertIntArg<unsigned int>(unsigned int v,
+                                          FormatConversionSpecImpl conv,
+                                          FormatSinkImpl *sink);
+template bool ConvertIntArg<long>(long v,  // NOLINT
+                                  FormatConversionSpecImpl conv,
+                                  FormatSinkImpl *sink);
+template bool ConvertIntArg<unsigned long>(unsigned long v,  // NOLINT
+                                           FormatConversionSpecImpl conv,
+                                           FormatSinkImpl *sink);
+template bool ConvertIntArg<long long>(long long v,  // NOLINT
+                                       FormatConversionSpecImpl conv,
+                                       FormatSinkImpl *sink);
+template bool ConvertIntArg<unsigned long long>(unsigned long long v,  // NOLINT
+                                                FormatConversionSpecImpl conv,
+                                                FormatSinkImpl *sink);

 // ==================== Strings ====================
 StringConvertResult FormatConvertImpl(const std::string &v,
--- a/absl/strings/internal/str_format/arg.h
+++ b/absl/strings/internal/str_format/arg.h
@@ -53,6 +53,19 @@ struct ArgConvertResult {
  bool value;
 };

+using IntegralConvertResult = ArgConvertResult<FormatConversionCharSetUnion(
+    FormatConversionCharSetInternal::c,
+    FormatConversionCharSetInternal::kNumeric,
+    FormatConversionCharSetInternal::kStar,
+    FormatConversionCharSetInternal::v)>;
+using FloatingConvertResult = ArgConvertResult<FormatConversionCharSetUnion(
+    FormatConversionCharSetInternal::kFloating,
+    FormatConversionCharSetInternal::v)>;
+using CharConvertResult = ArgConvertResult<FormatConversionCharSetUnion(
+    FormatConversionCharSetInternal::c,
+    FormatConversionCharSetInternal::kNumeric,
+    FormatConversionCharSetInternal::kStar)>;
+
 template <typename T, typename = void>
 struct HasUserDefinedConvert : std::false_type {};

@@ -69,6 +82,44 @@ struct HasUserDefinedConvert<T, void_t<decltype(AbslFormatConvert(
 void AbslFormatConvert();
 void AbslStringify();

+template <typename T>
+bool ConvertIntArg(T v, FormatConversionSpecImpl conv, FormatSinkImpl* sink);
+
+// Forward declarations of internal `ConvertIntArg` function template
+// instantiations are here to avoid including the template body in the headers
+// and instantiating it in large numbers of translation units. Explicit
+// instantiations can be found in "absl/strings/internal/str_format/arg.cc"
+extern template bool ConvertIntArg<char>(char v, FormatConversionSpecImpl conv,
+                                         FormatSinkImpl* sink);
+extern template bool ConvertIntArg<signed char>(signed char v,
+                                                FormatConversionSpecImpl conv,
+                                                FormatSinkImpl* sink);
+extern template bool ConvertIntArg<unsigned char>(unsigned char v,
+                                                  FormatConversionSpecImpl conv,
+                                                  FormatSinkImpl* sink);
+extern template bool ConvertIntArg<short>(short v,  // NOLINT
+                                          FormatConversionSpecImpl conv,
+                                          FormatSinkImpl* sink);
+extern template bool ConvertIntArg<unsigned short>(   // NOLINT
+    unsigned short v, FormatConversionSpecImpl conv,  // NOLINT
+    FormatSinkImpl* sink);
+extern template bool ConvertIntArg<int>(int v, FormatConversionSpecImpl conv,
+                                        FormatSinkImpl* sink);
+extern template bool ConvertIntArg<unsigned int>(unsigned int v,
+                                                 FormatConversionSpecImpl conv,
+                                                 FormatSinkImpl* sink);
+extern template bool ConvertIntArg<long>(                           // NOLINT
+    long v, FormatConversionSpecImpl conv, FormatSinkImpl* sink);   // NOLINT
+extern template bool ConvertIntArg<unsigned long>(unsigned long v,  // NOLINT
+                                                  FormatConversionSpecImpl conv,
+                                                  FormatSinkImpl* sink);
+extern template bool ConvertIntArg<long long>(long long v,  // NOLINT
+                                              FormatConversionSpecImpl conv,
+                                              FormatSinkImpl* sink);
+extern template bool ConvertIntArg<unsigned long long>(   // NOLINT
+    unsigned long long v, FormatConversionSpecImpl conv,  // NOLINT
+    FormatSinkImpl* sink);
+
 template <typename T>
 auto FormatConvertImpl(const T& v, FormatConversionSpecImpl conv,
                       FormatSinkImpl* sink)
@@ -84,11 +135,31 @@ auto FormatConvertImpl(const T& v, FormatConversionSpecImpl conv,
  return AbslFormatConvert(v, fcs, &fs);
 }

+template <typename T>
+auto FormatConvertImpl(const T& v, FormatConversionSpecImpl conv,
+                       FormatSinkImpl* sink)
+    -> std::enable_if_t<std::is_enum<T>::value &&
+                            std::is_void<decltype(AbslStringify(
+                                std::declval<FormatSink&>(), v))>::value,
+                        IntegralConvertResult> {
+  if (conv.conversion_char() == FormatConversionCharInternal::v) {
+    using FormatSinkT =
+        absl::enable_if_t<sizeof(const T& (*)()) != 0, FormatSink>;
+    auto fs = sink->Wrap<FormatSinkT>();
+    AbslStringify(fs, v);
+    return {true};
+  } else {
+    return {ConvertIntArg(
+        static_cast<typename std::underlying_type<T>::type>(v), conv, sink)};
+  }
+}
+
 template <typename T>
 auto FormatConvertImpl(const T& v, FormatConversionSpecImpl,
                       FormatSinkImpl* sink)
-    -> std::enable_if_t<std::is_void<decltype(AbslStringify(
-                            std::declval<FormatSink&>(), v))>::value,
+    -> std::enable_if_t<!std::is_enum<T>::value &&
+                            std::is_void<decltype(AbslStringify(
+                                std::declval<FormatSink&>(), v))>::value,
                        ArgConvertResult<FormatConversionCharSetInternal::v>> {
  using FormatSinkT =
      absl::enable_if_t<sizeof(const T& (*)()) != 0, FormatSink>;
@@ -194,19 +265,6 @@ StringConvertResult FormatConvertImpl(const AbslCord& value,
  return {true};
 }

-using IntegralConvertResult = ArgConvertResult<FormatConversionCharSetUnion(
-    FormatConversionCharSetInternal::c,
-    FormatConversionCharSetInternal::kNumeric,
-    FormatConversionCharSetInternal::kStar,
-    FormatConversionCharSetInternal::v)>;
-using FloatingConvertResult = ArgConvertResult<FormatConversionCharSetUnion(
-    FormatConversionCharSetInternal::kFloating,
-    FormatConversionCharSetInternal::v)>;
-using CharConvertResult = ArgConvertResult<FormatConversionCharSetUnion(
-    FormatConversionCharSetInternal::c,
-    FormatConversionCharSetInternal::kNumeric,
-    FormatConversionCharSetInternal::kStar)>;
-
 bool ConvertBoolArg(bool v, FormatSinkImpl* sink);

 // Floats.
--- a/absl/strings/str_format_test.cc
+++ b/absl/strings/str_format_test.cc
@@ -1142,18 +1142,51 @@ TEST_F(FormatExtensionTest, AbslStringifyExampleUsingFormat) {
  EXPECT_EQ(absl::StrFormat("a %v z", p), "a (10, 20) z");
 }

-enum class EnumWithStringify { Many = 0, Choices = 1 };
+enum class EnumClassWithStringify { Many = 0, Choices = 1 };
+
+template <typename Sink>
+void AbslStringify(Sink& sink, EnumClassWithStringify e) {
+  absl::Format(&sink, "%s",
+               e == EnumClassWithStringify::Many ? "Many" : "Choices");
+}
+
+enum EnumWithStringify { Many, Choices };

 template <typename Sink>
 void AbslStringify(Sink& sink, EnumWithStringify e) {
  absl::Format(&sink, "%s", e == EnumWithStringify::Many ? "Many" : "Choices");
 }

-TEST_F(FormatExtensionTest, AbslStringifyWithEnum) {
+TEST_F(FormatExtensionTest, AbslStringifyWithEnumWithV) {
+  const auto e_class = EnumClassWithStringify::Choices;
+  EXPECT_EQ(absl::StrFormat("My choice is %v", e_class),
+            "My choice is Choices");
+
  const auto e = EnumWithStringify::Choices;
  EXPECT_EQ(absl::StrFormat("My choice is %v", e), "My choice is Choices");
 }

+TEST_F(FormatExtensionTest, AbslStringifyEnumWithD) {
+  const auto e_class = EnumClassWithStringify::Many;
+  EXPECT_EQ(absl::StrFormat("My choice is %d", e_class), "My choice is 0");
+
+  const auto e = EnumWithStringify::Choices;
+  EXPECT_EQ(absl::StrFormat("My choice is %d", e), "My choice is 1");
+}
+
+enum class EnumWithLargerValue { x = 32 };
+
+template <typename Sink>
+void AbslStringify(Sink& sink, EnumWithLargerValue e) {
+  absl::Format(&sink, "%s", "Many");
+}
+
+TEST_F(FormatExtensionTest, AbslStringifyEnumOtherSpecifiers) {
+  const auto e = EnumWithLargerValue::x;
+  EXPECT_EQ(absl::StrFormat("My choice is %g", e), "My choice is 32");
+  EXPECT_EQ(absl::StrFormat("My choice is %x", e), "My choice is 20");
+}
+
 }  // namespace

 // Some codegen thunks that we can use to easily dump the generated assembly for