base: Support using (D)CHECK on scoped enums.

base/logging.h

Index: base/logging.h
diff --git a/base/logging.h b/base/logging.h
index 0476a4450fb09453db113b38e59b71b5ff226fec..ce1bd4d2e9a3af719a675a628fe861ec3f10d8dc 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -11,6 +11,8 @@
 #include <cstring>
 #include <sstream>
 #include <string>
+#include <type_traits>
+#include <utility>
 
 #include "base/base_export.h"
 #include "base/debug/debugger.h"
@@ -519,17 +521,41 @@ class CheckOpResult {
 
 #endif  // !(OFFICIAL_BUILD && NDEBUG)
 
+// Uses expression SFINAE to detect whether using operator<< would work.
+template <typename T, typename = void>
+struct SupportsOstreamOperator : std::false_type {};

[danakj, 2016/05/20 21:54:16]

Can you move this out of logging.h. template_util.h perhaps?

It seems like we name type trait things in this_style to match the stl mostly,
maybe this should do the same as supports_ostream_operator. No strong feelings
either way though..

[jbroman, 2016/05/25 19:08:04]

Put it in base::internal since it's internal to base anyway, and the trait stuff
we have there matches Chromium naming.

+template <typename T>
+struct SupportsOstreamOperator<T,
+                               decltype(void(std::declval<std::ostream&>()

[jbroman, 2016/05/20 19:56:35]

"void" here because the type has to match the one above, and shouldn't depend on
what the return type of operator<< is, as long as that expression has a valid
type. (In C++17, I'd use std::void_t here, but being a little ad-hoc here seemed
fine.)

+                                             << std::declval<T>()))>
+    : std::true_type {};
+
 // This formats a value for a failing CHECK_XX statement.  Ordinarily,
 // it uses the definition for operator<<, with a few special cases below.
+
 template <typename T>
-inline void MakeCheckOpValueString(std::ostream* os, const T& v) {
+inline typename std::enable_if<SupportsOstreamOperator<const T&>::value,
+                               void>::type
+MakeCheckOpValueString(std::ostream* os, const T& v) {
   (*os) << v;
 }
 
-// We need an explicit specialization for std::nullptr_t.
-template <>
-BASE_EXPORT void MakeCheckOpValueString(std::ostream* os,
-                                        const std::nullptr_t& p);
+// We need overloads for enums that don't support operator<<.
+// (i.e. scoped enums where no operator<< overload was declared).
+template <typename T>
+inline typename std::enable_if<!SupportsOstreamOperator<const T&>::value &&

[danakj, 2016/05/20 21:54:16]

Random horrible thought: what would go wrong if we just used this overload
always when is_enum is true?

[jbroman, 2016/05/25 19:08:04]

We'd have to change the above condition to !std::is_enum<T>::value to avoid
multiple matching overloads, and we'd degrade behaviour of cases where a better
definition of operator<< than this one is available (some enums do define one).

I'd prefer to only substitute our own operator<< equivalent if none is already
defined, personally.

+                                   std::is_enum<T>::value,
+                               void>::type
+MakeCheckOpValueString(std::ostream* os, const T& v) {
+  // TODO(jbroman): Use std::underlying_type here, once it's available in all
+  // our STL implementations.
+  static_assert(sizeof(T) <= sizeof(int64_t),

[jbroman, 2016/05/20 19:56:35]

64 bits ought to be enough for anybody. :)

+                "enum can't be losslessly printed as int64_t");
+  (*os) << static_cast<int64_t>(v);

[dcheng, 2016/05/20 19:59:30]

What if the enum's underlying type is uint64_t? =)

[jbroman, 2016/05/20 20:05:38]

It'll do the obvious thing (values over 2^63 will become their corresponding
negative values). I think that's probably OK since this is only for DCHECK
messages, and that's an extreme edge case. I consider it an improvement
regardless, though we could of course implement underlying_type in base if we
really wanted to (Skia has, delegating to std::underlying_type when present and
using __underlying_type for libstdc++4.6, since our compiler supports the
underlying primitive anyways). It seemed more work than necessary, when we could
just wait for libstdc++ 4.6 to go away.

[danakj, 2016/05/20 21:08:23]

Oh, that might actually be nice, I didn't know __underlying_type exists. We have
some other missing libstdc++ stuff in
https://code.google.com/p/chromium/codesearch#chromium/src/base/template_util...

4.6 keeps dragging on, if we can do better it doesn't seem like much work to do
so.

[jbroman, 2016/05/25 19:08:04]

Done, with the same semi-hacky condition Skia uses.

+}
+
+// We need an explicit overload for std::nullptr_t.
+BASE_EXPORT void MakeCheckOpValueString(std::ostream* os, std::nullptr_t p);
 
 // Build the error message string.  This is separate from the "Impl"
 // function template because it is not performance critical and so can