logging: make objectToString() identical to toAppend() when available

Summary:
If `toAppend(std::string*, const Object&)` is defined for a particular object
type, make `objectToString()` simply return the exact output of `toAppend()`.

Previously the code returned `(type_name: toAppendResult)`

The new logic is based on the assumption that if `toAppend()` is implemented
for an object, it should emit all relevant information about the object that
may be needed by a developer.

The old behavior of always including type information was potentially helpful
in cases where `objectToString()` was called only in exceptional cases.
(e.g., after an exception thrown by `folly::format()`).  However it made
`objectToString()` too verbose in other non-exceptional situations where we
want to format objects (e.g., in `XCHECK_EQ()`).  There doesn't really seem to
be a strong need to keep the type information in the `folly::format()`
exception case, so this just drops it rather than adding two separate versions
of this function.

Reviewed By: chadaustin

Differential Revision: D14545303

fbshipit-source-id: 29e2f12d65fd22112486ed11f989b63875980088

folly/logging/ObjectToString.h

@@ -90,19 +90,12 @@ template <typename Arg>
 auto appendObjectToString(std::string& str, const Arg* arg, int) -> decltype(
     toAppend(std::declval<Arg>(), std::declval<std::string*>()),
     std::declval<void>()) {
-  str.push_back('(');
   try {
-#if FOLLY_HAS_RTTI
-    toAppend(folly::demangle(typeid(*arg)), &str);
-    str.append(": ");
-#endif
     toAppend(*arg, &str);
-  } catch (const std::exception& ex) {
-    str.append("<error_converting_to_string: ");
-    str.append(ex.what());
-    str.append(">");
+  } catch (const std::exception&) {
+    // If anything goes wrong in `toAppend()` fall back to appendRawObjectInfo()
+    ::folly::logging::appendRawObjectInfo(str, arg);
   }
-  str.push_back(')');
 }
 
 template <typename Arg>

folly/logging/test/LoggerTest.cpp

@@ -109,13 +109,12 @@ TEST_F(LoggerTest, follyFormatError) {
   ASSERT_EQ(1, messages.size());
   // Use a regex match here, since the type IDs are reported slightly
   // differently on different platforms.
-  EXPECT_THAT(
-      messages[0].first.getMessage(),
-      MatchesRegex(
-          R"(error formatting log message: )"
-          R"(invalid format argument \{:6.3f\}: invalid specifier 'f'; )"
-          R"(format string: "param1: \{:06d\}, param2: \{:6.3f\}", )"
-          R"(arguments: \((.*: )?1234\), \((.*: )?hello world\!\))"));
+  EXPECT_EQ(
+      R"(error formatting log message: )"
+      R"(invalid format argument {:6.3f}: invalid specifier 'f'; )"
+      R"(format string: "param1: {:06d}, param2: {:6.3f}", )"
+      R"(arguments: 1234, hello world!)",
+      messages[0].first.getMessage());
   EXPECT_EQ("LoggerTest.cpp", pathBasename(messages[0].first.getFileName()));
   EXPECT_EQ(LogLevel::WARN, messages[0].first.getLevel());
   EXPECT_FALSE(messages[0].first.containsNewlines());
@@ -216,9 +215,8 @@ TEST_F(LoggerTest, formatFallbackError) {
           R"(error formatting log message: invalid format argument \{\}: )"
           R"(argument index out of range, max=2; )"
           R"(format string: "param1: \{\}, param2: \{\}, \{\}", )"
-          R"(arguments: \((.*: )?1234\), )"
-          R"(\((.*ToStringFailure.*: )?<error_converting_to_string: )"
-          R"(error converting ToStringFailure object to a string>\))"));
+          R"(arguments: 1234, )"
+          R"(\[(.*ToStringFailure.*|object) of size (.*):.*\])"));
   EXPECT_EQ("LoggerTest.cpp", pathBasename(messages[0].first.getFileName()));
   EXPECT_EQ(LogLevel::WARN, messages[0].first.getLevel());
   EXPECT_FALSE(messages[0].first.containsNewlines());
@@ -235,7 +233,7 @@ TEST_F(LoggerTest, formatFallbackUnsupported) {
   auto expectedRegex =
       R"(error formatting log message: test; )"
       R"(format string: "param1: \{\}, param2: \{\}", )"
-      R"(arguments: \((.*: )?1234\), )"
+      R"(arguments: 1234, )"
       R"(\[(.*FormattableButNoToString.*|object) of size 4: )" +
       objectHex + R"(\])";
 
@@ -351,12 +349,11 @@ TEST_F(LoggerTest, logMacros) {
   // Bad format arguments should not throw
   FB_LOGF(footest1234, ERR, "whoops: {}, {}", getValue());
   ASSERT_EQ(1, messages.size());
-  EXPECT_THAT(
-      messages[0].first.getMessage(),
-      MatchesRegex(
-          R"(error formatting log message: invalid format argument \{\}: )"
-          R"(argument index out of range, max=1; )"
-          R"(format string: "whoops: \{\}, \{\}", arguments: \((.*: )?5\))"));
+  EXPECT_EQ(
+      R"(error formatting log message: invalid format argument {}: )"
+      R"(argument index out of range, max=1; )"
+      R"(format string: "whoops: {}, {}", arguments: 5)",
+      messages[0].first.getMessage());
   messages.clear();
 }