[PATCH 0/3] enum_flags: Fix problems and add comprehensive unit tests

[PATCH 0/3] enum_flags: Fix problems and add comprehensive unit tests

[Pedro Alves]

Recently, while working on making symfile_add_flags and objfile->flags
strongly typed [1], I noticed a few enum_flags issues, like this
failing to compile:

  symfile_add_flags add_flags = (SYMFILE_MAINLINE
                                 | current_inferior ()->symfile_flags);

while the form that landed in master does compile:

  symfile_add_flags add_flags = (current_inferior ()->symfile_flags
                                 | SYMFILE_MAINLINE);
  
This series started out by wanting to fix that, but it ended up fixing
a bunch more, and adding comprehensive enum_flags unit tests along the
way.  Writing the tests in turn exposed more problems.  Rinse, repeat.

The enum_flags methods and global operators are made constexpr where
possible, and then C++11's deleted functions are used to remove
overloads that should not compile.  The unit tests then build on
SFINAE + decltype + constexpr to check that mixing enum flags types
incorrectly would really fail to compile.

This series makes use of C++11 extensively: decltype, constexpr,
=delete/=default, typedef -> type alias / using, static_assert, and
more.

[1] https://sourceware.org/ml/gdb-patches/2016-10/msg00715.html

Pedro Alves (3):
  enum_flags: Use C++11 std::underlying_type
  enum_flags: Fix problems and add comprehensive unit tests
  enum_flags: Fix ternary operator and remove implicit convertion to raw
    enum

 gdb/Makefile.in               |   2 +-
 gdb/btrace.c                  |   4 +-
 gdb/common/enum-flags.h       | 251 ++++++++++++-------
 gdb/compile/compile-c-types.c |   2 +-
 gdb/enum-flags-selftests.c    | 557 ++++++++++++++++++++++++++++++++++++++++++
 gdb/go-exp.y                  |   2 +-
 gdb/record-btrace.c           |  10 +-
 gdb/selftest.h                |  11 +
 8 files changed, 738 insertions(+), 101 deletions(-)
 create mode 100644 gdb/enum-flags-selftests.c

-- 
2.5.5

[PATCH 3/3] enum_flags: Fix ternary operator and remove implicit convertion to raw enum

[Pedro Alves]

The unit tests added by the previous patch revealed that this useful
use case doesn't work:

    enum flag { FLAG1 = 1, FLAG2 = 2 };
    enum_flags<flag> src = FLAG1;
    enum_flags<flag> f1 = condition ? src : FLAG2;

It fails to compile because enum_flags<flag> and flag are convertible
to each other.

Turns out that making enum_flags be implicitly convertible to the
backing raw enum type was not a good idea.

If we make it convertible to the underlying type instead, we fix that
ternary operator use case, and, we find cases throughout the codebase
that should be using the enum_flags but were using the raw backing
enum instead.  So it's a good change overall.

There's one case in compile/compile-c-types.c where we need to call a
function in a C API that expects the raw enum.  To address cases like
that, this adds a "raw()" method to enum_flags.  This way we can keep
using the safer enum_flags to construct the value, and then be
explicit when we need to get at the raw enum.

Tested with gcc 4.8, 4.9, 5.3, 7 (trunk) and clang 3.7.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* common/enum-flags.h (enum_flags::operator&=): Change parameter
	type to enum_flags from enum_type and adjust.
	(enum_flags::operator|=): Likewise.
	(enum_flags::operator^=): Likewise.
	(enum_flags::operator enum_type): Delete.
	(enum_flags::raw): New method.
	(ENUM_FLAGS_GEN_BINOP): Adjust operator functions.
	* compile/compile-c-types.c (convert_qualified): Use
	enum_flags::raw.
	* enum-flags-selftests.c Adjust ternary operator CHECK_VALID
	tests.
	(selftests::enum_flags_tests::self_test): Add more ternary
	operator tests.
	* record-btrace.c (btrace_thread_flag_to_str): Change parameter's
	type to btrace_thread_flags from btrace_thread_flag.
	(record_btrace_cancel_resume, record_btrace_step_thread): Change
	local's type to btrace_thread_flags from btrace_thread_flag.  Add
	cast in DEBUG call.
---
 gdb/btrace.c                  |  4 +--
 gdb/common/enum-flags.h       | 26 +++++++++++--------
 gdb/compile/compile-c-types.c |  2 +-
 gdb/enum-flags-selftests.c    | 60 +++++++++++++++++++++++++++++++------------
 gdb/record-btrace.c           | 10 ++++----
 5 files changed, 67 insertions(+), 35 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 39d537c..2717ae2 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -229,7 +229,7 @@ ftrace_new_function (struct btrace_function *prev,
 static void
 ftrace_update_caller (struct btrace_function *bfun,
 		      struct btrace_function *caller,
-		      enum btrace_function_flag flags)
+		      btrace_function_flags flags)
 {
   if (bfun->up != NULL)
     ftrace_debug (bfun, "updating caller");
@@ -246,7 +246,7 @@ ftrace_update_caller (struct btrace_function *bfun,
 static void
 ftrace_fixup_caller (struct btrace_function *bfun,
 		     struct btrace_function *caller,
-		     enum btrace_function_flag flags)
+		     btrace_function_flags flags)
 {
   struct btrace_function *prev, *next;
 
diff --git a/gdb/common/enum-flags.h b/gdb/common/enum-flags.h
index 9e128c3..2939a3a 100644
--- a/gdb/common/enum-flags.h
+++ b/gdb/common/enum-flags.h
@@ -114,24 +114,28 @@ public:
     : m_enum_value ((enum_type) 0)
   {}
 
-  enum_flags &operator&= (enum_type e)
+  enum_flags &operator&= (enum_flags e)
   {
-    m_enum_value = (enum_type) (m_enum_value & e);
+    m_enum_value = (enum_type) (m_enum_value & e.m_enum_value);
     return *this;
   }
-  enum_flags &operator|= (enum_type e)
+  enum_flags &operator|= (enum_flags e)
   {
-    m_enum_value = (enum_type) (m_enum_value | e);
+    m_enum_value = (enum_type) (m_enum_value | e.m_enum_value);
     return *this;
   }
-  enum_flags &operator^= (enum_type e)
+  enum_flags &operator^= (enum_flags e)
   {
-    m_enum_value = (enum_type) (m_enum_value ^ e);
+    m_enum_value = (enum_type) (m_enum_value ^ e.m_enum_value);
     return *this;
   }
 
-  /* Allow conversion to the enum type.  */
-  constexpr operator enum_type () const
+  /* Like raw enums, allow conversion to the underlying type.  */
+  constexpr operator underlying_type () const
+  { return m_enum_value; }
+
+  /* Get the underlying value as a raw enum.  */
+  constexpr enum_type raw () const
   { return m_enum_value; }
 
   /* Binary operations involving some unrelated type (which would be a
@@ -167,19 +171,19 @@ private:
   template <typename enum_type>						\
   constexpr typename enum_flags_type<enum_type>::type			\
   OPERATOR_OP (enum_flags<enum_type> e1, enum_type e2)			\
-  { return enum_type (e1) OP e2; }					\
+  { return e1.raw () OP e2; }						\
 									\
   /* enum_flags on the RHS.  */						\
   template <typename enum_type>						\
   constexpr typename enum_flags_type<enum_type>::type			\
   OPERATOR_OP (enum_type e1, enum_flags<enum_type> e2)			\
-  { return e1 OP enum_type (e2); }					\
+  { return e1 OP e2.raw (); }						\
 									\
   /* enum_flags on both LHS/RHS.  */					\
   template <typename enum_type>						\
   constexpr typename enum_flags_type<enum_type>::type			\
   OPERATOR_OP (enum_flags<enum_type> e1, enum_flags<enum_type> e2)	\
-  { return enum_type (e1) OP enum_type (e2); }				\
+  { return e1.raw () OP e2.raw (); }					\
 									\
   /* Delete cases involving unrelated types.  */			\
 									\
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 505bce6..6cf1dc7 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -306,7 +306,7 @@ convert_qualified (struct compile_c_instance *context, struct type *type)
 
   return C_CTX (context)->c_ops->build_qualified_type (C_CTX (context),
 						       unqual_converted,
-						       quals);
+						       quals.raw ());
 }
 
 /* Convert a complex type to its gcc representation.  */
diff --git a/gdb/enum-flags-selftests.c b/gdb/enum-flags-selftests.c
index 32c8b7a..101ad56 100644
--- a/gdb/enum-flags-selftests.c
+++ b/gdb/enum-flags-selftests.c
@@ -146,7 +146,7 @@ CHECK_VALID (true,  RE,   RE (1))
 CHECK_VALID (true,  RE,   RE (RE2 ()))
 CHECK_VALID (false, void, RE (EF2 ()))
 CHECK_VALID (true,  RE,   RE (RE ()))
-CHECK_VALID (true,  RE,   RE (EF ()))
+CHECK_VALID (false, void, RE (EF ()))
 
 /* other -> EF.  */
 
@@ -268,23 +268,30 @@ CHECK_VALID (true,  EF,   ~EF ())
 
 /* Check ternary operator.  This exercises implicit conversions.  */
 
-/* Invalid since each type can be converted to the other.  */
-/* GCC 4.8 incorrectly fails to compile this test with:
-     error: operands to ?: have different types ‘enum_flags<RE>’ and ‘RE’
-   Confirmed to compile/pass with gcc 4.9, 5.3 and clang 3.7.
-*/
-#if GCC_VERSION >= 4009
-CHECK_VALID (false, void, true ? EF () : RE ())
-CHECK_VALID (false, void, true ? RE () : EF ())
-#endif
+CHECK_VALID (true,  EF,   true ? EF () : RE ())
+CHECK_VALID (true,  EF,   true ? RE () : EF ())
+
+/* Both enums should have the same underlying type.  Pick any.  */
+typedef std::underlying_type<RE>::type und;
 
 /* These are valid, but it's not a big deal since you won't be able to
-   assign the resulting int to an enum or an enum_flags without a
-   cast.  */
-CHECK_VALID (true,  int,  true ? EF () : EF2 ())
-CHECK_VALID (true,  int,  true ? EF2 () : EF ())
-CHECK_VALID (true,  int,  true ? EF () : RE2 ())
-CHECK_VALID (true,  int,  true ? RE2 () : EF ())
+   assign the resulting integer to an enum or an enum_flags without a
+   cast.
+
+   The latter two tests are disabled on older GCCs because they
+   incorrectly fail to compile with gcc 4.8 and 4.9 at least, with:
+
+    no known conversion from ‘enum_flags<RE>::underlying_type {aka
+    unsigned int}’ to ‘enum_flags_tests::RE2’
+
+   They've been confirmed to compile/pass with gcc 5.3, gcc 7.1 and
+   clang 3.7.  */
+CHECK_VALID (true,  und,  true ? EF () : EF2 ())
+CHECK_VALID (true,  und,  true ? EF2 () : EF ())
+#if GCC_VERSION >= 5003
+CHECK_VALID (true,  und,  true ? EF () : RE2 ())
+CHECK_VALID (true,  und,  true ? RE2 () : EF ())
+#endif
 
 /* Unfortunately this can't work due to the way C++ computes the
    return type of the ternary conditional operator.  int isn't
@@ -487,13 +494,34 @@ self_test ()
   }
 
   /* Check the ternary operator.  */
+
   {
+    /* raw enum, raw enum */
     constexpr test_flags f1 = true ? FLAG1 : FLAG2;
     STATIC_SELF_CHECK (f1 == FLAG1);
     constexpr test_flags f2 = false ? FLAG1 : FLAG2;
     STATIC_SELF_CHECK (f2 == FLAG2);
   }
 
+  {
+    /* enum flags, raw enum */
+    constexpr test_flags src = FLAG1;
+    constexpr test_flags f1 = true ? src : FLAG2;
+    STATIC_SELF_CHECK (f1 == FLAG1);
+    constexpr test_flags f2 = false ? src : FLAG2;
+    STATIC_SELF_CHECK (f2 == FLAG2);
+  }
+
+  {
+    /* enum flags, enum flags */
+    constexpr test_flags src1 = FLAG1;
+    constexpr test_flags src2 = FLAG2;
+    constexpr test_flags f1 = true ? src1 : src2;
+    STATIC_SELF_CHECK (f1 == src1);
+    constexpr test_flags f2 = false ? src1 : src2;
+    STATIC_SELF_CHECK (f2 == src2);
+  }
+
   /* Check that we can use operator| in switch cases, where only
      constants are allowed.  This should work because operator| is
      constexpr.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 7c0e39f..f68ce3c 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1863,7 +1863,7 @@ record_btrace_to_get_tailcall_unwinder (struct target_ops *self)
 /* Return a human-readable string for FLAG.  */
 
 static const char *
-btrace_thread_flag_to_str (enum btrace_thread_flag flag)
+btrace_thread_flag_to_str (btrace_thread_flags flag)
 {
   switch (flag)
     {
@@ -2165,7 +2165,7 @@ record_btrace_commit_resume (struct target_ops *ops)
 static void
 record_btrace_cancel_resume (struct thread_info *tp)
 {
-  enum btrace_thread_flag flags;
+  btrace_thread_flags flags;
 
   flags = tp->btrace.flags & (BTHR_MOVE | BTHR_STOP);
   if (flags == 0)
@@ -2173,7 +2173,7 @@ record_btrace_cancel_resume (struct thread_info *tp)
 
   DEBUG ("cancel resume thread %s (%s): %x (%s)",
 	 print_thread_id (tp),
-	 target_pid_to_str (tp->ptid), flags,
+	 target_pid_to_str (tp->ptid), (unsigned) flags,
 	 btrace_thread_flag_to_str (flags));
 
   tp->btrace.flags &= ~(BTHR_MOVE | BTHR_STOP);
@@ -2398,7 +2398,7 @@ record_btrace_step_thread (struct thread_info *tp)
 {
   struct btrace_thread_info *btinfo;
   struct target_waitstatus status;
-  enum btrace_thread_flag flags;
+  btrace_thread_flags flags;
 
   btinfo = &tp->btrace;
 
@@ -2406,7 +2406,7 @@ record_btrace_step_thread (struct thread_info *tp)
   btinfo->flags &= ~(BTHR_MOVE | BTHR_STOP);
 
   DEBUG ("stepping thread %s (%s): %x (%s)", print_thread_id (tp),
-	 target_pid_to_str (tp->ptid), flags,
+	 target_pid_to_str (tp->ptid), (unsigned) flags,
 	 btrace_thread_flag_to_str (flags));
 
   /* We can't step without an execution history.  */
-- 
2.5.5

[PATCH 1/3] enum_flags: Use C++11 std::underlying_type

[Pedro Alves]

Now that we can require C++11, we can use std::underlying_type instead
of rolling our own.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* common/enum-flags.h: Include <type_traits>.
	(integer_for_size, enum_underlying_type): Delete.
	(class enum_flags): Use std::underlying_type.
---
 gdb/common/enum-flags.h | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/gdb/common/enum-flags.h b/gdb/common/enum-flags.h
index 9b8c952..ff48067 100644
--- a/gdb/common/enum-flags.h
+++ b/gdb/common/enum-flags.h
@@ -51,6 +51,8 @@
 
 #ifdef __cplusplus
 
+#include <type_traits>
+
 /* Traits type used to prevent the global operator overloads from
    instantiating for non-flag enums.  */
 template<typename T> struct enum_flags_type {};
@@ -66,32 +68,12 @@ template<typename T> struct enum_flags_type {};
     typedef enum_flags<enum_type> type;			\
   }
 
-/* Until we can rely on std::underlying type being universally
-   available (C++11), roll our own for enums.  */
-template<int size, bool sign> class integer_for_size { typedef void type; };
-template<> struct integer_for_size<1, 0> { typedef uint8_t type; };
-template<> struct integer_for_size<2, 0> { typedef uint16_t type; };
-template<> struct integer_for_size<4, 0> { typedef uint32_t type; };
-template<> struct integer_for_size<8, 0> { typedef uint64_t type; };
-template<> struct integer_for_size<1, 1> { typedef int8_t type; };
-template<> struct integer_for_size<2, 1> { typedef int16_t type; };
-template<> struct integer_for_size<4, 1> { typedef int32_t type; };
-template<> struct integer_for_size<8, 1> { typedef int64_t type; };
-
-template<typename T>
-struct enum_underlying_type
-{
-  typedef typename
-    integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
-    type;
-};
-
 template <typename E>
 class enum_flags
 {
 public:
   typedef E enum_type;
-  typedef typename enum_underlying_type<enum_type>::type underlying_type;
+  typedef typename std::underlying_type<enum_type>::type underlying_type;
 
 private:
   /* Private type used to support initializing flag types with zero:
-- 
2.5.5

Re: [PATCH 1/3] enum_flags: Use C++11 std::underlying_type

[Simon Marchi]

On 2016-11-03 23:22, Pedro Alves wrote:
>  template <typename E>
>  class enum_flags
>  {
>  public:
>    typedef E enum_type;

I have a question about this, even though it's not a change part of this 
patch.  Is it useful to have a typedef to define enum_type?  Can't we 
directly use template <typename enum_type> above the class?

[PATCH 2/3] enum_flags: Fix problems and add comprehensive unit tests

[Pedro Alves]

This patch starts by adding comprehensive unit tests for enum_flags.

It adds:

 - tests of normal expected uses of the API.

 - checks that _invalid_ uses of the API would fail to compile.  I.e.,
   it validates that enum_flags really is a strong type, and that
   incorrect mixing of enum types would be caught at compile time.  It
   pulls that off making use of SFINEA and C++11's decltype/constexpr.

This revealed many holes in the enum_flags API.  For example, the f1
assignment below currently incorrectly fails to compile:

 enum_flags<flags> f1 = FLAG1;
 enum_flags<flags> f2 = FLAG2 | f1;

This hole and more are all plugged by this patch, by reworking how the
enum_flags operators are implemented, and making use of C++11's
feature of being able to delete methods/functions.

This makes most of the enum_flags operators constexpr.  Beyond
enabling more compiler optimizations and enabling the new unit tests,
this has other advantages, like making it possible to use operator|
with enum_flags values in switch cases, where only compile-time
constants are allowed:

    enum_flags<flags> f = FLAG1 | FLAG2;
    switch (f)
      {
      case FLAG1 | FLAG2:
	break;
      }

Currently that fails to compile.

This adds a STATIC_SELF_TEST macro to selftest.h, which is a variant
of SELF_TEST, but uses C++11's static_assert to do checking at compile
time.

To avoid potential ODR violations, since the tests add enums with
short names that could easily conflict with other names, the new
selftests are placed in a namespace (selftests::enum_flags_tests).  I
think that's a good practice that we should start following.  This
required splitting the global operator overload enablement out of the
DEF_ENUM_FLAGS_TYPE macro into a separate macro, because
DEF_ENUM_FLAGS_TYPE wants to create the enum flags typedef in the
global namespace too.

Tested with gcc 4.8, 4.9, 5.3, 7 (trunk) and clang 3.7.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* Makefile.in (COMMON_OBS): Add enum-flags-selftests.o.
	* common/enum-flags.h (ENABLE_ENUM_FLAGS_OPERATORS): New, factored
	out from DEF_ENUM_FLAGS_TYPE.
	(enum_flags::underlying_value): Delete.
	(enum_flags::enum_flags): Use default.
	(enum_flags::operator=): Delete.
	(enum_flags::enum_flags(enum_type e)): Now constexpr.
	(enum_flags::enum_flags(enum_flags::zero_type *zero)): Likewise.
	(enum_flags::operator&=(enum_type e)): No longer implement in
	terms of the underlying type here.
	(enum_flags::operator|=(enum_type e)): Likewise.
	(enum_flags::operator^=(enum_type e)): Likewise.
	(enum_flags::enum_type ()): Now constexpr.
	(enum_flags::enum_flags operator&(enum_type e)): Delete.
	(enum_flags::operator|(enum_type e)): Delete.
	(enum_flags::operator^(enum_type e)): Delete.
	(enum_flags::operator~()): Now constexpr.
	(operator&, operator|, operator^): Delete.
	(ENUM_FLAGS_GEN_BINOP, ENUM_FLAGS_GEN_COMPOUND_ASSIGN): New,
	reimplementing global operators.
	(operator~): Now constexpr and reimplemented.
	(operator<<, operator>>): New deleted functions.
	* enum-flags-selftests.c: New file.
	* go-exp.y (parse_string_or_char): Add cast to int.
	* selftest.h (SC_STRINGIZE_1, SC_STRINGIZE)
	(STATIC_SELF_CHECK_FAIL_MSG, STATIC_SELF_CHECK): New.
---
 gdb/Makefile.in            |   2 +-
 gdb/common/enum-flags.h    | 219 +++++++++++++------
 gdb/enum-flags-selftests.c | 529 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/go-exp.y               |   2 +-
 gdb/selftest.h             |  11 +
 5 files changed, 693 insertions(+), 70 deletions(-)
 create mode 100644 gdb/enum-flags-selftests.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 6db63c7..5f42783 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1088,7 +1088,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
 	print-utils.o rsp-low.o errors.o common-debug.o debug.o \
 	common-exceptions.o btrace-common.o fileio.o \
-	common-regcache.o new-op.o \
+	common-regcache.o new-op.o enum-flags-selftests.o \
 	$(SUBDIR_GCC_COMPILE_OBS)
 
 TSOBS = inflow.o
diff --git a/gdb/common/enum-flags.h b/gdb/common/enum-flags.h
index ff48067..9e128c3 100644
--- a/gdb/common/enum-flags.h
+++ b/gdb/common/enum-flags.h
@@ -49,6 +49,8 @@
     some_flags f = 1; // error
 */
 
+/* Comprehensive unit tests are found in gdb/enum-flags-selftests.c.  */
+
 #ifdef __cplusplus
 
 #include <type_traits>
@@ -57,17 +59,24 @@
    instantiating for non-flag enums.  */
 template<typename T> struct enum_flags_type {};
 
-/* Use this to mark an enum as flags enum.  It defines FLAGS as
-   enum_flags wrapper class for ENUM, and enables the global operator
-   overloads for ENUM.  */
-#define DEF_ENUM_FLAGS_TYPE(enum_type, flags_type)	\
-  typedef enum_flags<enum_type> flags_type;		\
+/* Use this to mark an enum as flags enum, enabling the global
+   operator overloads for ENUM_TYPE.  This must be called in the
+   global namespace.  */
+#define ENABLE_ENUM_FLAGS_OPERATORS(enum_type)		\
   template<>						\
   struct enum_flags_type<enum_type>			\
   {							\
     typedef enum_flags<enum_type> type;			\
   }
 
+/* Use this to mark an enum as flags enum.  It defines FLAGS_TYPE as
+   enum_flags wrapper class for ENUM, and enables the global operator
+   overloads for ENUM.  This must be called in the global
+   namespace.  */
+#define DEF_ENUM_FLAGS_TYPE(enum_type, flags_type)	\
+  typedef enum_flags<enum_type> flags_type;		\
+  ENABLE_ENUM_FLAGS_OPERATORS(enum_type);
+
 template <typename E>
 class enum_flags
 {
@@ -90,73 +99,48 @@ private:
      pass to the constructor is the NULL pointer, or, zero.  */
   struct zero_type;
 
-  underlying_type
-  underlying_value () const
-  {
-    return m_enum_value;
-  }
-
 public:
   /* Allow default construction, just like raw enums.  */
-  enum_flags ()
-  {}
-
-  enum_flags (const enum_flags &other)
-    : m_enum_value (other.m_enum_value)
-  {}
+  enum_flags () = default;
 
-  enum_flags &operator= (const enum_flags &other)
-  {
-    m_enum_value = other.m_enum_value;
-    return *this;
-  }
+  /* The default move/copy/assignment do the right thing.  */
 
   /* If you get an error saying these two overloads are ambiguous,
      then you tried to mix values of different enum types.  */
-  enum_flags (enum_type e)
+  constexpr enum_flags (enum_type e)
     : m_enum_value (e)
   {}
-  enum_flags (struct enum_flags::zero_type *zero)
+  constexpr enum_flags (enum_flags::zero_type *zero)
     : m_enum_value ((enum_type) 0)
   {}
 
   enum_flags &operator&= (enum_type e)
   {
-    m_enum_value = (enum_type) (underlying_value () & e);
+    m_enum_value = (enum_type) (m_enum_value & e);
     return *this;
   }
   enum_flags &operator|= (enum_type e)
   {
-    m_enum_value = (enum_type) (underlying_value () | e);
+    m_enum_value = (enum_type) (m_enum_value | e);
     return *this;
   }
   enum_flags &operator^= (enum_type e)
   {
-    m_enum_value = (enum_type) (underlying_value () ^ e);
+    m_enum_value = (enum_type) (m_enum_value ^ e);
     return *this;
   }
 
-  operator enum_type () const
-  {
-    return m_enum_value;
-  }
+  /* Allow conversion to the enum type.  */
+  constexpr operator enum_type () const
+  { return m_enum_value; }
 
-  enum_flags operator& (enum_type e) const
-  {
-    return (enum_type) (underlying_value () & e);
-  }
-  enum_flags operator| (enum_type e) const
-  {
-    return (enum_type) (underlying_value () | e);
-  }
-  enum_flags operator^ (enum_type e) const
-  {
-    return (enum_type) (underlying_value () ^ e);
-  }
-  enum_flags operator~ () const
-  {
-    return (enum_type) ~underlying_value ();
-  }
+  /* Binary operations involving some unrelated type (which would be a
+     bug) are implemented as non-members, and deleted.  */
+
+  /* Unary operators.  */
+
+  constexpr enum_flags operator~ () const
+  { return (enum_type) ~m_enum_value; }
 
 private:
   /* Stored as enum_type because GDB knows to print the bit flags
@@ -166,33 +150,132 @@ private:
 
 /* Global operator overloads.  */
 
-template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator& (enum_type e1, enum_type e2)
-{
-  return enum_flags<enum_type> (e1) & e2;
-}
+/* Generate binary operators.  */
+
+#define ENUM_FLAGS_GEN_BINOP(OPERATOR_OP, OP)				\
+									\
+  /* Raw enum on the both LHS/RHS.  Returns raw enum type.  */		\
+  template <typename enum_type>						\
+  constexpr typename enum_flags_type<enum_type>::type::enum_type	\
+  OPERATOR_OP (enum_type e1, enum_type e2)				\
+  {									\
+    using underlying = typename std::underlying_type<enum_type>::type;	\
+    return (enum_type) (underlying (e1) OP underlying (e2));		\
+  }									\
+									\
+  /* enum_flags on the LHS.  */						\
+  template <typename enum_type>						\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (enum_flags<enum_type> e1, enum_type e2)			\
+  { return enum_type (e1) OP e2; }					\
+									\
+  /* enum_flags on the RHS.  */						\
+  template <typename enum_type>						\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (enum_type e1, enum_flags<enum_type> e2)			\
+  { return e1 OP enum_type (e2); }					\
+									\
+  /* enum_flags on both LHS/RHS.  */					\
+  template <typename enum_type>						\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (enum_flags<enum_type> e1, enum_flags<enum_type> e2)	\
+  { return enum_type (e1) OP enum_type (e2); }				\
+									\
+  /* Delete cases involving unrelated types.  */			\
+									\
+  template <typename enum_type, typename unrelated_type>		\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (enum_type e1, unrelated_type e2) = delete;		\
+									\
+  template <typename enum_type, typename unrelated_type>		\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (unrelated_type e1, enum_type e2) = delete;		\
+									\
+  template <typename enum_type, typename unrelated_type>		\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (enum_flags<enum_type> e1, unrelated_type e2) = delete;	\
+									\
+  template <typename enum_type, typename unrelated_type>		\
+  constexpr typename enum_flags_type<enum_type>::type			\
+  OPERATOR_OP (unrelated_type e1, enum_flags<enum_type> e2) = delete;
+
+/* Generate non-member compound assignment operators.  Only the raw
+   enum versions are defined here.  The enum_flags versions are
+   defined as member functions, simply because it's less code that
+   way.
+
+   Note we delete operators that would allow e.g.,
+
+     "enum_type | 1" or "enum_type1 | enum_type2"
+
+   because that would allow a mistake like :
+     enum flags1 { F1_FLAGS1 = 1 };
+     enum flags2 { F2_FLAGS2 = 2 };
+     enum flags1 val;
+     switch (val) {
+       case F1_FLAGS1 | F2_FLAGS2:
+     ...
+
+   If you really need to 'or' different flags, cast to integer first.
+*/
+#define ENUM_FLAGS_GEN_COMPOUND_ASSIGN(OPERATOR_OP, OP)			\
+  /* lval reference version.  */					\
+  template <typename enum_type>						\
+  constexpr typename enum_flags_type<enum_type>::type::enum_type &	\
+  OPERATOR_OP (enum_type &e1, const enum_type &e2)			\
+  { return e1 = e1 OP e2; }						\
+									\
+  /* rval reference version.  */					\
+  template <typename enum_type>						\
+  constexpr typename enum_flags_type<enum_type>::type::enum_type &	\
+  OPERATOR_OP (enum_type &&e1, const enum_type &e2)			\
+  { return e1 = e1 OP e2; }						\
+									\
+  /* Delete assignment from unrelated types.  */			\
+									\
+  template <typename enum_type, typename other_enum_type>		\
+  constexpr typename enum_flags_type<enum_type>::type::enum_type &	\
+  OPERATOR_OP (enum_type &e1, const other_enum_type &e2) = delete;	\
+									\
+  template <typename enum_type, typename other_enum_type>		\
+  constexpr typename enum_flags_type<enum_type>::type::enum_type &	\
+  OPERATOR_OP (enum_type &&e1, const other_enum_type &e2) = delete;
+
+ENUM_FLAGS_GEN_BINOP(operator|, |)
+ENUM_FLAGS_GEN_BINOP(operator&, &)
+ENUM_FLAGS_GEN_BINOP(operator^, ^)
+
+ENUM_FLAGS_GEN_COMPOUND_ASSIGN(operator|=, |)
+ENUM_FLAGS_GEN_COMPOUND_ASSIGN(operator&=, &)
+ENUM_FLAGS_GEN_COMPOUND_ASSIGN(operator^=, ^)
+
+/* Unary operators for the raw flags enum.  */
 
 template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator| (enum_type e1, enum_type e2)
+constexpr typename enum_flags_type<enum_type>::type::enum_type
+operator~ (enum_type e)
 {
-  return enum_flags<enum_type> (e1) | e2;
+  using underlying = typename std::underlying_type<enum_type>::type;
+  return (enum_type) ~underlying (e);
 }
 
-template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator^ (enum_type e1, enum_type e2)
-{
-  return enum_flags<enum_type> (e1) ^ e2;
-}
+/* Delete operator<< and operator>>.  */
 
-template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator~ (enum_type e)
-{
-  return ~enum_flags<enum_type> (e);
-}
+template <typename enum_type, typename any_type>
+constexpr typename enum_flags_type<enum_type>::type
+operator<< (const enum_type &, const any_type &) = delete;
+
+template <typename enum_type, typename any_type>
+constexpr typename enum_flags_type<enum_type>::type
+operator<< (const enum_flags<enum_type> &, const any_type &) = delete;
+
+template <typename enum_type, typename any_type>
+constexpr typename enum_flags_type<enum_type>::type
+operator>> (const enum_type &, const any_type &) = delete;
+
+template <typename enum_type, typename any_type>
+constexpr typename enum_flags_type<enum_type>::type
+operator>> (const enum_flags<enum_type> &, const any_type &) = delete;
 
 #else /* __cplusplus */
 
diff --git a/gdb/enum-flags-selftests.c b/gdb/enum-flags-selftests.c
new file mode 100644
index 0000000..32c8b7a
--- /dev/null
+++ b/gdb/enum-flags-selftests.c
@@ -0,0 +1,529 @@
+/* Self tests for enum-flags for GDB, the GNU debugger.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "common/enum-flags.h"
+#include "selftest.h"
+
+#if GDB_SELF_TEST
+
+namespace selftests { namespace enum_flags_tests {
+
+/* The (real) enum types used in CHECK_VALID.  Their names match the
+   template parameter names of the templated defined by CHECK_VALID
+   just matching real vs template param type more obvious.  They could
+   be named differently.  */
+
+/* A "real enum".  */
+enum RE
+  {
+    RE_FLAG1 = 1 << 1,
+    RE_FLAG2 = 1 << 2,
+  };
+/* Another "real enum".  */
+enum RE2
+  {
+    RE2_FLAG1 = 1 << 1,
+    RE2_FLAG2 = 1 << 2,
+  };
+
+/* The corresponding "enum flags" types.  */
+using EF = enum_flags<RE>;
+using EF2 = enum_flags<RE2>;
+
+}} /* namespace selftests::enum_flags */
+
+ENABLE_ENUM_FLAGS_OPERATORS (selftests::enum_flags_tests::RE);
+ENABLE_ENUM_FLAGS_OPERATORS (selftests::enum_flags_tests::RE2);
+
+namespace selftests { namespace enum_flags_tests {
+
+/* A couple globals used as lvalues in the CHECK_VALID expressions
+   below.  Their names (and types) match the uppercase type names
+   exposed by CHECK_VALID just to make the expressions easier to
+   follow.  */
+static RE re ATTRIBUTE_UNUSED;
+static EF ef ATTRIBUTE_UNUSED;
+
+/* First, compile-time tests that:
+
+   - make sure that incorrect operations with mismatching enum types
+     are caught at compile time.
+
+   - make sure that the same operations but involving the right enum
+     types do compile and that they return the correct type.
+*/
+
+#define CONCAT_1(a, b) a ## b
+#define CONCAT(a, b) CONCAT_1 (a, b)
+
+/* Use SFINAE to detect whether EXPR expression is either valid or
+   ill-formed.  I.e., check that bad uses of enum-flags (e.g.,
+   involving mismatching enums) would be caught at compile time.  If
+   the expression is valid, also check whether the expression has the
+   right type.
+
+   EXPR must be defined in terms of some of the template parameters
+   EF, EF2, RE and RE2, so that template substitution failure discards
+   the overload instead of causing a real compile error.
+
+   EF/EF2 are enum flag types, and RE/RE2 are the corresponding raw
+   enum types.
+
+   VALID is a boolean that indicates whether the expression is
+   supposed to be valid or invalid.
+
+   EXPR_TYPE is expected type of EXPR.  Only meaningful if VALID is
+   true.  */
+#define CHECK_VALID(VALID, EXPR_TYPE, EXPR)				\
+  namespace CONCAT (check_valid, __LINE__) {				\
+									\
+  /* First, the validity check.  */					\
+									\
+  /* If the expression is valid (would compile), then this overload is
+     selected.  */	\
+  template <typename EF, typename RE, typename EF2, typename RE2>	\
+  static auto check_valid (std::nullptr_t) -> decltype (EXPR, char(0)); \
+									\
+  /* If the expression is ill-formed, then this overload is		\
+     selected.  */							\
+  template <typename EF, typename RE, typename EF2, typename RE2>	\
+  static std::array<char, 2> check_valid (...);				\
+									\
+  static constexpr bool valid						\
+    = (sizeof (check_valid<EF, RE, EF2, RE2> (nullptr)) == 1);		\
+  static_assert (valid == VALID, STATIC_SELF_CHECK_FAIL_MSG);		\
+									\
+  /* Now the expression type check.  */					\
+									\
+  template <typename EF, typename RE, typename EF2, typename RE2>	\
+  static constexpr auto check_is_same (std::nullptr_t)			\
+    -> decltype (EXPR, bool(false))					\
+  {									\
+    return std::is_same<EXPR_TYPE, decltype (EXPR)>::value;		\
+  }									\
+									\
+  template <typename EF, typename RE, typename EF2, typename RE2>	\
+  static constexpr bool check_is_same (...)				\
+  {									\
+    return true;							\
+  }									\
+									\
+  static constexpr bool is_same						\
+    = check_is_same<EF, RE, EF2, RE2> (nullptr);			\
+  static_assert (is_same, STATIC_SELF_CHECK_FAIL_MSG);			\
+									\
+  } /* namespace */
+
+
+/* Test construction / conversion from/to different types.  */
+
+/* RE/EF -> int (explicit) */
+CHECK_VALID (true,  int,  int (RE ()))
+CHECK_VALID (true,  int,  int (EF ()))
+
+/* other -> RE */
+
+/* You can construct a raw enum value from an int explicitly to punch
+   a hole in the type system if need to.  */
+CHECK_VALID (true,  RE,   RE (1))
+CHECK_VALID (true,  RE,   RE (RE2 ()))
+CHECK_VALID (false, void, RE (EF2 ()))
+CHECK_VALID (true,  RE,   RE (RE ()))
+CHECK_VALID (true,  RE,   RE (EF ()))
+
+/* other -> EF.  */
+
+/* As expected, enum-flags is a stronger type than the backing raw
+   enum.  Unlike with raw enums, you can't construct an enum flags
+   from an int nor from an unrelated enum type explicitly.  Add an
+   intermediate conversion via the raw enum if you really need it.  */
+CHECK_VALID (false, void, EF (1))
+CHECK_VALID (false, void, EF (RE2 ()))
+CHECK_VALID (false, void, EF (EF2 ()))
+CHECK_VALID (true,  EF,   EF (RE ()))
+CHECK_VALID (true,  EF,   EF (EF ()))
+
+/* Test operators.  */
+
+/* operator OP (raw_enum, int) */
+
+CHECK_VALID (false, void, RE () | 1)
+CHECK_VALID (false, void, RE () & 1)
+CHECK_VALID (false, void, RE () ^ 1)
+
+/* operator OP (int, raw_enum) */
+
+CHECK_VALID (false, void, 1 | RE ())
+CHECK_VALID (false, void, 1 & RE ())
+CHECK_VALID (false, void, 1 ^ RE ())
+
+/* operator OP (enum_flags, int) */
+
+CHECK_VALID (false, void, EF () | 1)
+CHECK_VALID (false, void, EF () & 1)
+CHECK_VALID (false, void, EF () ^ 1)
+
+/* operator OP (int, enum_flags) */
+
+CHECK_VALID (false, void, 1 | EF ())
+CHECK_VALID (false, void, 1 & EF ())
+CHECK_VALID (false, void, 1 ^ EF ())
+
+/* operator OP (raw_enum, raw_enum) */
+
+CHECK_VALID (false, void, RE () | RE2 ())
+CHECK_VALID (false, void, RE () & RE2 ())
+CHECK_VALID (false, void, RE () ^ RE2 ())
+CHECK_VALID (true,  RE,   RE () | RE ())
+CHECK_VALID (true,  RE,   RE () & RE ())
+CHECK_VALID (true,  RE,   RE () ^ RE ())
+
+/* operator OP (enum_flags, raw_enum) */
+
+CHECK_VALID (false, void, EF () | RE2 ())
+CHECK_VALID (false, void, EF () & RE2 ())
+CHECK_VALID (false, void, EF () ^ RE2 ())
+CHECK_VALID (true,  EF,   EF () | RE ())
+CHECK_VALID (true,  EF,   EF () & RE ())
+CHECK_VALID (true,  EF,   EF () ^ RE ())
+
+/* operator OP= (raw_enum, raw_enum), rvalue ref on the lhs. */
+
+CHECK_VALID (false, void, RE () |= RE2 ())
+CHECK_VALID (false, void, RE () &= RE2 ())
+CHECK_VALID (false, void, RE () ^= RE2 ())
+CHECK_VALID (true,  RE&,  RE () |= RE ())
+CHECK_VALID (true,  RE&,  RE () &= RE ())
+CHECK_VALID (true,  RE&,  RE () ^= RE ())
+
+/* operator OP= (raw_enum, raw_enum), lvalue ref on the lhs. */
+
+CHECK_VALID (false, void, re |= RE2 ())
+CHECK_VALID (false, void, re &= RE2 ())
+CHECK_VALID (false, void, re ^= RE2 ())
+CHECK_VALID (true,  RE&,  re |= RE ())
+CHECK_VALID (true,  RE&,  re &= RE ())
+CHECK_VALID (true,  RE&,  re ^= RE ())
+
+/* operator OP= (enum_flags, raw_enum), rvalue ref on the lhs.  */
+
+CHECK_VALID (false, void, EF () |= RE2 ())
+CHECK_VALID (false, void, EF () &= RE2 ())
+CHECK_VALID (false, void, EF () ^= RE2 ())
+CHECK_VALID (true,  EF&,  EF () |= RE ())
+CHECK_VALID (true,  EF&,  EF () &= RE ())
+CHECK_VALID (true,  EF&,  EF () ^= RE ())
+
+/* operator OP= (enum_flags, raw_enum), lvalue ref on the lhs.  */
+
+CHECK_VALID (false, void, ef |= RE2 ())
+CHECK_VALID (false, void, ef &= RE2 ())
+CHECK_VALID (false, void, ef ^= RE2 ())
+CHECK_VALID (true,  EF&,  ef |= EF ())
+CHECK_VALID (true,  EF&,  ef &= EF ())
+CHECK_VALID (true,  EF&,  ef ^= EF ())
+
+/* operator OP= (enum_flags, enum_flags), rvalue ref on the lhs.  */
+
+CHECK_VALID (false, void, EF () |= EF2 ())
+CHECK_VALID (false, void, EF () &= EF2 ())
+CHECK_VALID (false, void, EF () ^= EF2 ())
+CHECK_VALID (true,  EF&,  EF () |= EF ())
+CHECK_VALID (true,  EF&,  EF () &= EF ())
+CHECK_VALID (true,  EF&,  EF () ^= EF ())
+
+/* operator OP= (enum_flags, enum_flags), lvalue ref on the lhs.  */
+
+CHECK_VALID (false, void, ef |= EF2 ())
+CHECK_VALID (false, void, ef &= EF2 ())
+CHECK_VALID (false, void, ef ^= EF2 ())
+CHECK_VALID (true,  EF&,  ef |= EF ())
+CHECK_VALID (true,  EF&,  ef &= EF ())
+CHECK_VALID (true,  EF&,  ef ^= EF ())
+
+/* operator~ (raw_enum) */
+
+CHECK_VALID (true,  RE,   ~RE ())
+
+/* operator~ (enum_flags) */
+
+CHECK_VALID (true,  EF,   ~EF ())
+
+/* Check ternary operator.  This exercises implicit conversions.  */
+
+/* Invalid since each type can be converted to the other.  */
+/* GCC 4.8 incorrectly fails to compile this test with:
+     error: operands to ?: have different types ‘enum_flags<RE>’ and ‘RE’
+   Confirmed to compile/pass with gcc 4.9, 5.3 and clang 3.7.
+*/
+#if GCC_VERSION >= 4009
+CHECK_VALID (false, void, true ? EF () : RE ())
+CHECK_VALID (false, void, true ? RE () : EF ())
+#endif
+
+/* These are valid, but it's not a big deal since you won't be able to
+   assign the resulting int to an enum or an enum_flags without a
+   cast.  */
+CHECK_VALID (true,  int,  true ? EF () : EF2 ())
+CHECK_VALID (true,  int,  true ? EF2 () : EF ())
+CHECK_VALID (true,  int,  true ? EF () : RE2 ())
+CHECK_VALID (true,  int,  true ? RE2 () : EF ())
+
+/* Unfortunately this can't work due to the way C++ computes the
+   return type of the ternary conditional operator.  int isn't
+   implicitly convertible to the raw enum type, so the type of the
+   expression is int.  And then int is not implicitly convertible to
+   enum_flags.
+
+   GCC 4.8 fails to compile this test with:
+     error: operands to ?: have different types ‘enum_flags<RE>’ and ‘int’
+   Confirmed to work with gcc 4.9, 5.3 and clang 3.7.
+*/
+#if GCC_VERSION >= 4009
+CHECK_VALID (false, void, true ? EF () : 0)
+CHECK_VALID (false, void, true ? 0 : EF ())
+#endif
+
+/* Check that the ++/--/<</>>/<<=/>>= operators are deleted.  */
+
+CHECK_VALID (false, void, RE ()++)
+CHECK_VALID (false, void, ++RE ())
+CHECK_VALID (false, void, --RE ())
+CHECK_VALID (false, void, RE ()--)
+
+CHECK_VALID (false, void, RE () << 1)
+CHECK_VALID (false, void, RE () >> 1)
+CHECK_VALID (false, void, EF () << 1)
+CHECK_VALID (false, void, EF () >> 1)
+
+CHECK_VALID (false, void, RE () <<= 1)
+CHECK_VALID (false, void, RE () >>= 1)
+CHECK_VALID (false, void, EF () <<= 1)
+CHECK_VALID (false, void, EF () >>= 1)
+
+/* -------------------------------------------------------------------- */
+
+/* Follows misc tests that exercise the API.  Some are compile time,
+   when possible, others are run time.  */
+
+enum test_flag
+  {
+    FLAG1 = 1 << 1,
+    FLAG2 = 1 << 2,
+    FLAG3 = 1 << 3,
+  };
+
+using test_flags = enum_flags<test_flag>;
+
+}} /* namespace selftests::enum_flags */
+
+ENABLE_ENUM_FLAGS_OPERATORS (selftests::enum_flags_tests::test_flag);
+
+namespace selftests { namespace enum_flags_tests {
+
+static void
+self_test ()
+{
+  /* Check that default construction works.  */
+  {
+    test_flags f;
+
+    /* Check that assignment from zero works.  */
+    f = 0;
+
+    SELF_CHECK (f == 0);
+  }
+
+  /* Check that construction from zero works.  */
+  {
+    constexpr test_flags zero1 = 0;
+    constexpr test_flags zero2 (0);
+    constexpr test_flags zero3 {0};
+    constexpr test_flags zero4 = {0};
+
+    STATIC_SELF_CHECK (zero1 == 0);
+    STATIC_SELF_CHECK (zero2 == 0);
+    STATIC_SELF_CHECK (zero3 == 0);
+    STATIC_SELF_CHECK (zero4 == 0);
+  }
+
+  /* Check construction from enum value.  */
+  {
+    STATIC_SELF_CHECK (test_flags (FLAG1) == FLAG1);
+    STATIC_SELF_CHECK (test_flags (FLAG2) != FLAG1);
+  }
+
+  /* Check copy/assignment.  */
+  {
+    constexpr test_flags src = FLAG1;
+
+    constexpr test_flags f1 = src;
+    constexpr test_flags f2 (src);
+    constexpr test_flags f3 {src};
+    constexpr test_flags f4 = {src};
+
+    STATIC_SELF_CHECK (f1 == FLAG1);
+    STATIC_SELF_CHECK (f2 == FLAG1);
+    STATIC_SELF_CHECK (f3 == FLAG1);
+    STATIC_SELF_CHECK (f4 == FLAG1);
+  }
+
+  /* Check moving.  */
+  {
+    test_flags src = FLAG1;
+    test_flags dst = 0;
+
+    dst = std::move (src);
+    SELF_CHECK (dst == FLAG1);
+  }
+
+  /* Check construction from an 'or' of multiple bits.  For this to
+     work, operator| must be overridden to return an enum type.  The
+     builtin version would return int instead and then the conversion
+     to test_flags would fail.  */
+  {
+    constexpr test_flags f = FLAG1 | FLAG2;
+    STATIC_SELF_CHECK (f == (FLAG1 | FLAG2));
+  }
+
+  /* Similarly, check that "FLAG1 | FLAG2" on the rhs of an assignment
+     operator works.  */
+  {
+    test_flags f = 0;
+    f |= FLAG1 | FLAG2;
+    SELF_CHECK (f == (FLAG1 | FLAG2));
+
+    f &= FLAG1 | FLAG2;
+    SELF_CHECK (f == (FLAG1 | FLAG2));
+
+    f ^= FLAG1 | FLAG2;
+    SELF_CHECK (f == 0);
+  }
+
+  /* Check explicit conversion to int works.  */
+  {
+    constexpr int some_bits (FLAG1 | FLAG2);
+
+    /* And comparison with int works too.  */
+    STATIC_SELF_CHECK (some_bits == (FLAG1 | FLAG2));
+    STATIC_SELF_CHECK (some_bits == test_flags (FLAG1 | FLAG2));
+  }
+
+  /* Check operator| and operator|=.  Particularly interesting is
+     making sure that putting the enum value on the lhs side of the
+     expression works (FLAG | f).  */
+  {
+    test_flags f = FLAG1;
+    f |= FLAG2;
+    SELF_CHECK (f == (FLAG1 | FLAG2));
+  }
+  {
+    test_flags f = FLAG1;
+    f = f | FLAG2;
+    SELF_CHECK (f == (FLAG1 | FLAG2));
+  }
+  {
+    test_flags f = FLAG1;
+    f = FLAG2 | f;
+    SELF_CHECK (f == (FLAG1 | FLAG2));
+  }
+
+  /* Check the &/&= operators.  */
+  {
+    test_flags f = FLAG1 & FLAG2;
+    SELF_CHECK (f == 0);
+
+    f = FLAG1 | FLAG2;
+    f &= FLAG2;
+    SELF_CHECK (f == FLAG2);
+
+    f = FLAG1 | FLAG2;
+    f = f & FLAG2;
+    SELF_CHECK (f == FLAG2);
+
+    f = FLAG1 | FLAG2;
+    f = FLAG2 & f;
+    SELF_CHECK (f == FLAG2);
+  }
+
+  /* Check the ^/^= operators.  */
+  {
+    constexpr test_flags f = FLAG1 ^ FLAG2;
+    STATIC_SELF_CHECK (f == (FLAG1 ^ FLAG2));
+  }
+
+  {
+    test_flags f = FLAG1 ^ FLAG2;
+    f ^= FLAG3;
+    SELF_CHECK (f == (FLAG1 | FLAG2 | FLAG3));
+    f = f ^ FLAG3;
+    SELF_CHECK (f == (FLAG1 | FLAG2));
+    f = FLAG3 ^ f;
+    SELF_CHECK (f == (FLAG1 | FLAG2 | FLAG3));
+  }
+
+  /* Check operator~.  */
+  {
+    constexpr test_flags f1 = ~FLAG1;
+    constexpr test_flags f2 = ~f1;
+    STATIC_SELF_CHECK (f2 == FLAG1);
+  }
+
+  /* Check the ternary operator.  */
+  {
+    constexpr test_flags f1 = true ? FLAG1 : FLAG2;
+    STATIC_SELF_CHECK (f1 == FLAG1);
+    constexpr test_flags f2 = false ? FLAG1 : FLAG2;
+    STATIC_SELF_CHECK (f2 == FLAG2);
+  }
+
+  /* Check that we can use operator| in switch cases, where only
+     constants are allowed.  This should work because operator| is
+     constexpr.  */
+  {
+    test_flags f = FLAG1 | FLAG2;
+    bool ok = false;
+
+    switch (f)
+      {
+      case FLAG1:
+	break;
+      case FLAG2:
+	break;
+      case FLAG1 | FLAG2:
+	ok = true;
+	break;
+      }
+
+    SELF_CHECK (ok);
+  }
+}
+
+}} /* namespace selftests::enum_flags_tests */
+
+#endif
+
+void
+_initialize_enum_flags_selftests (void)
+{
+#if GDB_SELF_TEST
+  register_self_test (selftests::enum_flags_tests::self_test);
+#endif
+}
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 1b0fe5b..7136599 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -925,7 +925,7 @@ parse_string_or_char (const char *tokptr, const char **outptr,
     }
   ++tokptr;
 
-  value->type = C_STRING | (quote == '\'' ? C_CHAR : 0); /*FIXME*/
+  value->type = (int) C_STRING | (quote == '\'' ? C_CHAR : 0); /*FIXME*/
   value->ptr = (char *) obstack_base (&tempbuf);
   value->length = obstack_object_size (&tempbuf);
 
diff --git a/gdb/selftest.h b/gdb/selftest.h
index 2b028dd..07d1477 100644
--- a/gdb/selftest.h
+++ b/gdb/selftest.h
@@ -41,4 +41,15 @@ extern void run_self_tests (void);
       error (_("self-test failed at %s:%d"), __FILE__, __LINE__);	\
   } while (0)
 
+/* Check that VALUE is true, and, if not, fail at compile time.  */
+
+#define SC_STRINGIZE_1(STR) #STR
+#define SC_STRINGIZE(STR) SC_STRINGIZE_1 (STR)
+
+#define STATIC_SELF_CHECK_FAIL_MSG					\
+  "static self-test failed at " __FILE__ ":" SC_STRINGIZE (__LINE__)
+
+#define STATIC_SELF_CHECK(VALUE)			\
+  static_assert (VALUE, STATIC_SELF_CHECK_FAIL_MSG)
+
 #endif /* SELFTEST_H */
-- 
2.5.5

Re: [PATCH 2/3] enum_flags: Fix problems and add comprehensive unit tests

[Simon Marchi]

On 2016-11-03 23:22, Pedro Alves wrote:
> This patch starts by adding comprehensive unit tests for enum_flags.
> 
> It adds:
> 
>  - tests of normal expected uses of the API.
> 
>  - checks that _invalid_ uses of the API would fail to compile.  I.e.,
>    it validates that enum_flags really is a strong type, and that
>    incorrect mixing of enum types would be caught at compile time.  It
>    pulls that off making use of SFINEA and C++11's decltype/constexpr.
> 
> This revealed many holes in the enum_flags API.  For example, the f1
> assignment below currently incorrectly fails to compile:
> 
>  enum_flags<flags> f1 = FLAG1;
>  enum_flags<flags> f2 = FLAG2 | f1;
> 
> This hole and more are all plugged by this patch, by reworking how the
> enum_flags operators are implemented, and making use of C++11's
> feature of being able to delete methods/functions.
> 
> This makes most of the enum_flags operators constexpr.  Beyond
> enabling more compiler optimizations and enabling the new unit tests,
> this has other advantages, like making it possible to use operator|
> with enum_flags values in switch cases, where only compile-time
> constants are allowed:
> 
>     enum_flags<flags> f = FLAG1 | FLAG2;
>     switch (f)
>       {
>       case FLAG1 | FLAG2:
> 	break;
>       }
> 
> Currently that fails to compile.
> 
> This adds a STATIC_SELF_TEST macro to selftest.h, which is a variant
> of SELF_TEST, but uses C++11's static_assert to do checking at compile
> time.
> 
> To avoid potential ODR violations, since the tests add enums with
> short names that could easily conflict with other names, the new
> selftests are placed in a namespace (selftests::enum_flags_tests).  I
> think that's a good practice that we should start following.  This
> required splitting the global operator overload enablement out of the
> DEF_ENUM_FLAGS_TYPE macro into a separate macro, because
> DEF_ENUM_FLAGS_TYPE wants to create the enum flags typedef in the
> global namespace too.
> 
> Tested with gcc 4.8, 4.9, 5.3, 7 (trunk) and clang 3.7.

Yay, more unit tests!

I don't understand much about the internal implementation, but as long 
as it makes the class easy to use correctly and hard to use incorrectly, 
I think it's a win.

As a side-note, there's still the C implementation at the bottom of this 
file, it can probably go away.

Thanks,

Simon

Re: [PATCH 2/3] enum_flags: Fix problems and add comprehensive unit tests

[Simon Marchi]

On 2016-11-03 23:22, Pedro Alves wrote:
> +/* The (real) enum types used in CHECK_VALID.  Their names match the
> +   template parameter names of the templated defined by CHECK_VALID
> +   just matching real vs template param type more obvious.  They could
> +   be named differently.  */

I think something is not right with the second sentence.

> +/* Invalid since each type can be converted to the other.  */
> +/* GCC 4.8 incorrectly fails to compile this test with:
> +     error: operands to ?: have different types ‘enum_flags<RE>’ and 
> ‘RE’
> +   Confirmed to compile/pass with gcc 4.9, 5.3 and clang 3.7.
> +*/
> +#if GCC_VERSION >= 4009

Does this #if enables testing with clang?

> +CHECK_VALID (false, void, true ? EF () : RE ())
> +CHECK_VALID (false, void, true ? RE () : EF ())
> +#endif

Re: [PATCH 0/3] enum_flags: Fix problems and add comprehensive unit tests

[Luis Machado]

On 11/03/2016 10:22 PM, Pedro Alves wrote:
> Recently, while working on making symfile_add_flags and objfile->flags
> strongly typed [1], I noticed a few enum_flags issues, like this
> failing to compile:
>
>   symfile_add_flags add_flags = (SYMFILE_MAINLINE
>                                  | current_inferior ()->symfile_flags);
>
> while the form that landed in master does compile:
>
>   symfile_add_flags add_flags = (current_inferior ()->symfile_flags
>                                  | SYMFILE_MAINLINE);
>
> This series started out by wanting to fix that, but it ended up fixing
> a bunch more, and adding comprehensive enum_flags unit tests along the
> way.  Writing the tests in turn exposed more problems.  Rinse, repeat.
>
> The enum_flags methods and global operators are made constexpr where
> possible, and then C++11's deleted functions are used to remove
> overloads that should not compile.  The unit tests then build on
> SFINAE + decltype + constexpr to check that mixing enum flags types
> incorrectly would really fail to compile.
>
> This series makes use of C++11 extensively: decltype, constexpr,
> =delete/=default, typedef -> type alias / using, static_assert, and
> more.
>
> [1] https://sourceware.org/ml/gdb-patches/2016-10/msg00715.html
>
> Pedro Alves (3):
>   enum_flags: Use C++11 std::underlying_type
>   enum_flags: Fix problems and add comprehensive unit tests
>   enum_flags: Fix ternary operator and remove implicit convertion to raw
>     enum
>
>  gdb/Makefile.in               |   2 +-
>  gdb/btrace.c                  |   4 +-
>  gdb/common/enum-flags.h       | 251 ++++++++++++-------
>  gdb/compile/compile-c-types.c |   2 +-
>  gdb/enum-flags-selftests.c    | 557 ++++++++++++++++++++++++++++++++++++++++++
>  gdb/go-exp.y                  |   2 +-
>  gdb/record-btrace.c           |  10 +-
>  gdb/selftest.h                |  11 +
>  8 files changed, 738 insertions(+), 101 deletions(-)
>  create mode 100644 gdb/enum-flags-selftests.c
>

It is a bit more general, but i thought i'd throw this out there. Do we 
have a policy for new unit tests, how they should be created and if/when 
they should be a replacement for .exp tests?

Anything that gets us further away from dejagnu/expect-based testing is 
a win in my opinion. If we can get unit tests to cover part or most of 
that, even better.