Re: [PATCH 2/3] Use type_instance_flags more throughout

[Pedro Alves <pedro@palves.net>]

On 8/26/20 4:21 PM, Andrew Burgess wrote:

>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
>> index 55a6dafb7e..b42cef6137 100644
>> --- a/gdb/gdbtypes.h
>> +++ b/gdb/gdbtypes.h
>> @@ -1585,7 +1585,10 @@ extern void allocate_gnat_aux_type (struct type *);
>>       TYPE_ZALLOC (type,							       \
>>  		  sizeof (*TYPE_MAIN_TYPE (type)->type_specific.func_stuff)))
>>  
>> -#define TYPE_INSTANCE_FLAGS(thistype) (thistype)->instance_flags
>> +#define TYPE_INSTANCE_FLAGS(thistype) \
>> +  type_instance_flags ((enum type_instance_flag_value) (thistype)->instance_flags)
>> +#define SET_TYPE_INSTANCE_FLAGS(thistype, flags) \
>> +  (thistype)->instance_flags = flags
> 
> There are some places where you've not updated a use of
> TYPE_INSTANCE_FLAGS.  The patch below fixes these.  In order to find
> these I changed TYPE_INSTANCE_FLAGS to make use of a member function
> returning a 'const type_instance_flags'.
> 
> Feel free to take any parts of this patch you think are useful.

Wow, these shouldn't even compile.  That is showing a hole in enum_flags.

>  
> -  TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_byte)
> -    |= TYPE_INSTANCE_FLAG_NOTTEXT;

TYPE_INSTANCE_FLAGS does a cast, so this is the equivalent of:

  int flags = 0;
  (int) flags |= 1;

which of course doesn't compile:

 test.c:1:18: error: lvalue required as left operand of assignment
   1 |   (int) flags |= 1;
     |                  ^

I'm merging the patch below into the main enum flags patch, to
disable the rvalue versions of compound assignment.  It catches the
spots you caught as well:

 src/gdb/d-lang.c: In function ‘void* build_d_types(gdbarch*)’:
 src/gdb/d-lang.c:325:8: error: use of deleted function ‘void enum_flags<E>::operator|=(enum_flags<E>) && [with E = type_instance_flag_value]’
   325 |     |= TYPE_INSTANCE_FLAG_NOTTEXT;
       |        ^~~~~~~~~~~~~~~~~~~~~~~~~~

 src/gdb/ft32-tdep.c: In function ‘gdbarch* ft32_gdbarch_init(gdbarch_info, gdbarch_list*)’:
 src/gdb/ft32-tdep.c:580:42: error: use of deleted function ‘void enum_flags<E>::operator|=(enum_flags<E>) && [with E = type_instance_flag_value]’
   580 |   TYPE_INSTANCE_FLAGS (tdep->pc_type) |= TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
       |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'll merge your changes into the type_instance_flags patch.

From aadb29540a43d96dc234d0d8126e5081926eec02 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Mon, 14 Sep 2020 14:48:34 +0100
Subject: [PATCH] delete rval

---
 gdb/unittests/enum-flags-selftests.c | 18 +++++++++---------
 gdbsupport/enum-flags.h              | 18 +++++++++++-------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c
index 17ab5c9b09..af585f04ae 100644
--- a/gdb/unittests/enum-flags-selftests.c
+++ b/gdb/unittests/enum-flags-selftests.c
@@ -174,9 +174,9 @@ CHECK_VALID (true,  EF,   EF () ^ RE ())
 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 ())
+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. */
 
@@ -192,9 +192,9 @@ CHECK_VALID (true,  RE&,  re ^= RE ())
 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 ())
+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.  */
 
@@ -210,9 +210,9 @@ CHECK_VALID (true,  EF&,  ef ^= EF ())
 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 ())
+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.  */
 
diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h
index b3e317ecb9..d30d90b68d 100644
--- a/gdbsupport/enum-flags.h
+++ b/gdbsupport/enum-flags.h
@@ -147,22 +147,27 @@ class enum_flags
     : m_enum_value ((enum_type) 0)
   {}
 
-  enum_flags &operator&= (enum_flags e)
+  enum_flags &operator&= (enum_flags e) &
   {
     m_enum_value = (enum_type) (m_enum_value & e.m_enum_value);
     return *this;
   }
-  enum_flags &operator|= (enum_flags e)
+  enum_flags &operator|= (enum_flags e) &
   {
     m_enum_value = (enum_type) (m_enum_value | e.m_enum_value);
     return *this;
   }
-  enum_flags &operator^= (enum_flags e)
+  enum_flags &operator^= (enum_flags e) &
   {
     m_enum_value = (enum_type) (m_enum_value ^ e.m_enum_value);
     return *this;
   }
 
+  /* 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
   {
@@ -278,9 +283,8 @@ using is_enum_flags_enum_type_t
   /* rval 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; }						\
+  void									\
+  OPERATOR_OP (enum_type &&e1, enum_type e2) = delete;			\
 									\
   /* Delete compound assignment from unrelated types.  */		\
 									\
@@ -291,7 +295,7 @@ using is_enum_flags_enum_type_t
 									\
   template <typename enum_type, typename other_enum_type,		\
 	    typename = is_enum_flags_enum_type_t<enum_type>>		\
-  constexpr enum_type &							\
+  void									\
   OPERATOR_OP (enum_type &&e1, other_enum_type e2) = delete;
 
 ENUM_FLAGS_GEN_BINOP (operator|, |)

-- 
2.14.5