Fixes issue where AbslStringify() breaks formatting with %d for enums
PiperOrigin-RevId: 493682437
Change-Id: I30f2ac36b998b86c24fe7513cd952b860560a66e
diff --git a/absl/strings/internal/str_format/arg.cc b/absl/strings/internal/str_format/arg.cc
index 967fe9c..018dd05 100644
--- a/absl/strings/internal/str_format/arg.cc
+++ b/absl/strings/internal/str_format/arg.cc
@@ -297,6 +297,37 @@
}
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;
IntDigits as_digits;
@@ -354,36 +385,37 @@
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,
diff --git a/absl/strings/internal/str_format/arg.h b/absl/strings/internal/str_format/arg.h
index bc4cde9..e4b1662 100644
--- a/absl/strings/internal/str_format/arg.h
+++ b/absl/strings/internal/str_format/arg.h
@@ -53,6 +53,19 @@
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 {};
@@ -70,6 +83,44 @@
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)
-> decltype(AbslFormatConvert(v,
@@ -85,10 +136,30 @@
}
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 @@
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.
diff --git a/absl/strings/str_format_test.cc b/absl/strings/str_format_test.cc
index 36def1e..5198fb3 100644
--- a/absl/strings/str_format_test.cc
+++ b/absl/strings/str_format_test.cc
@@ -1142,18 +1142,51 @@
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
