From 7765f9e3df5df49af9e834d10f16d7e004a04c4c Mon Sep 17 00:00:00 2001
From: Evan Shrubsole <eshr@webrtc.org>
Date: Mon, 25 Apr 2022 11:04:12 +0200
Subject: [PATCH] Fix logging MakeVal template

Types with ToLogString implemented were not being recognized correctly.
Now types like TimeDelta and Timestamp can be logged as normal.

Change-Id: Ia15f90bdd1d63a39f7452f9b4bba178d01b74352
Bug: webrtc:13995
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259863
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36646}
---
 rtc_base/logging.h           | 17 ++++++++++-------
 rtc_base/logging_unittest.cc | 16 ++++++++++++++++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/rtc_base/logging.h b/rtc_base/logging.h
index a1339aaee5..13738f0d0e 100644
--- a/rtc_base/logging.h
+++ b/rtc_base/logging.h
@@ -49,6 +49,7 @@
 #include <atomic>
 #include <sstream>  // no-presubmit-check TODO(webrtc:8982)
 #include <string>
+#include <type_traits>
 #include <utility>
 
 #include "absl/base/attributes.h"
@@ -277,8 +278,15 @@ inline Val<LogArgType::kLogMetadataTag, LogMetadataTag> MakeVal(
 template <typename T, class = void>
 struct has_to_log_string : std::false_type {};
 template <typename T>
-struct has_to_log_string<T, decltype(ToLogString(std::declval<T>()))>
-    : std::true_type {};
+struct has_to_log_string<T,
+                         absl::enable_if_t<std::is_convertible<
+                             decltype(ToLogString(std::declval<T>())),
+                             std::string>::value>> : std::true_type {};
+
+template <typename T, absl::enable_if_t<has_to_log_string<T>::value>* = nullptr>
+ToStringVal MakeVal(const T& x) {
+  return {ToLogString(x)};
+}
 
 // Handle arbitrary types other than the above by falling back to stringstream.
 // TODO(bugs.webrtc.org/9278): Get rid of this overload when callers don't need
@@ -300,11 +308,6 @@ ToStringVal MakeVal(const T& x) {
   return {os.str()};
 }
 
-template <typename T, absl::enable_if_t<has_to_log_string<T>::value>* = nullptr>
-ToStringVal MakeVal(const T& x) {
-  return {ToLogString(x)};
-}
-
 #if RTC_LOG_ENABLED()
 void Log(const LogArgType* fmt, ...);
 #else
diff --git a/rtc_base/logging_unittest.cc b/rtc_base/logging_unittest.cc
index cd8d753fb3..c267778951 100644
--- a/rtc_base/logging_unittest.cc
+++ b/rtc_base/logging_unittest.cc
@@ -22,6 +22,7 @@
 #include "rtc_base/event.h"
 #include "rtc_base/platform_thread.h"
 #include "rtc_base/time_utils.h"
+#include "test/gmock.h"
 #include "test/gtest.h"
 
 namespace rtc {
@@ -299,5 +300,20 @@ TEST(LogTest, NoopSeverityDoesNotRunStringFormatting) {
   EXPECT_FALSE(was_called);
 }
 
+struct TestStruct {};
+std::string ToLogString(TestStruct foo) {
+  return "bar";
+}
+
+TEST(LogTest, ToLogStringUsedForUnknownTypes) {
+  std::string str;
+  LogSinkImpl stream(&str);
+  LogMessage::AddLogToStream(&stream, LS_INFO);
+  TestStruct t;
+  RTC_LOG(LS_INFO) << t;
+  EXPECT_THAT(str, ::testing::HasSubstr("bar"));
+  LogMessage::RemoveLogToStream(&stream);
+}
+
 }  // namespace rtc
 #endif  // RTC_LOG_ENABLED()