Convert the enum objects in cc/scheduler/scheduler_state_machine.h to C++ "enum class" objects

Convert the enum objects in cc/scheduler/scheduler_state_machine.h to C++ "enum class" objects

The cc/scheduler/scheduler_state_machine uses a lot of enums.
Those enums are not real types and hence have subtle problems.
C++ introduced the concept of "enum class" types which would be better replacements.

BUG=440759

[Shanmuga Pandi, 2015-01-07 11:49:22]

shanmuga.m@samsung.com changed reviewers:
+ brianderson@chromium.org, skyostil@chromium.org

[Shanmuga Pandi, 2015-01-07 11:49:23]

Please review this

[Sami, 2015-01-07 14:13:03]

Thanks for the cleanup. Overall I like this change. Just one question about the
operator overloads.

https://codereview.chromium.org/835783003/diff/1/cc/scheduler/scheduler_state...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/835783003/diff/1/cc/scheduler/scheduler_state...
cc/scheduler/scheduler_state_machine.h:54: friend ::std::ostream&
operator<<(::std::ostream& os,
It's unfortunate that we need to add these just for gtest. I wonder if there's a
gtest bug with enum classes that should be fixed instead?

[jamesr, 2015-01-07 22:32:00]

jamesr@chromium.org changed reviewers:
+ jamesr@chromium.org

[jamesr, 2015-01-07 22:32:00]

https://codereview.chromium.org/835783003/diff/1/cc/scheduler/scheduler_state...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/835783003/diff/1/cc/scheduler/scheduler_state...
cc/scheduler/scheduler_state_machine.h:54: friend ::std::ostream&
operator<<(::std::ostream& os,
On 2015/01/07 14:13:03, Sami wrote:
> It's unfortunate that we need to add these just for gtest. I wonder if there's
a
> gtest bug with enum classes that should be fixed instead?

enum classes, unlike enums, aren't implicitly convertible to integers so if
gtest wants to print it out it has to be taught a way to do so.

instead of having this friend stuff, imo it's better to declare a PrintTo(const
T&, std::ostream*) function in the same namespace as T so ADL can find it. with
this model you can out-of-line the stream manipulations instead of putting the
super bloaty ostream template instantiations in the header file

[Shanmuga Pandi, 2015-01-08 05:13:30]

https://codereview.chromium.org/835783003/diff/1/cc/scheduler/scheduler_state...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/835783003/diff/1/cc/scheduler/scheduler_state...
cc/scheduler/scheduler_state_machine.h:54: friend ::std::ostream&
operator<<(::std::ostream& os,
On 2015/01/07 22:32:00, jamesr wrote:
> On 2015/01/07 14:13:03, Sami wrote:
> > It's unfortunate that we need to add these just for gtest. I wonder if
there's
> a
> > gtest bug with enum classes that should be fixed instead?
> 
> enum classes, unlike enums, aren't implicitly convertible to integers so if
> gtest wants to print it out it has to be taught a way to do so.
> 
> instead of having this friend stuff, imo it's better to declare a
PrintTo(const
> T&, std::ostream*) function in the same namespace as T so ADL can find it.
with
> this model you can out-of-line the stream manipulations instead of putting the
> super bloaty ostream template instantiations in the header file


I think PrintTo will solve gtest print issue.
But DCHECK_EQ() also need operator << oveloads.
If I not overload <<, then DCHECK_EQ gives compilation error.

[Sami, 2015-01-08 13:44:11]

On 2015/01/08 05:13:30, Shanmuga Pandi wrote:
> I think PrintTo will solve gtest print issue.
> But DCHECK_EQ() also need operator << oveloads.
> If I not overload <<, then DCHECK_EQ gives compilation error.

Here's something Alex and I put together that works around this. Unfortunately
we can't use it because it relies on c++11 library support. Maybe we should
postpone this conversion until the libraries arrive?

diff --git a/base/logging.h b/base/logging.h
index 6a1df76..11d969e 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -9,6 +9,7 @@
 #include <string>
 #include <cstring>
 #include <sstream>
+#include <type_traits>
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
@@ -493,6 +494,24 @@ const LogSeverity LOG_0 = LOG_ERROR;
 
 #endif
 
+template<typename T, typename = void>
+struct UnderlyingTypeConverter
+{
+   static const T& convert(const T& value) {
+     return value;
+   }
+};
+
+template<typename T>
+struct UnderlyingTypeConverter<T,
+    typename std::enable_if<std::is_enum<T>::value>::type>
+{
+   typedef typename std::underlying_type<T>::type UnderlyingType;
+   static UnderlyingType convert(T value) {
+     return static_cast<UnderlyingType>(value);
+   }
+};
+
 // Build the error message string.  This is separate from the "Impl"
 // function template because it is not performance critical and so can
 // be out of line, while the "Impl" code should be inline.  Caller
@@ -500,7 +519,9 @@ const LogSeverity LOG_0 = LOG_ERROR;
 template<class t1, class t2>
 std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
   std::ostringstream ss;
-  ss << names << " (" << v1 << " vs. " << v2 << ")";
+  ss << names << " (" <<
+      UnderlyingTypeConverter<t1>::convert(v1) << " vs. " <<
+      UnderlyingTypeConverter<t2>::convert(v2) << ")";
   std::string* msg = new std::string(ss.str());
   return msg;
 }

[mithro-old, 2015-02-27 02:25:02]

mithro@mithis.com changed reviewers:
+ mithro@mithis.com