Fix behaviors of StrCat() and StrFormat() regarding char types and enum types.

This is a conservative change, in that it only contains bugfixes and allows new patterns that used to be compile errors.  There are other changes we would like to make (as reflected in the comments in char_formatting_test.cc), but we are being careful about the implications of such behavior changes.

The two implementation changes are:
* Apply integral promotions to enums before printing them, so they are always treated as integers (and not chars) by StrCat and StrFormat.
* Classify `unsigned char` and `signed char` as integer-like rather than char-like, so that `StrFormat("%v", v)` accepts those types as integers (consistent with `StrCat()`.)

The behavior changes are:
* StrCat() now accepts char-backed enums (rather than failing to compile).
* StrFormat("%v") now accepts `signed char` and `unsigned char` as integral (rather than failing to compile).
* StrFormat("%v") now correctly formats 8-bit enums as integers (rather than failing internally and emitting nothing).

Tested:
  Modified the char_formatting_test.cc case to reflect changes.
  Re-ran all other tests.
PiperOrigin-RevId: 539659674
Change-Id: Ief56335f5a57e4582af396d00eaa9e7b6be4ddc6

5 files changed, 48 insertions, 57 deletions

diff --git a/absl/strings/char_formatting_test.cc b/absl/strings/char_formatting_test.cc
index 60416af3..1692da70 100644
--- a/absl/strings/char_formatting_test.cc
+++ b/absl/strings/char_formatting_test.cc
@@ -37,17 +37,12 @@ TEST(CharFormatting, CharEnum) {
   auto v = static_cast<CharEnum>('A');
 
   // Desired behavior: format as decimal
-  // (No APIs do this today.)
-
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
-
-  // Legacy behavior: does not compile:
-  // EXPECT_EQ(absl::StrCat(ch, "B"), "AB");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
+  EXPECT_EQ(absl::StrCat(v, "B"), "65B");
 
   // Legacy behavior: format as character:
 
-  // Some older versions of gcc behave differently in this one case.
+  // Some older versions of gcc behave differently in this one case
 #if !defined(__GNUC__) || defined(__clang__)
   EXPECT_EQ(absl::Substitute("$0B", v), "AB");
 #endif
@@ -57,14 +52,9 @@ enum class CharEnumClass: char {};
 TEST(CharFormatting, CharEnumClass) {
   auto v = static_cast<CharEnumClass>('A');
 
-  // Desired behavior: format as decimal
-  // (No APIs do this today.)
-
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
-
-  // Legacy behavior: does not compile:
-  // EXPECT_EQ(absl::StrCat(ch, "B"), "AB");
+  // Desired behavior: format as decimal:
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
+  EXPECT_EQ(absl::StrCat(v, "B"), "65B");
 
   // Legacy behavior: format as character:
   EXPECT_EQ(absl::Substitute("$0B", v), "AB");
@@ -76,9 +66,7 @@ TEST(CharFormatting, UnsignedChar) {
   // Desired behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
-
-  // Legacy behavior: does not compile:
-  // EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 
   // Signedness check
   const unsigned char w = 255;
@@ -93,9 +81,7 @@ TEST(CharFormatting, SignedChar) {
   // Desired behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
-
-  // Legacy behavior: does not compile:
-  // EXPECT_EQ(absl::StrFormat("%vB", v), "AB");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 
   // Signedness check
   const signed char w = -128;
@@ -110,14 +96,13 @@ TEST(CharFormatting, UnsignedCharEnum) {
   // Desired behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
-
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 
   // Signedness check
   auto w = static_cast<UnsignedCharEnum>(255);
   EXPECT_EQ(absl::StrCat(w, "B"), "255B");
   EXPECT_EQ(absl::Substitute("$0B", w), "255B");
+  EXPECT_EQ(absl::StrFormat("%vB", w), "255B");
 }
 
 enum SignedCharEnum : signed char {};
@@ -127,14 +112,13 @@ TEST(CharFormatting, SignedCharEnum) {
   // Desired behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
-
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 
   // Signedness check
   auto w = static_cast<SignedCharEnum>(-128);
   EXPECT_EQ(absl::StrCat(w, "B"), "-128B");
   EXPECT_EQ(absl::Substitute("$0B", w), "-128B");
+  EXPECT_EQ(absl::StrFormat("%vB", w), "-128B");
 }
 
 enum class UnsignedCharEnumClass : unsigned char {};
@@ -144,14 +128,13 @@ TEST(CharFormatting, UnsignedCharEnumClass) {
   // Desired behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
-
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 
   // Signedness check
   auto w = static_cast<UnsignedCharEnumClass>(255);
   EXPECT_EQ(absl::StrCat(w, "B"), "255B");
   EXPECT_EQ(absl::Substitute("$0B", w), "255B");
+  EXPECT_EQ(absl::StrFormat("%vB", w), "255B");
 }
 
 enum SignedCharEnumClass : signed char {};
@@ -161,14 +144,13 @@ TEST(CharFormatting, SignedCharEnumClass) {
   // Desired behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
-
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 
   // Signedness check
   auto w = static_cast<SignedCharEnumClass>(-128);
   EXPECT_EQ(absl::StrCat(w, "B"), "-128B");
   EXPECT_EQ(absl::Substitute("$0B", w), "-128B");
+  EXPECT_EQ(absl::StrFormat("%vB", w), "-128B");
 }
 
 #ifdef __cpp_lib_byte
@@ -177,12 +159,10 @@ TEST(CharFormatting, StdByte) {
   // Desired behavior: format as 0xff
   // (No APIs do this today.)
 
-  // BUG: internally fails
-  EXPECT_EQ(absl::StrFormat("%vB", v), "");
-
   // Legacy behavior: format as decimal:
   EXPECT_EQ(absl::StrCat(v, "B"), "65B");
   EXPECT_EQ(absl::Substitute("$0B", v), "65B");
+  EXPECT_EQ(absl::StrFormat("%vB", v), "65B");
 }
 #endif  // _cpp_lib_byte
 
diff --git a/absl/strings/internal/str_format/arg.cc b/absl/strings/internal/str_format/arg.cc
index 6033a75a..7f4057f9 100644
--- a/absl/strings/internal/str_format/arg.cc
+++ b/absl/strings/internal/str_format/arg.cc
@@ -461,18 +461,18 @@ CharConvertResult FormatConvertImpl(char v, const FormatConversionSpecImpl conv,
                                     FormatSinkImpl *sink) {
   return {ConvertIntArg(v, conv, sink)};
 }
-CharConvertResult FormatConvertImpl(signed char v,
-                                    const FormatConversionSpecImpl conv,
-                                    FormatSinkImpl *sink) {
+
+// ==================== Ints ====================
+IntegralConvertResult FormatConvertImpl(signed char v,
+                                        const FormatConversionSpecImpl conv,
+                                        FormatSinkImpl *sink) {
   return {ConvertIntArg(v, conv, sink)};
 }
-CharConvertResult FormatConvertImpl(unsigned char v,
-                                    const FormatConversionSpecImpl conv,
-                                    FormatSinkImpl *sink) {
+IntegralConvertResult FormatConvertImpl(unsigned char v,
+                                        const FormatConversionSpecImpl conv,
+                                        FormatSinkImpl *sink) {
   return {ConvertIntArg(v, conv, sink)};
 }
-
-// ==================== Ints ====================
 IntegralConvertResult FormatConvertImpl(short v,  // NOLINT
                                         const FormatConversionSpecImpl conv,
                                         FormatSinkImpl *sink) {
diff --git a/absl/strings/internal/str_format/arg.h b/absl/strings/internal/str_format/arg.h
index e4b16628..3ce30feb 100644
--- a/absl/strings/internal/str_format/arg.h
+++ b/absl/strings/internal/str_format/arg.h
@@ -279,14 +279,14 @@ FloatingConvertResult FormatConvertImpl(long double v,
 // Chars.
 CharConvertResult FormatConvertImpl(char v, FormatConversionSpecImpl conv,
                                     FormatSinkImpl* sink);
-CharConvertResult FormatConvertImpl(signed char v,
-                                    FormatConversionSpecImpl conv,
-                                    FormatSinkImpl* sink);
-CharConvertResult FormatConvertImpl(unsigned char v,
-                                    FormatConversionSpecImpl conv,
-                                    FormatSinkImpl* sink);
 
 // Ints.
+IntegralConvertResult FormatConvertImpl(signed char v,
+                                        FormatConversionSpecImpl conv,
+                                        FormatSinkImpl* sink);
+IntegralConvertResult FormatConvertImpl(unsigned char v,
+                                        FormatConversionSpecImpl conv,
+                                        FormatSinkImpl* sink);
 IntegralConvertResult FormatConvertImpl(short v,  // NOLINT
                                         FormatConversionSpecImpl conv,
                                         FormatSinkImpl* sink);
@@ -441,7 +441,7 @@ class FormatArgImpl {
   // For everything else:
   //   - Decay char* and char arrays into `const char*`
   //   - Decay any other pointer to `const void*`
-  //   - Decay all enums to their underlying type.
+  //   - Decay all enums to the integral promotion of their underlying type.
   //   - Decay function pointers to void*.
   template <typename T, typename = void>
   struct DecayType {
@@ -461,7 +461,7 @@ class FormatArgImpl {
                        !str_format_internal::HasUserDefinedConvert<T>::value &&
                        !strings_internal::HasAbslStringify<T>::value &&
                        std::is_enum<T>::value>::type> {
-    using type = typename std::underlying_type<T>::type;
+    using type = decltype(+typename std::underlying_type<T>::type());
   };
 
  public:
diff --git a/absl/strings/str_cat.h b/absl/strings/str_cat.h
index fcd48c4e..d5f71ff0 100644
--- a/absl/strings/str_cat.h
+++ b/absl/strings/str_cat.h
@@ -374,14 +374,24 @@ class AlphaNum {
   const char* data() const { return piece_.data(); }
   absl::string_view Piece() const { return piece_; }
 
-  // Normal enums are already handled by the integer formatters.
-  // This overload matches only scoped enums.
+  // Match unscoped enums.  Use integral promotion so that a `char`-backed
+  // enum becomes a wider integral type AlphaNum will accept.
   template <typename T,
             typename = typename std::enable_if<
-                std::is_enum<T>{} && !std::is_convertible<T, int>{} &&
+                std::is_enum<T>{} && std::is_convertible<T, int>{} &&
                 !strings_internal::HasAbslStringify<T>::value>::type>
   AlphaNum(T e)  // NOLINT(runtime/explicit)
-      : AlphaNum(static_cast<typename std::underlying_type<T>::type>(e)) {}
+      : AlphaNum(+e) {}
+
+  // This overload matches scoped enums.  We must explicitly cast to the
+  // underlying type, but use integral promotion for the same reason as above.
+  template <typename T,
+            typename std::enable_if<
+                std::is_enum<T>{} && !std::is_convertible<T, int>{} &&
+                    !strings_internal::HasAbslStringify<T>::value,
+                char*>::type = nullptr>
+  AlphaNum(T e)  // NOLINT(runtime/explicit)
+      : AlphaNum(+static_cast<typename std::underlying_type<T>::type>(e)) {}
 
   // vector<bool>::reference and const_reference require special help to
   // convert to `AlphaNum` because it requires two user defined conversions.
diff --git a/absl/strings/str_format.h b/absl/strings/str_format.h
index fc4bf39e..023e4350 100644
--- a/absl/strings/str_format.h
+++ b/absl/strings/str_format.h
@@ -259,6 +259,7 @@ class FormatCountCapture {
 // *   Characters: `char`, `signed char`, `unsigned char`
 // *   Integers: `int`, `short`, `unsigned short`, `unsigned`, `long`,
 //         `unsigned long`, `long long`, `unsigned long long`
+// *   Enums: printed as their underlying integral value
 // *   Floating-point: `float`, `double`, `long double`
 //
 // However, in the `str_format` library, a format conversion specifies a broader