Rewrite enum_flags, add unit tests, fix problems

This patch started by adding comprehensive unit tests for enum_flags.

For the testing part, 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;

The unit tests also 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.

Also, several operators were missing.

These holes and more are 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.

There are cases in gdb/compile/ where we need to call a function in a
C plugin 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.

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.

It also switches to a different mechanism of enabling the global
operators.  The current mechanism isn't namespace friendly, the new
one is.

It also switches to C++11-style SFINAE -- instead of wrapping the
return type in a SFINAE-friently structure, we use an unnamed template
parameter.  I.e., this:

  template <typename enum_type,
	    typename = is_enum_flags_enum_type_t<enum_type>>
  enum_type
  operator& (enum_type e1, enum_type e2)

instead of:

  template <typename enum_type>
  typename enum_flags_type<enum_type>::type
  operator& (enum_type e1, enum_type e2)

Note that the static_assert inside operator~() was converted to a
couple overloads (signed vs unsigned), because static_assert is too
late for SFINAE-based tests, which is important for the CHECK_VALID
unit tests.

Tested with gcc {4.8, 7.1, 9.3} and clang {5.0.2, 10.0.0}.

gdb/ChangeLog:

	* Makefile.in (SELFTESTS_SRCS): Add
	unittests/enum-flags-selftests.c.
	* btrace.c (ftrace_update_caller, ftrace_fixup_calle): Use
	btrace_function_flags instead of enum btrace_function_flag.
	* compile/compile-c-types.c (convert_qualified): Use
	enum_flags::raw.
	* compile/compile-cplus-symbols.c (convert_one_symbol)
	(convert_symbol_bmsym):
	* compile/compile-cplus-types.c (compile_cplus_convert_method)
	(compile_cplus_convert_struct_or_union_methods)
	(compile_cplus_instance::convert_qualified_base):
	* go-exp.y (parse_string_or_char): Add cast to int.
	* unittests/enum-flags-selftests.c: New file.
	* 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.

gdbsupport/ChangeLog:

	* enum-flags.h: Include "traits.h".
	(DEF_ENUM_FLAGS_TYPE): Declare a function instead of defining a
	structure.
	(enum_underlying_type): Update comment.
	(namespace enum_flags_detail): New.  Move struct zero_type here.
	(EnumIsUnsigned, EnumIsSigned): New.
	(class enum_flags): Make most methods constexpr.
	(operator&=, operator|=, operator^=): Take an enum_flags instead
	of an enum_type.  Make rvalue ref versions deleted.
	(operator enum_type()): Delete.
	(operator&, operator|, operator^, operator~): Delete, moved out of
	class.
	(raw()): New method.
	(is_enum_flags_enum_type_t): Declare.
	(ENUM_FLAGS_GEN_BINOP, ENUM_FLAGS_GEN_COMPOUND_ASSIGN)
	(ENUM_FLAGS_GEN_COMP): New.  Use them to reimplement global
	operators.
	(operator~): Now constexpr and reimplemented.
	(operator<<, operator>>): New deleted functions.
	* valid-expr.h (CHECK_VALID_EXPR_5, CHECK_VALID_EXPR_6): New.

12 files changed, 951 insertions, 98 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cb435c1..45c7125 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,26 @@
 2020-09-14  Pedro Alves  <pedro@palves.net>
 
+	* Makefile.in (SELFTESTS_SRCS): Add
+	unittests/enum-flags-selftests.c.
+	* btrace.c (ftrace_update_caller, ftrace_fixup_calle): Use
+	btrace_function_flags instead of enum btrace_function_flag.
+	* compile/compile-c-types.c (convert_qualified): Use
+	enum_flags::raw.
+	* compile/compile-cplus-symbols.c (convert_one_symbol)
+	(convert_symbol_bmsym):
+	* compile/compile-cplus-types.c (compile_cplus_convert_method)
+	(compile_cplus_convert_struct_or_union_methods)
+	(compile_cplus_instance::convert_qualified_base):
+	* go-exp.y (parse_string_or_char): Add cast to int.
+	* unittests/enum-flags-selftests.c: New file.
+	* 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.
+
+2020-09-14  Pedro Alves  <pedro@palves.net>
+
 	* c-typeprint.c (c_type_print_modifier): Adjust to rename.
 	* gdbtypes.c (address_space_name_to_int): Rename to ...
 	(address_space_name_to_type_instance_flags): ... this.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 4808357..dbede7a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -435,6 +435,7 @@ SELFTESTS_SRCS = \
 	unittests/command-def-selftests.c \
 	unittests/common-utils-selftests.c \
 	unittests/copy_bitwise-selftests.c \
+	unittests/enum-flags-selftests.c \
 	unittests/environ-selftests.c \
 	unittests/filtered_iterator-selftests.c \
 	unittests/format_pieces-selftests.c \
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 2a0c61d..9022aed 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -265,7 +265,7 @@ ftrace_new_function (struct btrace_thread_info *btinfo,
 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 != 0)
     ftrace_debug (bfun, "updating caller");
@@ -283,7 +283,7 @@ static void
 ftrace_fixup_caller (struct btrace_thread_info *btinfo,
 		     struct btrace_function *bfun,
 		     struct btrace_function *caller,
-		     enum btrace_function_flag flags)
+		     btrace_function_flags flags)
 {
   unsigned int prev, next;
 
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 6d9bb37..6c91359 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -254,7 +254,8 @@ convert_qualified (compile_c_instance *context, struct type *type)
   if (TYPE_RESTRICT (type))
     quals |= GCC_QUALIFIER_RESTRICT;
 
-  return context->plugin ().build_qualified_type (unqual_converted, quals);
+  return context->plugin ().build_qualified_type (unqual_converted,
+						  quals.raw ());
 }
 
 /* Convert a complex type to its gcc representation.  */
diff --git a/gdb/compile/compile-cplus-symbols.c b/gdb/compile/compile-cplus-symbols.c
index 1998584..6b905f3 100644
--- a/gdb/compile/compile-cplus-symbols.c
+++ b/gdb/compile/compile-cplus-symbols.c
@@ -208,7 +208,7 @@ convert_one_symbol (compile_cplus_instance *instance,
 
 	  /* Define the decl.  */
 	  instance->plugin ().build_decl
-	    ("variable", name.c_str (), kind, sym_type,
+	    ("variable", name.c_str (), kind.raw (), sym_type,
 	     symbol_name.get (), addr, filename, line);
 
 	  /* Pop scope for non-local symbols.  */
@@ -323,7 +323,7 @@ convert_symbol_bmsym (compile_cplus_instance *instance,
   sym_type = instance->convert_type (type);
   instance->plugin ().push_namespace ("");
   instance->plugin ().build_decl
-    ("minsym", msym->natural_name (), kind, sym_type, nullptr, addr,
+    ("minsym", msym->natural_name (), kind.raw (), sym_type, nullptr, addr,
      nullptr, 0);
   instance->plugin ().pop_binding_level ("");
 }
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index bb4b267..2d4352f 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -668,7 +668,7 @@ compile_cplus_convert_method (compile_cplus_instance *instance,
      type and corresponding qualifier flags.  */
   gcc_type func_type = compile_cplus_convert_func (instance, method_type, true);
   gcc_type class_type = instance->convert_type (parent_type);
-  gcc_cp_qualifiers_flags quals = (enum gcc_cp_qualifiers) 0;
+  gcc_cp_qualifiers_flags quals = 0;
 
   if (TYPE_CONST (method_type))
     quals |= GCC_CP_QUALIFIER_CONST;
@@ -681,7 +681,7 @@ compile_cplus_convert_method (compile_cplus_instance *instance,
   gcc_cp_ref_qualifiers_flags rquals = GCC_CP_REF_QUAL_NONE;
 
   return instance->plugin ().build_method_type
-    (class_type, func_type, quals, rquals);
+    (class_type, func_type, quals.raw (), rquals.raw ());
 }
 
 /* Convert a member or method pointer represented by TYPE.  */
@@ -745,7 +745,7 @@ compile_cplus_convert_struct_or_union_methods (compile_cplus_instance *instance,
 		     (sym_kind
 		      | get_method_access_flag (type, i, j)
 		      | GCC_CP_FLAG_VIRTUAL_FUNCTION
-		      | GCC_CP_FLAG_PURE_VIRTUAL_FUNCTION),
+		      | GCC_CP_FLAG_PURE_VIRTUAL_FUNCTION).raw (),
 		     method_type, nullptr, 0, nullptr, 0);
 		  continue;
 		}
@@ -787,7 +787,7 @@ compile_cplus_convert_struct_or_union_methods (compile_cplus_instance *instance,
 
 	  instance->plugin ().build_decl
 	    (kind, overloaded_name.get (),
-	     sym_kind | get_method_access_flag (type, i, j),
+	     (sym_kind | get_method_access_flag (type, i, j)).raw (),
 	     method_type, nullptr, address, filename, line);
 	}
     }
@@ -1060,7 +1060,7 @@ compile_cplus_instance::convert_qualified_base (gcc_type base,
   gcc_type result = base;
 
   if (quals != 0)
-    result = plugin ().build_qualified_type (base, quals);
+    result = plugin ().build_qualified_type (base, quals.raw ());
 
   return result;
 }
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 17c76ac..ee1db2b 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -924,7 +924,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/record-btrace.c b/gdb/record-btrace.c
index a1a3efc..fd0d13f 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1928,7 +1928,7 @@ record_btrace_target::get_tailcall_unwinder ()
 /* 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)
     {
@@ -2221,7 +2221,7 @@ record_btrace_target::commit_resume ()
 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)
@@ -2229,7 +2229,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).c_str (), flags,
+	 target_pid_to_str (tp->ptid).c_str (), flags.raw (),
 	 btrace_thread_flag_to_str (flags));
 
   tp->btrace.flags &= ~(BTHR_MOVE | BTHR_STOP);
@@ -2449,7 +2449,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;
 
@@ -2457,7 +2457,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).c_str (), flags,
+	 target_pid_to_str (tp->ptid).c_str (), flags.raw (),
 	 btrace_thread_flag_to_str (flags));
 
   /* We can't step without an execution history.  */
diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c
new file mode 100644
index 0000000..af585f0
--- /dev/null
+++ b/gdb/unittests/enum-flags-selftests.c
@@ -0,0 +1,586 @@
+/* Self tests for enum-flags for GDB, the GNU debugger.
+
+   Copyright (C) 2016-2020 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 "gdbsupport/enum-flags.h"
+#include "gdbsupport/valid-expr.h"
+#include "gdbsupport/selftest.h"
+
+namespace selftests {
+namespace enum_flags_tests {
+
+/* The (real) enum types used in CHECK_VALID.  Their names match the
+   template parameter names of the templates defined by CHECK_VALID to
+   make it simpler to use.  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,
+  };
+
+/* An unsigned "real enum".  */
+enum URE : unsigned
+  {
+    URE_FLAG1 = 1 << 1,
+    URE_FLAG2 = 1 << 2,
+    URE_FLAG3 = 0xffffffff,
+  };
+
+/* A non-flags enum.  */
+enum NF
+  {
+    NF_FLAG1 = 1 << 1,
+    NF_FLAG2 = 1 << 2,
+  };
+
+/* The corresponding "enum flags" types.  */
+DEF_ENUM_FLAGS_TYPE (RE, EF);
+DEF_ENUM_FLAGS_TYPE (RE2, EF2);
+DEF_ENUM_FLAGS_TYPE (URE, UEF);
+
+#if HAVE_IS_TRIVIALLY_COPYABLE
+
+/* So that std::vectors of types that have enum_flags fields can
+   reallocate efficiently memcpy.  */
+gdb_static_assert (std::is_trivially_copyable<EF>::value);
+
+#endif
+
+/* 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 CHECK_VALID(VALID, EXPR_TYPE, EXPR)		\
+  CHECK_VALID_EXPR_6 (EF, RE, EF2, RE2, UEF, URE, VALID, EXPR_TYPE, EXPR)
+
+typedef std::underlying_type<RE>::type und;
+
+/* Test construction / conversion from/to different types.  */
+
+/* RE/EF -> underlying (explicit) */
+CHECK_VALID (true,  und,  und (RE ()))
+CHECK_VALID (true,  und,  und (EF ()))
+
+/* 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 (false, void, 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 integer 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 (1u))
+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 (false, void, RE () |= RE ())
+CHECK_VALID (false, void, RE () &= RE ())
+CHECK_VALID (false, void, 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 (false, void, EF () |= RE ())
+CHECK_VALID (false, void, EF () &= RE ())
+CHECK_VALID (false, void, 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 (false, void, EF () |= EF ())
+CHECK_VALID (false, void, EF () &= EF ())
+CHECK_VALID (false, void, 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 (false,  void,   ~RE ())
+CHECK_VALID (true,   URE,    ~URE ())
+
+/* operator~ (enum_flags) */
+
+CHECK_VALID (false,  void,   ~EF ())
+CHECK_VALID (true,   UEF,    ~UEF ())
+
+/* Check ternary operator.  This exercises implicit conversions.  */
+
+CHECK_VALID (true,  EF,   true ? EF () : RE ())
+CHECK_VALID (true,  EF,   true ? RE () : EF ())
+
+/* These are valid, but it's not a big deal since you won't be able to
+   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 with gcc 4.8 and 4.9 at least.  Running the test
+   outside a SFINAE context shows:
+
+    invalid user-defined conversion from ‘EF’ to ‘RE2’
+
+   They've been confirmed to compile/pass with gcc 5.3, gcc 7.1 and
+   clang 3.7.  */
+
+CHECK_VALID (true,  int,  true ? EF () : EF2 ())
+CHECK_VALID (true,  int,  true ? EF2 () : EF ())
+#if GCC_VERSION >= 5003 || defined __clang__
+CHECK_VALID (true,  int,  true ? EF () : RE2 ())
+CHECK_VALID (true,  int,  true ? RE2 () : EF ())
+#endif
+
+/* Same, but with an unsigned enum.  */
+
+typedef unsigned int uns;
+
+CHECK_VALID (true,  uns,  true ? EF () : UEF ())
+CHECK_VALID (true,  uns,  true ? UEF () : EF ())
+#if GCC_VERSION >= 5003 || defined __clang__
+CHECK_VALID (true,  uns,  true ? EF () : URE ())
+CHECK_VALID (true,  uns,  true ? URE () : EF ())
+#endif
+
+/* 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 || defined __clang__
+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)
+
+/* Test comparison operators.  */
+
+CHECK_VALID (false, void, EF () == EF2 ())
+CHECK_VALID (false, void, EF () == RE2 ())
+CHECK_VALID (false, void, RE () == EF2 ())
+
+CHECK_VALID (true,  bool, EF (RE (1)) == EF (RE (1)))
+CHECK_VALID (true,  bool, EF (RE (1)) == RE (1))
+CHECK_VALID (true,  bool, RE (1)      == EF (RE (1)))
+
+CHECK_VALID (false, void, EF () != EF2 ())
+CHECK_VALID (false, void, EF () != RE2 ())
+CHECK_VALID (false, void, RE () != EF2 ())
+
+/* On clang, disable -Wenum-compare due to "error: comparison of two
+   values with different enumeration types [-Werror,-Wenum-compare]".
+   clang doesn't suppress -Wenum-compare in SFINAE contexts.  Not a
+   big deal since misuses like these in GDB will be caught by -Werror
+   anyway.  This check is here mainly for completeness.  */
+#if defined __clang__
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wenum-compare"
+#endif
+CHECK_VALID (true,  bool, RE () == RE2 ())
+CHECK_VALID (true,  bool, RE () != RE2 ())
+#if defined __clang__
+# pragma GCC diagnostic pop
+#endif
+
+CHECK_VALID (true,  bool, EF (RE (1)) != EF (RE (2)))
+CHECK_VALID (true,  bool, EF (RE (1)) != RE (2))
+CHECK_VALID (true,  bool, RE (1)      != EF (RE (2)))
+
+CHECK_VALID (true,  bool, EF () == 0)
+
+/* Check we didn't disable/delete comparison between non-flags enums
+   and unrelated types by mistake.  */
+CHECK_VALID (true,  bool, NF (1) == NF (1))
+CHECK_VALID (true,  bool, NF (1) == int (1))
+CHECK_VALID (true,  bool, NF (1) == char (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,
+  };
+
+enum test_uflag : unsigned
+  {
+    UFLAG1 = 1 << 1,
+    UFLAG2 = 1 << 2,
+    UFLAG3 = 1 << 3,
+  };
+
+DEF_ENUM_FLAGS_TYPE (test_flag, test_flags);
+DEF_ENUM_FLAGS_TYPE (test_uflag, test_uflags);
+
+static void
+self_test ()
+{
+  /* Check that default construction works.  */
+  {
+    constexpr test_flags f;
+
+    gdb_static_assert (f == 0);
+  }
+
+  /* Check that assignment from zero works.  */
+  {
+    test_flags f (FLAG1);
+
+    SELF_CHECK (f == FLAG1);
+
+    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};
+
+    gdb_static_assert (zero1 == 0);
+    gdb_static_assert (zero2 == 0);
+    gdb_static_assert (zero3 == 0);
+    gdb_static_assert (zero4 == 0);
+  }
+
+  /* Check construction from enum value.  */
+  {
+    gdb_static_assert (test_flags (FLAG1) == FLAG1);
+    gdb_static_assert (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};
+
+    gdb_static_assert (f1 == FLAG1);
+    gdb_static_assert (f2 == FLAG1);
+    gdb_static_assert (f3 == FLAG1);
+    gdb_static_assert (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;
+    gdb_static_assert (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.  */
+    gdb_static_assert (some_bits == (FLAG1 | FLAG2));
+    gdb_static_assert (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;
+    gdb_static_assert (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~.  Note this only compiles with unsigned
+     flags.  */
+  {
+    constexpr test_uflags f1 = ~UFLAG1;
+    constexpr test_uflags f2 = ~f1;
+    gdb_static_assert (f2 == UFLAG1);
+  }
+
+  /* Check the ternary operator.  */
+
+  {
+    /* raw enum, raw enum */
+    constexpr test_flags f1 = true ? FLAG1 : FLAG2;
+    gdb_static_assert (f1 == FLAG1);
+    constexpr test_flags f2 = false ? FLAG1 : FLAG2;
+    gdb_static_assert (f2 == FLAG2);
+  }
+
+  {
+    /* enum flags, raw enum */
+    constexpr test_flags src = FLAG1;
+    constexpr test_flags f1 = true ? src : FLAG2;
+    gdb_static_assert (f1 == FLAG1);
+    constexpr test_flags f2 = false ? src : FLAG2;
+    gdb_static_assert (f2 == FLAG2);
+  }
+
+  {
+    /* enum flags, enum flags */
+    constexpr test_flags src1 = FLAG1;
+    constexpr test_flags src2 = FLAG2;
+    constexpr test_flags f1 = true ? src1 : src2;
+    gdb_static_assert (f1 == src1);
+    constexpr test_flags f2 = false ? src1 : src2;
+    gdb_static_assert (f2 == src2);
+  }
+
+  /* Check that we can use flags in switch expressions (requires
+     unambiguous conversion to integer).  Also 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 enum_flags_tests */
+} /* namespace selftests */
+
+void _initialize_enum_flags_selftests ();
+
+void
+_initialize_enum_flags_selftests ()
+{
+  selftests::register_test ("enum-flags",
+			    selftests::enum_flags_tests::self_test);
+}
diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index 4858cc6..2c35ee7 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,5 +1,28 @@
 2020-09-14  Pedro Alves  <pedro@palves.net>
 
+	* enum-flags.h: Include "traits.h".
+	(DEF_ENUM_FLAGS_TYPE): Declare a function instead of defining a
+	structure.
+	(enum_underlying_type): Update comment.
+	(namespace enum_flags_detail): New.  Move struct zero_type here.
+	(EnumIsUnsigned, EnumIsSigned): New.
+	(class enum_flags): Make most methods constexpr.
+	(operator&=, operator|=, operator^=): Take an enum_flags instead
+	of an enum_type.  Make rvalue ref versions deleted.
+	(operator enum_type()): Delete.
+	(operator&, operator|, operator^, operator~): Delete, moved out of
+	class.
+	(raw()): New method.
+	(is_enum_flags_enum_type_t): Declare.
+	(ENUM_FLAGS_GEN_BINOP, ENUM_FLAGS_GEN_COMPOUND_ASSIGN)
+	(ENUM_FLAGS_GEN_COMP): New.  Use them to reimplement global
+	operators.
+	(operator~): Now constexpr and reimplemented.
+	(operator<<, operator>>): New deleted functions.
+	* valid-expr.h (CHECK_VALID_EXPR_5, CHECK_VALID_EXPR_6): New.
+
+2020-09-14  Pedro Alves  <pedro@palves.net>
+
 	* traits.h (struct nonesuch, struct detector, detected_or)
 	(detected_or_t, is_detected, detected_t, detected_or)
 	(detected_or_t, is_detected_exact, is_detected_convertible): New.
diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h
index 825ff4f..d30d90b 100644
--- a/gdbsupport/enum-flags.h
+++ b/gdbsupport/enum-flags.h
@@ -18,6 +18,8 @@
 #ifndef COMMON_ENUM_FLAGS_H
 #define COMMON_ENUM_FLAGS_H
 
+#include "traits.h"
+
 /* Type-safe wrapper for enum flags.  enum flags are enums where the
    values are bits that are meant to be ORed together.
 
@@ -51,23 +53,31 @@
 
 #ifdef __cplusplus
 
-/* Traits type used to prevent the global operator overloads from
-   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
+/* 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.  */
 #define DEF_ENUM_FLAGS_TYPE(enum_type, flags_type)	\
   typedef enum_flags<enum_type> flags_type;		\
-  template<>						\
-  struct enum_flags_type<enum_type>			\
-  {							\
-    typedef enum_flags<enum_type> type;			\
-  }
+  void is_enum_flags_enum_type (enum_type *)
+
+/* To enable the global enum_flags operators for enum, declare an
+   "is_enum_flags_enum_type" overload that has exactly one parameter,
+   of type a pointer to that enum class.  E.g.,:
+
+     void is_enum_flags_enum_type (enum some_flag *);
+
+   The function does not need to be defined, only declared.
+   DEF_ENUM_FLAGS_TYPE declares this.
+
+   A function declaration is preferred over a traits type, because the
+   former allows calling the DEF_ENUM_FLAGS_TYPE macro inside a
+   namespace to define the corresponding enum flags type in that
+   namespace.  The compiler finds the corresponding
+   is_enum_flags_enum_type function via ADL.  */
 
-/* Until we can rely on std::underlying type being universally
-   available (C++11), roll our own for enums.  */
+/* Note that std::underlying_type<enum_type> is not what we want here,
+   since that returns unsigned int even when the enum decays to signed
+   int.  */
 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; };
@@ -86,128 +96,324 @@ struct enum_underlying_type
     type;
 };
 
-template <typename E>
-class enum_flags
+namespace enum_flags_detail
 {
-public:
-  typedef E enum_type;
-  typedef typename enum_underlying_type<enum_type>::type underlying_type;
 
-private:
-  /* Private type used to support initializing flag types with zero:
+/* Private type used to support initializing flag types with zero:
 
-       foo_flags f = 0;
+   foo_flags f = 0;
 
-     but not other integers:
+   but not other integers:
 
-       foo_flags f = 1;
+   foo_flags f = 1;
 
-     The way this works is that we define an implicit constructor that
-     takes a pointer to this private type.  Since nothing can
-     instantiate an object of this type, the only possible pointer to
-     pass to the constructor is the NULL pointer, or, zero.  */
-  struct zero_type;
+   The way this works is that we define an implicit constructor that
+   takes a pointer to this private type.  Since nothing can
+   instantiate an object of this type, the only possible pointer to
+   pass to the constructor is the NULL pointer, or, zero.  */
+struct zero_type;
 
-  underlying_type
-  underlying_value () const
-  {
-    return m_enum_value;
-  }
+/* gdb::Requires trait helpers.  */
+template <typename enum_type>
+using EnumIsUnsigned
+  = std::is_unsigned<typename enum_underlying_type<enum_type>::type>;
+template <typename enum_type>
+using EnumIsSigned
+  = std::is_signed<typename enum_underlying_type<enum_type>::type>;
+
+}
+
+template <typename E>
+class enum_flags
+{
+public:
+  typedef E enum_type;
+  typedef typename enum_underlying_type<enum_type>::type underlying_type;
 
 public:
   /* Allow default construction.  */
-  enum_flags ()
+  constexpr enum_flags ()
     : m_enum_value ((enum_type) 0)
   {}
 
+  /* The default move/copy ctor/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_detail::zero_type *zero)
     : m_enum_value ((enum_type) 0)
   {}
 
-  enum_flags &operator&= (enum_type e)
+  enum_flags &operator&= (enum_flags e) &
   {
-    m_enum_value = (enum_type) (underlying_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) (underlying_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) (underlying_value () ^ e);
+    m_enum_value = (enum_type) (m_enum_value ^ e.m_enum_value);
     return *this;
   }
 
-  operator enum_type () const
+  /* Delete rval versions.  */
+  void operator&= (enum_flags e) && = delete;
+  void operator|= (enum_flags e) && = delete;
+  void operator^= (enum_flags e) && = delete;
+
+  /* Like raw enums, allow conversion to the underlying type.  */
+  constexpr operator underlying_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
+  /* Get the underlying value as a raw enum.  */
+  constexpr enum_type raw () const
   {
-    // We only the underlying type to be unsigned when actually using
-    // operator~ -- if it were not unsigned, undefined behavior could
-    // result.  However, asserting this in the class itself would
-    // require too many unnecessary changes to otherwise ok enum
-    // types.
-    gdb_static_assert (std::is_unsigned<underlying_type>::value);
-    return (enum_type) ~underlying_value ();
+    return m_enum_value;
   }
 
+  /* Binary operations involving some unrelated type (which would be a
+     bug) are implemented as non-members, and deleted.  */
+
 private:
   /* Stored as enum_type because GDB knows to print the bit flags
      neatly if the enum values look like bit flags.  */
   enum_type m_enum_value;
 };
 
+template <typename E>
+using is_enum_flags_enum_type_t
+  = decltype (is_enum_flags_enum_type (std::declval<E *> ()));
+
 /* Global operator overloads.  */
 
-template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator& (enum_type e1, enum_type e2)
+/* Generate binary operators.  */
+
+#define ENUM_FLAGS_GEN_BINOP(OPERATOR_OP, OP)				\
+									\
+  /* Raw enum on both LHS/RHS.  Returns raw enum type.  */		\
+  template <typename enum_type,						\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_type							\
+  OPERATOR_OP (enum_type e1, enum_type e2)				\
+  {									\
+    using underlying = typename enum_flags<enum_type>::underlying_type;	\
+    return (enum_type) (underlying (e1) OP underlying (e2));		\
+  }									\
+									\
+  /* enum_flags on the LHS.  */						\
+  template <typename enum_type,						\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_type>					\
+  OPERATOR_OP (enum_flags<enum_type> e1, enum_type e2)			\
+  { return e1.raw () OP e2; }						\
+									\
+  /* enum_flags on the RHS.  */						\
+  template <typename enum_type,						\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_type>					\
+  OPERATOR_OP (enum_type e1, enum_flags<enum_type> e2)			\
+  { return e1 OP e2.raw (); }						\
+									\
+  /* enum_flags on both LHS/RHS.  */					\
+  template <typename enum_type,						\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_type>					\
+  OPERATOR_OP (enum_flags<enum_type> e1, enum_flags<enum_type> e2)	\
+  { return e1.raw () OP e2.raw (); }					\
+									\
+  /* Delete cases involving unrelated types.  */			\
+									\
+  template <typename enum_type, typename unrelated_type,		\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_type>					\
+  OPERATOR_OP (enum_type e1, unrelated_type e2) = delete;		\
+									\
+  template <typename enum_type, typename unrelated_type,		\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_type>					\
+  OPERATOR_OP (unrelated_type e1, enum_type e2) = delete;		\
+									\
+  template <typename enum_type, typename unrelated_type,		\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_type>					\
+  OPERATOR_OP (enum_flags<enum_type> e1, unrelated_type e2) = delete;	\
+									\
+  template <typename enum_type, typename unrelated_type,		\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_flags<enum_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' enumerators of different flag types,
+   cast to integer first.
+*/
+#define ENUM_FLAGS_GEN_COMPOUND_ASSIGN(OPERATOR_OP, OP)			\
+  /* lval reference version.  */					\
+  template <typename enum_type,						\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_type &							\
+  OPERATOR_OP (enum_type &e1, enum_type e2)				\
+  { return e1 = e1 OP e2; }						\
+									\
+  /* rval reference version.  */					\
+  template <typename enum_type,						\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  void									\
+  OPERATOR_OP (enum_type &&e1, enum_type e2) = delete;			\
+									\
+  /* Delete compound assignment from unrelated types.  */		\
+									\
+  template <typename enum_type, typename other_enum_type,		\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  constexpr enum_type &							\
+  OPERATOR_OP (enum_type &e1, other_enum_type e2) = delete;		\
+									\
+  template <typename enum_type, typename other_enum_type,		\
+	    typename = is_enum_flags_enum_type_t<enum_type>>		\
+  void									\
+  OPERATOR_OP (enum_type &&e1, 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^=, ^)
+
+/* Allow comparison with enum_flags, raw enum, and integers, only.
+   The latter case allows "== 0".  As side effect, it allows comparing
+   with integer variables too, but that's not a common mistake to
+   make.  It's important to disable comparison with unrelated types to
+   prevent accidentally comparing with unrelated enum values, which
+   are convertible to integer, and thus coupled with enum_flags
+   convertion to underlying type too, would trigger the built-in 'bool
+   operator==(unsigned, int)' operator.  */
+
+#define ENUM_FLAGS_GEN_COMP(OPERATOR_OP, OP)				\
+									\
+  /* enum_flags OP enum_flags */					\
+									\
+  template <typename enum_type>						\
+  constexpr bool							\
+  OPERATOR_OP (enum_flags<enum_type> lhs, enum_flags<enum_type> rhs)	\
+  { return lhs.raw () OP rhs.raw (); }					\
+									\
+  /* enum_flags OP other */						\
+									\
+  template <typename enum_type>						\
+  constexpr bool							\
+  OPERATOR_OP (enum_flags<enum_type> lhs, enum_type rhs)		\
+  { return lhs.raw () OP rhs; }						\
+									\
+  template <typename enum_type>						\
+  constexpr bool							\
+  OPERATOR_OP (enum_flags<enum_type> lhs, int rhs)			\
+  { return lhs.raw () OP rhs; }						\
+									\
+  template <typename enum_type, typename U>				\
+  constexpr bool							\
+  OPERATOR_OP (enum_flags<enum_type> lhs, U rhs) = delete;		\
+									\
+  /* other OP enum_flags */						\
+									\
+  template <typename enum_type>						\
+  constexpr bool							\
+  OPERATOR_OP (enum_type lhs, enum_flags<enum_type> rhs)		\
+  { return lhs OP rhs.raw (); }						\
+									\
+  template <typename enum_type>						\
+  constexpr bool							\
+  OPERATOR_OP (int lhs, enum_flags<enum_type> rhs)			\
+  { return lhs OP rhs.raw (); }						\
+									\
+  template <typename enum_type, typename U>				\
+  constexpr bool							\
+  OPERATOR_OP (U lhs, enum_flags<enum_type> rhs) = delete;
+
+ENUM_FLAGS_GEN_COMP (operator==, ==)
+ENUM_FLAGS_GEN_COMP (operator!=, !=)
+
+/* Unary operators for the raw flags enum.  */
+
+/* We require underlying type to be unsigned when using operator~ --
+   if it were not unsigned, undefined behavior could result.  However,
+   asserting this in the class itself would require too many
+   unnecessary changes to usages of otherwise OK enum types.  */
+template <typename enum_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>,
+	  typename
+	    = gdb::Requires<enum_flags_detail::EnumIsUnsigned<enum_type>>>
+constexpr enum_type
+operator~ (enum_type e)
 {
-  return enum_flags<enum_type> (e1) & e2;
+  using underlying = typename enum_flags<enum_type>::underlying_type;
+  return (enum_type) ~underlying (e);
 }
 
-template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator| (enum_type e1, enum_type e2)
+template <typename enum_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>,
+	  typename = gdb::Requires<enum_flags_detail::EnumIsSigned<enum_type>>>
+constexpr void operator~ (enum_type e) = delete;
+
+template <typename enum_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>,
+	  typename
+	    = gdb::Requires<enum_flags_detail::EnumIsUnsigned<enum_type>>>
+constexpr enum_flags<enum_type>
+operator~ (enum_flags<enum_type> e)
 {
-  return enum_flags<enum_type> (e1) | e2;
+  using underlying = typename enum_flags<enum_type>::underlying_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;
-}
+template <typename enum_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>,
+	  typename = gdb::Requires<enum_flags_detail::EnumIsSigned<enum_type>>>
+constexpr void operator~ (enum_flags<enum_type> e) = delete;
 
-template <typename enum_type>
-typename enum_flags_type<enum_type>::type
-operator~ (enum_type e)
-{
-  return ~enum_flags<enum_type> (e);
-}
+/* Delete operator<< and operator>>.  */
+
+template <typename enum_type, typename any_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>>
+void operator<< (const enum_type &, const any_type &) = delete;
+
+template <typename enum_type, typename any_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>>
+void operator<< (const enum_flags<enum_type> &, const any_type &) = delete;
+
+template <typename enum_type, typename any_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>>
+void operator>> (const enum_type &, const any_type &) = delete;
+
+template <typename enum_type, typename any_type,
+	  typename = is_enum_flags_enum_type_t<enum_type>>
+void operator>> (const enum_flags<enum_type> &, const any_type &) = delete;
 
 #else /* __cplusplus */
 
diff --git a/gdbsupport/valid-expr.h b/gdbsupport/valid-expr.h
index a22fa61..459de17 100644
--- a/gdbsupport/valid-expr.h
+++ b/gdbsupport/valid-expr.h
@@ -91,4 +91,19 @@
 			ESC_PARENS (T1, T2, T3, T4),			\
 			VALID, EXPR_TYPE, EXPR)
 
+#define CHECK_VALID_EXPR_5(T1, T2, T3, T4, T5, VALID, EXPR_TYPE, EXPR)	\
+  CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2,		\
+				    typename T3, typename T4,		\
+				    typename T5),			\
+			ESC_PARENS (T1, T2, T3, T4, T5),		\
+			VALID, EXPR_TYPE, EXPR)
+
+#define CHECK_VALID_EXPR_6(T1, T2, T3, T4, T5, T6,			\
+			   VALID, EXPR_TYPE, EXPR)			\
+  CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2,		\
+				    typename T3, typename T4,		\
+				    typename T5, typename T6),		\
+			ESC_PARENS (T1, T2, T3, T4, T5, T6),		\
+			VALID, EXPR_TYPE, EXPR)
+
 #endif /* COMMON_VALID_EXPR_H */