[2/3] Use type_instance_flags more throughout

Message ID 20200821144523.19451-3-pedro@palves.net
State New
Headers show
Series
  • Rewrite enum_flags, add unit tests, fix problems
Related show

Commit Message

Pedro Alves Aug. 21, 2020, 2:45 p.m.
The next patch in this series will rewrites enum_flags fixing some API
holes.  That would cause build failures around code using
type_instance_flags.  Or rather, that should be using it, but wasn't.

This patch fixes it by using type_instance_flags throughout instead of
plain integers.

Note that we can't make the seemingly obvious change to struct
type::instance_flags:

 -  unsigned instance_flags : 9;
 +  ENUM_BITFIELD (type_instance_flag_value) instance_flags : 9;

Because G++ complains then that 9 bits isn't sufficient for holding
all values of type_instance_flag_value.

So the patch adds a cast to TYPE_INSTANCE_FLAGS, and adds a separate
SET_TYPE_INSTANCE_FLAGS macro.

gdb/ChangeLog:

	* dwarf2/read.c (read_tag_pointer_type): Use type_instance_flags.
	* eval.c (fake_method::fake_method): Use SET_TYPE_INSTANCE_FLAGS.
	* gdbarch.h, gdbarch.c: Regenerate.
	* gdbarch.sh (address_class_type_flags): Use type_instance_flags.
	(address_class_name_to_type_flags): Use type_instance_flags and
	bool.
	* gdbtypes.c (address_space_name_to_int)
	(address_space_int_to_name, make_qualified_type): Use
	type_instance_flags.
	(make_qualified_type): Use type_instance_flags and
	SET_TYPE_INSTANCE_FLAGS.
	(make_type_with_address_space, make_cv_type, make_vector_type)
	(check_typedef): Use type_instance_flags.
	(recursive_dump_type): Cast type_instance_flags to unsigned for
	printing.
	(copy_type_recursive): Use SET_TYPE_INSTANCE_FLAGS.
	* gdbtypes.h (TYPE_INSTANCE_FLAGS): Return a type_instance_flags.
	(SET_TYPE_INSTANCE_FLAGS): New.
	(address_space_name_to_int, address_space_int_to_name)
	(make_type_with_address_space): Pass flags using
	type_instance_flags instead of int.
	* stabsread.c (cleanup_undefined_types_noname): Use
	SET_TYPE_INSTANCE_FLAGS.
	* type-stack.c (type_stack::follow_types): Use type_instance_flags.
---
 gdb/dwarf2/read.c |  7 +++----
 gdb/eval.c        |  2 +-
 gdb/gdbarch.c     |  6 +++---
 gdb/gdbarch.h     | 12 ++++++------
 gdb/gdbarch.sh    |  8 ++++----
 gdb/gdbtypes.c    | 58 ++++++++++++++++++++++++++++++-------------------------
 gdb/gdbtypes.h    | 15 +++++++++-----
 gdb/stabsread.c   |  2 +-
 gdb/type-stack.c  |  4 ++--
 9 files changed, 62 insertions(+), 52 deletions(-)

-- 
2.14.5

Comments

Lancelot SIX via Gdb-patches Aug. 25, 2020, 11:36 a.m. | #1
Hi,

On 8/21/20 11:45 AM, Pedro Alves wrote:
> The next patch in this series will rewrites enum_flags fixing some API

> holes.  That would cause build failures around code using

> type_instance_flags.  Or rather, that should be using it, but wasn't.

> 

> This patch fixes it by using type_instance_flags throughout instead of

> plain integers.

> 

> Note that we can't make the seemingly obvious change to struct

> type::instance_flags:

> 

>   -  unsigned instance_flags : 9;

>   +  ENUM_BITFIELD (type_instance_flag_value) instance_flags : 9;

> 

> Because G++ complains then that 9 bits isn't sufficient for holding

> all values of type_instance_flag_value.


I ran into the problem above the last time I used instance flags 
(address classes), but mostly because of the (rather silly) 9 bit (or 
whatever current number is there) limitation.

Isn't this more an artifact of trying to save space where there isn't a 
need to anymore? Wouldn't a 64-bit integer suffice here? That would give 
us 64 flags worth of data and virtually no problems until we ran out of 
bits.

Honestly, I don't particularly like the hackery that is going on in 
traits.h, just to catch something that may be easier fixed with a longer 
integer type.

I understand the C++ standard is evolving, but GDB's code base is still 
full of old code. We don't necessarily need to convert GDB's code base 
to use the latest and greatest out there, right? We just need to make it 
good quality and well maintained?

Just an opinion from someone that has used these flags before and has 
been bitten by their storage-space-saving ugliness.

> 

> So the patch adds a cast to TYPE_INSTANCE_FLAGS, and adds a separate

> SET_TYPE_INSTANCE_FLAGS macro.

> 

> gdb/ChangeLog:

> 

> 	* dwarf2/read.c (read_tag_pointer_type): Use type_instance_flags.

> 	* eval.c (fake_method::fake_method): Use SET_TYPE_INSTANCE_FLAGS.

> 	* gdbarch.h, gdbarch.c: Regenerate.

> 	* gdbarch.sh (address_class_type_flags): Use type_instance_flags.

> 	(address_class_name_to_type_flags): Use type_instance_flags and

> 	bool.

> 	* gdbtypes.c (address_space_name_to_int)

> 	(address_space_int_to_name, make_qualified_type): Use

> 	type_instance_flags.

> 	(make_qualified_type): Use type_instance_flags and

> 	SET_TYPE_INSTANCE_FLAGS.

> 	(make_type_with_address_space, make_cv_type, make_vector_type)

> 	(check_typedef): Use type_instance_flags.

> 	(recursive_dump_type): Cast type_instance_flags to unsigned for

> 	printing.

> 	(copy_type_recursive): Use SET_TYPE_INSTANCE_FLAGS.

> 	* gdbtypes.h (TYPE_INSTANCE_FLAGS): Return a type_instance_flags.

> 	(SET_TYPE_INSTANCE_FLAGS): New.

> 	(address_space_name_to_int, address_space_int_to_name)

> 	(make_type_with_address_space): Pass flags using

> 	type_instance_flags instead of int.

> 	* stabsread.c (cleanup_undefined_types_noname): Use

> 	SET_TYPE_INSTANCE_FLAGS.

> 	* type-stack.c (type_stack::follow_types): Use type_instance_flags.

> ---

>   gdb/dwarf2/read.c |  7 +++----

>   gdb/eval.c        |  2 +-

>   gdb/gdbarch.c     |  6 +++---

>   gdb/gdbarch.h     | 12 ++++++------

>   gdb/gdbarch.sh    |  8 ++++----

>   gdb/gdbtypes.c    | 58 ++++++++++++++++++++++++++++++-------------------------

>   gdb/gdbtypes.h    | 15 +++++++++-----

>   gdb/stabsread.c   |  2 +-

>   gdb/type-stack.c  |  4 ++--

>   9 files changed, 62 insertions(+), 52 deletions(-)

> 

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c

> index 0ac8533263..4ced5ac02b 100644

> --- a/gdb/dwarf2/read.c

> +++ b/gdb/dwarf2/read.c

> @@ -17292,10 +17292,9 @@ read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)

>       {

>         if (gdbarch_address_class_type_flags_p (gdbarch))

>   	{

> -	  int type_flags;

> -

> -	  type_flags = gdbarch_address_class_type_flags

> -			 (gdbarch, byte_size, addr_class);

> +	  type_instance_flags type_flags

> +	    = gdbarch_address_class_type_flags (gdbarch, byte_size,

> +						addr_class);

>   	  gdb_assert ((type_flags & ~TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)

>   		      == 0);

>   	  type = make_type_with_address_space (type, type_flags);

> diff --git a/gdb/eval.c b/gdb/eval.c

> index c62c35f318..cd300ddfef 100644

> --- a/gdb/eval.c

> +++ b/gdb/eval.c

> @@ -659,7 +659,7 @@ fake_method::fake_method (type_instance_flags flags,

>     TYPE_LENGTH (type) = 1;

>     type->set_code (TYPE_CODE_METHOD);

>     TYPE_CHAIN (type) = type;

> -  TYPE_INSTANCE_FLAGS (type) = flags;

> +  SET_TYPE_INSTANCE_FLAGS (type, flags);

>     if (num_types > 0)

>       {

>         if (param_types[num_types - 1] == NULL)

> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c

> index f8fe03ca68..2be959ecfc 100644

> --- a/gdb/gdbarch.c

> +++ b/gdb/gdbarch.c

> @@ -3501,7 +3501,7 @@ gdbarch_address_class_type_flags_p (struct gdbarch *gdbarch)

>     return gdbarch->address_class_type_flags != NULL;

>   }

>   

> -int

> +type_instance_flags

>   gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class)

>   {

>     gdb_assert (gdbarch != NULL);

> @@ -3566,8 +3566,8 @@ gdbarch_address_class_name_to_type_flags_p (struct gdbarch *gdbarch)

>     return gdbarch->address_class_name_to_type_flags != NULL;

>   }

>   

> -int

> -gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr)

> +bool

> +gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr)

>   {

>     gdb_assert (gdbarch != NULL);

>     gdb_assert (gdbarch->address_class_name_to_type_flags != NULL);

> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h

> index 7a3060e628..8a4a384fda 100644

> --- a/gdb/gdbarch.h

> +++ b/gdb/gdbarch.h

> @@ -848,8 +848,8 @@ extern void set_gdbarch_have_nonsteppable_watchpoint (struct gdbarch *gdbarch, i

>   

>   extern int gdbarch_address_class_type_flags_p (struct gdbarch *gdbarch);

>   

> -typedef int (gdbarch_address_class_type_flags_ftype) (int byte_size, int dwarf2_addr_class);

> -extern int gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class);

> +typedef type_instance_flags (gdbarch_address_class_type_flags_ftype) (int byte_size, int dwarf2_addr_class);

> +extern type_instance_flags gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class);

>   extern void set_gdbarch_address_class_type_flags (struct gdbarch *gdbarch, gdbarch_address_class_type_flags_ftype *address_class_type_flags);

>   

>   extern int gdbarch_address_class_type_flags_to_name_p (struct gdbarch *gdbarch);

> @@ -866,13 +866,13 @@ extern bool gdbarch_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdb_by

>   extern void set_gdbarch_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdbarch_execute_dwarf_cfa_vendor_op_ftype *execute_dwarf_cfa_vendor_op);

>   

>   /* Return the appropriate type_flags for the supplied address class.

> -   This function should return 1 if the address class was recognized and

> -   type_flags was set, zero otherwise. */

> +   This function should return true if the address class was recognized and

> +   type_flags was set, false otherwise. */

>   

>   extern int gdbarch_address_class_name_to_type_flags_p (struct gdbarch *gdbarch);

>   

> -typedef int (gdbarch_address_class_name_to_type_flags_ftype) (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr);

> -extern int gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr);

> +typedef bool (gdbarch_address_class_name_to_type_flags_ftype) (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr);

> +extern bool gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr);

>   extern void set_gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, gdbarch_address_class_name_to_type_flags_ftype *address_class_name_to_type_flags);

>   

>   /* Is a register in a group */

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh

> index 6d3c5c889d..7e9204119b 100755

> --- a/gdb/gdbarch.sh

> +++ b/gdb/gdbarch.sh

> @@ -689,16 +689,16 @@ v;int;cannot_step_breakpoint;;;0;0;;0

>   # See comment in target.h about continuable, steppable and

>   # non-steppable watchpoints.

>   v;int;have_nonsteppable_watchpoint;;;0;0;;0

> -F;int;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class

> +F;type_instance_flags;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class

>   M;const char *;address_class_type_flags_to_name;int type_flags;type_flags

>   # Execute vendor-specific DWARF Call Frame Instruction.  OP is the instruction.

>   # FS are passed from the generic execute_cfa_program function.

>   m;bool;execute_dwarf_cfa_vendor_op;gdb_byte op, struct dwarf2_frame_state *fs;op, fs;;default_execute_dwarf_cfa_vendor_op;;0

>   

>   # Return the appropriate type_flags for the supplied address class.

> -# This function should return 1 if the address class was recognized and

> -# type_flags was set, zero otherwise.

> -M;int;address_class_name_to_type_flags;const char *name, int *type_flags_ptr;name, type_flags_ptr

> +# This function should return true if the address class was recognized and

> +# type_flags was set, false otherwise.

> +M;bool;address_class_name_to_type_flags;const char *name, type_instance_flags *type_flags_ptr;name, type_flags_ptr

>   # Is a register in a group

>   m;int;register_reggroup_p;int regnum, struct reggroup *reggroup;regnum, reggroup;;default_register_reggroup_p;;0

>   # Fetch the pointer to the ith function argument.

> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c

> index c1eb03d898..64e44bfe23 100644

> --- a/gdb/gdbtypes.c

> +++ b/gdb/gdbtypes.c

> @@ -574,11 +574,11 @@ lookup_function_type_with_arguments (struct type *type,

>   /* Identify address space identifier by name --

>      return the integer flag defined in gdbtypes.h.  */

>   

> -int

> +type_instance_flags

>   address_space_name_to_int (struct gdbarch *gdbarch,

>   			   const char *space_identifier)

>   {

> -  int type_flags;

> +  type_instance_flags type_flags;

>   

>     /* Check for known address space delimiters.  */

>     if (!strcmp (space_identifier, "code"))

> @@ -598,7 +598,8 @@ address_space_name_to_int (struct gdbarch *gdbarch,

>      gdbtypes.h -- return the string version of the adress space name.  */

>   

>   const char *

> -address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)

> +address_space_int_to_name (struct gdbarch *gdbarch,

> +			   type_instance_flags space_flag)

>   {

>     if (space_flag & TYPE_INSTANCE_FLAG_CODE_SPACE)

>       return "code";

> @@ -617,7 +618,7 @@ address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)

>      STORAGE must be in the same obstack as TYPE.  */

>   

>   static struct type *

> -make_qualified_type (struct type *type, int new_flags,

> +make_qualified_type (struct type *type, type_instance_flags new_flags,

>   		     struct type *storage)

>   {

>     struct type *ntype;

> @@ -657,7 +658,7 @@ make_qualified_type (struct type *type, int new_flags,

>     TYPE_CHAIN (type) = ntype;

>   

>     /* Now set the instance flags and return the new type.  */

> -  TYPE_INSTANCE_FLAGS (ntype) = new_flags;

> +  SET_TYPE_INSTANCE_FLAGS (ntype, new_flags);

>   

>     /* Set length of new type to that of the original type.  */

>     TYPE_LENGTH (ntype) = TYPE_LENGTH (type);

> @@ -675,13 +676,14 @@ make_qualified_type (struct type *type, int new_flags,

>      representations.  */

>   

>   struct type *

> -make_type_with_address_space (struct type *type, int space_flag)

> +make_type_with_address_space (struct type *type,

> +			      type_instance_flags space_flag)

>   {

> -  int new_flags = ((TYPE_INSTANCE_FLAGS (type)

> -		    & ~(TYPE_INSTANCE_FLAG_CODE_SPACE

> -			| TYPE_INSTANCE_FLAG_DATA_SPACE

> -		        | TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL))

> -		   | space_flag);

> +  type_instance_flags new_flags = ((TYPE_INSTANCE_FLAGS (type)

> +				    & ~(TYPE_INSTANCE_FLAG_CODE_SPACE

> +					| TYPE_INSTANCE_FLAG_DATA_SPACE

> +					| TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL))

> +				   | space_flag);

>   

>     return make_qualified_type (type, new_flags, NULL);

>   }

> @@ -705,9 +707,9 @@ make_cv_type (int cnst, int voltl,

>   {

>     struct type *ntype;	/* New type */

>   

> -  int new_flags = (TYPE_INSTANCE_FLAGS (type)

> -		   & ~(TYPE_INSTANCE_FLAG_CONST

> -		       | TYPE_INSTANCE_FLAG_VOLATILE));

> +  type_instance_flags new_flags = (TYPE_INSTANCE_FLAGS (type)

> +				   & ~(TYPE_INSTANCE_FLAG_CONST

> +				       | TYPE_INSTANCE_FLAG_VOLATILE));

>   

>     if (cnst)

>       new_flags |= TYPE_INSTANCE_FLAG_CONST;

> @@ -1410,7 +1412,6 @@ void

>   make_vector_type (struct type *array_type)

>   {

>     struct type *inner_array, *elt_type;

> -  int flags;

>   

>     /* Find the innermost array type, in case the array is

>        multi-dimensional.  */

> @@ -1421,7 +1422,8 @@ make_vector_type (struct type *array_type)

>     elt_type = TYPE_TARGET_TYPE (inner_array);

>     if (elt_type->code () == TYPE_CODE_INT)

>       {

> -      flags = TYPE_INSTANCE_FLAGS (elt_type) | TYPE_INSTANCE_FLAG_NOTTEXT;

> +      type_instance_flags flags

> +	= TYPE_INSTANCE_FLAGS (elt_type) | TYPE_INSTANCE_FLAG_NOTTEXT;

>         elt_type = make_qualified_type (elt_type, flags, NULL);

>         TYPE_TARGET_TYPE (inner_array) = elt_type;

>       }

> @@ -2732,12 +2734,13 @@ struct type *

>   check_typedef (struct type *type)

>   {

>     struct type *orig_type = type;

> -  /* While we're removing typedefs, we don't want to lose qualifiers.

> -     E.g., const/volatile.  */

> -  int instance_flags = TYPE_INSTANCE_FLAGS (type);

>   

>     gdb_assert (type);

>   

> +  /* While we're removing typedefs, we don't want to lose qualifiers.

> +     E.g., const/volatile.  */

> +  type_instance_flags instance_flags = TYPE_INSTANCE_FLAGS (type);

> +

>     while (type->code () == TYPE_CODE_TYPEDEF)

>       {

>         if (!TYPE_TARGET_TYPE (type))

> @@ -2778,10 +2781,13 @@ check_typedef (struct type *type)

>   	 outer cast in a chain of casting win), instead of assuming

>   	 "it can't happen".  */

>         {

> -	const int ALL_SPACES = (TYPE_INSTANCE_FLAG_CODE_SPACE

> -				| TYPE_INSTANCE_FLAG_DATA_SPACE);

> -	const int ALL_CLASSES = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;

> -	int new_instance_flags = TYPE_INSTANCE_FLAGS (type);

> +	const type_instance_flags ALL_SPACES

> +	  = (TYPE_INSTANCE_FLAG_CODE_SPACE

> +	     | TYPE_INSTANCE_FLAG_DATA_SPACE);

> +	const type_instance_flags ALL_CLASSES

> +	  = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;

> +	type_instance_flags new_instance_flags

> +	  = TYPE_INSTANCE_FLAGS (type);

>   

>   	/* Treat code vs data spaces and address classes separately.  */

>   	if ((instance_flags & ALL_SPACES) != 0)

> @@ -5026,7 +5032,7 @@ recursive_dump_type (struct type *type, int spaces)

>     gdb_print_host_address (TYPE_CHAIN (type), gdb_stdout);

>     printf_filtered ("\n");

>     printfi_filtered (spaces, "instance_flags 0x%x",

> -		    TYPE_INSTANCE_FLAGS (type));

> +		    (unsigned) TYPE_INSTANCE_FLAGS (type));

>     if (TYPE_CONST (type))

>       {

>         puts_filtered (" TYPE_CONST");

> @@ -5300,7 +5306,7 @@ copy_type_recursive (struct objfile *objfile,

>     if (type->name ())

>       new_type->set_name (xstrdup (type->name ()));

>   

> -  TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);

> +  SET_TYPE_INSTANCE_FLAGS (new_type, TYPE_INSTANCE_FLAGS (type));

>     TYPE_LENGTH (new_type) = TYPE_LENGTH (type);

>   

>     /* Copy the fields.  */

> @@ -5427,7 +5433,7 @@ copy_type (const struct type *type)

>     gdb_assert (TYPE_OBJFILE_OWNED (type));

>   

>     new_type = alloc_type_copy (type);

> -  TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);

> +  SET_TYPE_INSTANCE_FLAGS (new_type, TYPE_INSTANCE_FLAGS (type));

>     TYPE_LENGTH (new_type) = TYPE_LENGTH (type);

>     memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),

>   	  sizeof (struct main_type));

> 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

>   #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type

>   #define TYPE_TARGET_TYPE(thistype) TYPE_MAIN_TYPE(thistype)->target_type

>   #define TYPE_POINTER_TYPE(thistype) (thistype)->pointer_type

> @@ -2117,12 +2120,14 @@ extern struct type *make_atomic_type (struct type *);

>   

>   extern void replace_type (struct type *, struct type *);

>   

> -extern int address_space_name_to_int (struct gdbarch *, const char *);

> +extern type_instance_flags address_space_name_to_int (struct gdbarch *,

> +						      const char *);

>   

> -extern const char *address_space_int_to_name (struct gdbarch *, int);

> +extern const char *address_space_int_to_name (struct gdbarch *,

> +					      type_instance_flags);

>   

> -extern struct type *make_type_with_address_space (struct type *type,

> -						  int space_identifier);

> +extern struct type *make_type_with_address_space

> +  (struct type *type, type_instance_flags space_identifier);

>   

>   extern struct type *lookup_memberptr_type (struct type *, struct type *);

>   

> diff --git a/gdb/stabsread.c b/gdb/stabsread.c

> index d2ff54a47b..ed31dc0111 100644

> --- a/gdb/stabsread.c

> +++ b/gdb/stabsread.c

> @@ -4397,7 +4397,7 @@ cleanup_undefined_types_noname (struct objfile *objfile)

>                and needs to be copied over from the reference type.

>                Since replace_type expects them to be identical, we need

>                to set these flags manually before hand.  */

> -          TYPE_INSTANCE_FLAGS (nat.type) = TYPE_INSTANCE_FLAGS (*type);

> +          SET_TYPE_INSTANCE_FLAGS (nat.type, TYPE_INSTANCE_FLAGS (*type));

>             replace_type (nat.type, *type);

>           }

>       }

> diff --git a/gdb/type-stack.c b/gdb/type-stack.c

> index f8661d7565..608142c849 100644

> --- a/gdb/type-stack.c

> +++ b/gdb/type-stack.c

> @@ -109,7 +109,7 @@ type_stack::follow_types (struct type *follow_type)

>     int done = 0;

>     int make_const = 0;

>     int make_volatile = 0;

> -  int make_addr_space = 0;

> +  type_instance_flags make_addr_space = 0;

>     bool make_restrict = false;

>     bool make_atomic = false;

>     int array_size;

> @@ -128,7 +128,7 @@ type_stack::follow_types (struct type *follow_type)

>   	make_volatile = 1;

>   	break;

>         case tp_space_identifier:

> -	make_addr_space = pop_int ();

> +	make_addr_space = (enum type_instance_flag_value) pop_int ();

>   	break;

>         case tp_atomic:

>   	make_atomic = true;

>
Andrew Burgess Aug. 26, 2020, 10:06 a.m. | #2
* Pedro Alves <pedro@palves.net> [2020-08-21 15:45:22 +0100]:

> The next patch in this series will rewrites enum_flags fixing some API

> holes.  That would cause build failures around code using

> type_instance_flags.  Or rather, that should be using it, but wasn't.

> 

> This patch fixes it by using type_instance_flags throughout instead of

> plain integers.

> 

> Note that we can't make the seemingly obvious change to struct

> type::instance_flags:

> 

>  -  unsigned instance_flags : 9;

>  +  ENUM_BITFIELD (type_instance_flag_value) instance_flags : 9;

> 

> Because G++ complains then that 9 bits isn't sufficient for holding

> all values of type_instance_flag_value.

> 

> So the patch adds a cast to TYPE_INSTANCE_FLAGS, and adds a separate

> SET_TYPE_INSTANCE_FLAGS macro.

> 

> gdb/ChangeLog:

> 

> 	* dwarf2/read.c (read_tag_pointer_type): Use type_instance_flags.

> 	* eval.c (fake_method::fake_method): Use SET_TYPE_INSTANCE_FLAGS.

> 	* gdbarch.h, gdbarch.c: Regenerate.

> 	* gdbarch.sh (address_class_type_flags): Use type_instance_flags.

> 	(address_class_name_to_type_flags): Use type_instance_flags and

> 	bool.

> 	* gdbtypes.c (address_space_name_to_int)

> 	(address_space_int_to_name, make_qualified_type): Use

> 	type_instance_flags.

> 	(make_qualified_type): Use type_instance_flags and

> 	SET_TYPE_INSTANCE_FLAGS.

> 	(make_type_with_address_space, make_cv_type, make_vector_type)

> 	(check_typedef): Use type_instance_flags.

> 	(recursive_dump_type): Cast type_instance_flags to unsigned for

> 	printing.

> 	(copy_type_recursive): Use SET_TYPE_INSTANCE_FLAGS.

> 	* gdbtypes.h (TYPE_INSTANCE_FLAGS): Return a type_instance_flags.

> 	(SET_TYPE_INSTANCE_FLAGS): New.

> 	(address_space_name_to_int, address_space_int_to_name)

> 	(make_type_with_address_space): Pass flags using

> 	type_instance_flags instead of int.

> 	* stabsread.c (cleanup_undefined_types_noname): Use

> 	SET_TYPE_INSTANCE_FLAGS.

> 	* type-stack.c (type_stack::follow_types): Use type_instance_flags.


Pedro,

While testing this I found a few issues building with
--enable-targets=all.  The patch below fixes all of the issues I ran
into.  Feel free to merge this with your work of keep it as a separate
patch, whatever works for you.

Thanks,
Andrew

---

commit b6c8d1ca8077fa7ea2d6f6336b2115b3dcf9dfbb
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Aug 26 11:02:22 2020 +0100

    gdb: additional changes to make use of type_instance_flags more
    
    Further updates to make use of type_instance_flags.
    
    gdb/ChangeLog:
    
            * avr-tdep.c (avr_address_class_type_flags): Update return type.
            (avr_address_class_type_flags_to_name): Update argument types.
            (avr_address_class_name_to_type_flags): Update return type and
            argument types.
            * ft32-tdep.c (ft32_address_class_type_flags): Update return type.
            (ft32_address_class_type_flags_to_name): Update argument types.
            (ft32_address_class_name_to_type_flags): Update return type and
            argument types.
            * gdbarch.c: Regenerate.
            * gdbarch.h: Regenerate.
            * gdbarch.sh (address_class_type_flags_to_name): Update argument
            types.
            * s390-tdep.c (s390_address_class_type_flags): Update return type.
            (s390_address_class_type_flags_to_name): Update argument types.
            (s390_address_class_name_to_type_flags): Update return type and
            argument types.

diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 74ab531711e..0148f4e4db2 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -1372,7 +1372,7 @@ avr_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
    This method maps DW_AT_address_class attributes to a
    type_instance_flag_value.  */
 
-static int
+static type_instance_flags
 avr_address_class_type_flags (int byte_size, int dwarf2_addr_class)
 {
   /* The value 1 of the DW_AT_address_class attribute corresponds to the
@@ -1389,7 +1389,8 @@ avr_address_class_type_flags (int byte_size, int dwarf2_addr_class)
    Convert a type_instance_flag_value to an address space qualifier.  */
 
 static const char*
-avr_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+avr_address_class_type_flags_to_name (struct gdbarch *gdbarch,
+				      type_instance_flags type_flags)
 {
   if (type_flags & AVR_TYPE_INSTANCE_FLAG_ADDRESS_CLASS_FLASH)
     return "flash";
@@ -1401,18 +1402,18 @@ avr_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
 
    Convert an address space qualifier to a type_instance_flag_value.  */
 
-static int
+static bool
 avr_address_class_name_to_type_flags (struct gdbarch *gdbarch,
                                       const char* name,
-                                      int *type_flags_ptr)
+                                      type_instance_flags *type_flags_ptr)
 {
   if (strcmp (name, "flash") == 0)
     {
       *type_flags_ptr = AVR_TYPE_INSTANCE_FLAG_ADDRESS_CLASS_FLASH;
-      return 1;
+      return true;
     }
   else
-    return 0;
+    return false;
 }
 
 /* Initialize the gdbarch structure for the AVR's.  */
diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 99ef69de770..8ce16c06505 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -341,7 +341,7 @@ ft32_pointer_to_address (struct gdbarch *gdbarch,
    This method maps DW_AT_address_class attributes to a
    type_instance_flag_value.  */
 
-static int
+static type_instance_flags
 ft32_address_class_type_flags (int byte_size, int dwarf2_addr_class)
 {
   /* The value 1 of the DW_AT_address_class attribute corresponds to the
@@ -357,7 +357,8 @@ ft32_address_class_type_flags (int byte_size, int dwarf2_addr_class)
    Convert a type_instance_flag_value to an address space qualifier.  */
 
 static const char*
-ft32_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+ft32_address_class_type_flags_to_name (struct gdbarch *gdbarch,
+				       type_instance_flags type_flags)
 {
   if (type_flags & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1)
     return "flash";
@@ -369,18 +370,18 @@ ft32_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
 
    Convert an address space qualifier to a type_instance_flag_value.  */
 
-static int
+static bool
 ft32_address_class_name_to_type_flags (struct gdbarch *gdbarch,
 				       const char* name,
-				       int *type_flags_ptr)
+				       type_instance_flags *type_flags_ptr)
 {
   if (strcmp (name, "flash") == 0)
     {
       *type_flags_ptr = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
-      return 1;
+      return true;
     }
   else
-    return 0;
+    return false;
 }
 
 /* Given a return value in `regbuf' with a type `valtype',
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 2be959ecfc9..a3a67020078 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -3526,7 +3526,7 @@ gdbarch_address_class_type_flags_to_name_p (struct gdbarch *gdbarch)
 }
 
 const char *
-gdbarch_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+gdbarch_address_class_type_flags_to_name (struct gdbarch *gdbarch, type_instance_flags type_flags)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->address_class_type_flags_to_name != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 8a4a384fda9..5ac4f5495c4 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -854,8 +854,8 @@ extern void set_gdbarch_address_class_type_flags (struct gdbarch *gdbarch, gdbar
 
 extern int gdbarch_address_class_type_flags_to_name_p (struct gdbarch *gdbarch);
 
-typedef const char * (gdbarch_address_class_type_flags_to_name_ftype) (struct gdbarch *gdbarch, int type_flags);
-extern const char * gdbarch_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags);
+typedef const char * (gdbarch_address_class_type_flags_to_name_ftype) (struct gdbarch *gdbarch, type_instance_flags type_flags);
+extern const char * gdbarch_address_class_type_flags_to_name (struct gdbarch *gdbarch, type_instance_flags type_flags);
 extern void set_gdbarch_address_class_type_flags_to_name (struct gdbarch *gdbarch, gdbarch_address_class_type_flags_to_name_ftype *address_class_type_flags_to_name);
 
 /* Execute vendor-specific DWARF Call Frame Instruction.  OP is the instruction.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 7e9204119bd..a64afb5a3d2 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -690,7 +690,7 @@ v;int;cannot_step_breakpoint;;;0;0;;0
 # non-steppable watchpoints.
 v;int;have_nonsteppable_watchpoint;;;0;0;;0
 F;type_instance_flags;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class
-M;const char *;address_class_type_flags_to_name;int type_flags;type_flags
+M;const char *;address_class_type_flags_to_name;type_instance_flags type_flags;type_flags
 # Execute vendor-specific DWARF Call Frame Instruction.  OP is the instruction.
 # FS are passed from the generic execute_cfa_program function.
 m;bool;execute_dwarf_cfa_vendor_op;gdb_byte op, struct dwarf2_frame_state *fs;op, fs;;default_execute_dwarf_cfa_vendor_op;;0
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 65cb23705d2..49a507f7c2d 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -1583,7 +1583,7 @@ s390_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
 /* Implement addr_class_type_flags gdbarch method.
    Only used for ABI_LINUX_ZSERIES.  */
 
-static int
+static type_instance_flags
 s390_address_class_type_flags (int byte_size, int dwarf2_addr_class)
 {
   if (byte_size == 4)
@@ -1596,7 +1596,8 @@ s390_address_class_type_flags (int byte_size, int dwarf2_addr_class)
    Only used for ABI_LINUX_ZSERIES.  */
 
 static const char *
-s390_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
+s390_address_class_type_flags_to_name (struct gdbarch *gdbarch,
+				       type_instance_flags type_flags)
 {
   if (type_flags & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1)
     return "mode32";
@@ -1607,18 +1608,18 @@ s390_address_class_type_flags_to_name (struct gdbarch *gdbarch, int type_flags)
 /* Implement addr_class_name_to_type_flags gdbarch method.
    Only used for ABI_LINUX_ZSERIES.  */
 
-static int
+static bool
 s390_address_class_name_to_type_flags (struct gdbarch *gdbarch,
 				       const char *name,
-				       int *type_flags_ptr)
+				       type_instance_flags *type_flags_ptr)
 {
   if (strcmp (name, "mode32") == 0)
     {
       *type_flags_ptr = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
-      return 1;
+      return true;
     }
   else
-    return 0;
+    return false;
 }
 
 /* Inferior function calls.  */
Andrew Burgess Aug. 26, 2020, 3:21 p.m. | #3
* Pedro Alves <pedro@palves.net> [2020-08-21 15:45:22 +0100]:

> The next patch in this series will rewrites enum_flags fixing some API

> holes.  That would cause build failures around code using

> type_instance_flags.  Or rather, that should be using it, but wasn't.

> 

> This patch fixes it by using type_instance_flags throughout instead of

> plain integers.

> 

> Note that we can't make the seemingly obvious change to struct

> type::instance_flags:

> 

>  -  unsigned instance_flags : 9;

>  +  ENUM_BITFIELD (type_instance_flag_value) instance_flags : 9;

> 

> Because G++ complains then that 9 bits isn't sufficient for holding

> all values of type_instance_flag_value.

> 

> So the patch adds a cast to TYPE_INSTANCE_FLAGS, and adds a separate

> SET_TYPE_INSTANCE_FLAGS macro.

> 

> gdb/ChangeLog:

> 

> 	* dwarf2/read.c (read_tag_pointer_type): Use type_instance_flags.

> 	* eval.c (fake_method::fake_method): Use SET_TYPE_INSTANCE_FLAGS.

> 	* gdbarch.h, gdbarch.c: Regenerate.

> 	* gdbarch.sh (address_class_type_flags): Use type_instance_flags.

> 	(address_class_name_to_type_flags): Use type_instance_flags and

> 	bool.

> 	* gdbtypes.c (address_space_name_to_int)

> 	(address_space_int_to_name, make_qualified_type): Use

> 	type_instance_flags.

> 	(make_qualified_type): Use type_instance_flags and

> 	SET_TYPE_INSTANCE_FLAGS.

> 	(make_type_with_address_space, make_cv_type, make_vector_type)

> 	(check_typedef): Use type_instance_flags.

> 	(recursive_dump_type): Cast type_instance_flags to unsigned for

> 	printing.

> 	(copy_type_recursive): Use SET_TYPE_INSTANCE_FLAGS.

> 	* gdbtypes.h (TYPE_INSTANCE_FLAGS): Return a type_instance_flags.

> 	(SET_TYPE_INSTANCE_FLAGS): New.

> 	(address_space_name_to_int, address_space_int_to_name)

> 	(make_type_with_address_space): Pass flags using

> 	type_instance_flags instead of int.

> 	* stabsread.c (cleanup_undefined_types_noname): Use

> 	SET_TYPE_INSTANCE_FLAGS.

> 	* type-stack.c (type_stack::follow_types): Use type_instance_flags.

> ---

>  gdb/dwarf2/read.c |  7 +++----

>  gdb/eval.c        |  2 +-

>  gdb/gdbarch.c     |  6 +++---

>  gdb/gdbarch.h     | 12 ++++++------

>  gdb/gdbarch.sh    |  8 ++++----

>  gdb/gdbtypes.c    | 58 ++++++++++++++++++++++++++++++-------------------------

>  gdb/gdbtypes.h    | 15 +++++++++-----

>  gdb/stabsread.c   |  2 +-

>  gdb/type-stack.c  |  4 ++--

>  9 files changed, 62 insertions(+), 52 deletions(-)

> 

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c

> index 0ac8533263..4ced5ac02b 100644

> --- a/gdb/dwarf2/read.c

> +++ b/gdb/dwarf2/read.c

> @@ -17292,10 +17292,9 @@ read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)

>      {

>        if (gdbarch_address_class_type_flags_p (gdbarch))

>  	{

> -	  int type_flags;

> -

> -	  type_flags = gdbarch_address_class_type_flags

> -			 (gdbarch, byte_size, addr_class);

> +	  type_instance_flags type_flags

> +	    = gdbarch_address_class_type_flags (gdbarch, byte_size,

> +						addr_class);

>  	  gdb_assert ((type_flags & ~TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)

>  		      == 0);

>  	  type = make_type_with_address_space (type, type_flags);

> diff --git a/gdb/eval.c b/gdb/eval.c

> index c62c35f318..cd300ddfef 100644

> --- a/gdb/eval.c

> +++ b/gdb/eval.c

> @@ -659,7 +659,7 @@ fake_method::fake_method (type_instance_flags flags,

>    TYPE_LENGTH (type) = 1;

>    type->set_code (TYPE_CODE_METHOD);

>    TYPE_CHAIN (type) = type;

> -  TYPE_INSTANCE_FLAGS (type) = flags;

> +  SET_TYPE_INSTANCE_FLAGS (type, flags);

>    if (num_types > 0)

>      {

>        if (param_types[num_types - 1] == NULL)

> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c

> index f8fe03ca68..2be959ecfc 100644

> --- a/gdb/gdbarch.c

> +++ b/gdb/gdbarch.c

> @@ -3501,7 +3501,7 @@ gdbarch_address_class_type_flags_p (struct gdbarch *gdbarch)

>    return gdbarch->address_class_type_flags != NULL;

>  }

>  

> -int

> +type_instance_flags

>  gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class)

>  {

>    gdb_assert (gdbarch != NULL);

> @@ -3566,8 +3566,8 @@ gdbarch_address_class_name_to_type_flags_p (struct gdbarch *gdbarch)

>    return gdbarch->address_class_name_to_type_flags != NULL;

>  }

>  

> -int

> -gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr)

> +bool

> +gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr)

>  {

>    gdb_assert (gdbarch != NULL);

>    gdb_assert (gdbarch->address_class_name_to_type_flags != NULL);

> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h

> index 7a3060e628..8a4a384fda 100644

> --- a/gdb/gdbarch.h

> +++ b/gdb/gdbarch.h

> @@ -848,8 +848,8 @@ extern void set_gdbarch_have_nonsteppable_watchpoint (struct gdbarch *gdbarch, i

>  

>  extern int gdbarch_address_class_type_flags_p (struct gdbarch *gdbarch);

>  

> -typedef int (gdbarch_address_class_type_flags_ftype) (int byte_size, int dwarf2_addr_class);

> -extern int gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class);

> +typedef type_instance_flags (gdbarch_address_class_type_flags_ftype) (int byte_size, int dwarf2_addr_class);

> +extern type_instance_flags gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class);

>  extern void set_gdbarch_address_class_type_flags (struct gdbarch *gdbarch, gdbarch_address_class_type_flags_ftype *address_class_type_flags);

>  

>  extern int gdbarch_address_class_type_flags_to_name_p (struct gdbarch *gdbarch);

> @@ -866,13 +866,13 @@ extern bool gdbarch_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdb_by

>  extern void set_gdbarch_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdbarch_execute_dwarf_cfa_vendor_op_ftype *execute_dwarf_cfa_vendor_op);

>  

>  /* Return the appropriate type_flags for the supplied address class.

> -   This function should return 1 if the address class was recognized and

> -   type_flags was set, zero otherwise. */

> +   This function should return true if the address class was recognized and

> +   type_flags was set, false otherwise. */

>  

>  extern int gdbarch_address_class_name_to_type_flags_p (struct gdbarch *gdbarch);

>  

> -typedef int (gdbarch_address_class_name_to_type_flags_ftype) (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr);

> -extern int gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr);

> +typedef bool (gdbarch_address_class_name_to_type_flags_ftype) (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr);

> +extern bool gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr);

>  extern void set_gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, gdbarch_address_class_name_to_type_flags_ftype *address_class_name_to_type_flags);

>  

>  /* Is a register in a group */

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh

> index 6d3c5c889d..7e9204119b 100755

> --- a/gdb/gdbarch.sh

> +++ b/gdb/gdbarch.sh

> @@ -689,16 +689,16 @@ v;int;cannot_step_breakpoint;;;0;0;;0

>  # See comment in target.h about continuable, steppable and

>  # non-steppable watchpoints.

>  v;int;have_nonsteppable_watchpoint;;;0;0;;0

> -F;int;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class

> +F;type_instance_flags;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class

>  M;const char *;address_class_type_flags_to_name;int type_flags;type_flags

>  # Execute vendor-specific DWARF Call Frame Instruction.  OP is the instruction.

>  # FS are passed from the generic execute_cfa_program function.

>  m;bool;execute_dwarf_cfa_vendor_op;gdb_byte op, struct dwarf2_frame_state *fs;op, fs;;default_execute_dwarf_cfa_vendor_op;;0

>  

>  # Return the appropriate type_flags for the supplied address class.

> -# This function should return 1 if the address class was recognized and

> -# type_flags was set, zero otherwise.

> -M;int;address_class_name_to_type_flags;const char *name, int *type_flags_ptr;name, type_flags_ptr

> +# This function should return true if the address class was recognized and

> +# type_flags was set, false otherwise.

> +M;bool;address_class_name_to_type_flags;const char *name, type_instance_flags *type_flags_ptr;name, type_flags_ptr

>  # Is a register in a group

>  m;int;register_reggroup_p;int regnum, struct reggroup *reggroup;regnum, reggroup;;default_register_reggroup_p;;0

>  # Fetch the pointer to the ith function argument.

> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c

> index c1eb03d898..64e44bfe23 100644

> --- a/gdb/gdbtypes.c

> +++ b/gdb/gdbtypes.c

> @@ -574,11 +574,11 @@ lookup_function_type_with_arguments (struct type *type,

>  /* Identify address space identifier by name --

>     return the integer flag defined in gdbtypes.h.  */

>  

> -int

> +type_instance_flags

>  address_space_name_to_int (struct gdbarch *gdbarch,

>  			   const char *space_identifier)

>  {

> -  int type_flags;

> +  type_instance_flags type_flags;

>  

>    /* Check for known address space delimiters.  */

>    if (!strcmp (space_identifier, "code"))

> @@ -598,7 +598,8 @@ address_space_name_to_int (struct gdbarch *gdbarch,

>     gdbtypes.h -- return the string version of the adress space name.  */

>  

>  const char *

> -address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)

> +address_space_int_to_name (struct gdbarch *gdbarch,

> +			   type_instance_flags space_flag)

>  {

>    if (space_flag & TYPE_INSTANCE_FLAG_CODE_SPACE)

>      return "code";

> @@ -617,7 +618,7 @@ address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)

>     STORAGE must be in the same obstack as TYPE.  */

>  

>  static struct type *

> -make_qualified_type (struct type *type, int new_flags,

> +make_qualified_type (struct type *type, type_instance_flags new_flags,

>  		     struct type *storage)

>  {

>    struct type *ntype;

> @@ -657,7 +658,7 @@ make_qualified_type (struct type *type, int new_flags,

>    TYPE_CHAIN (type) = ntype;

>  

>    /* Now set the instance flags and return the new type.  */

> -  TYPE_INSTANCE_FLAGS (ntype) = new_flags;

> +  SET_TYPE_INSTANCE_FLAGS (ntype, new_flags);

>  

>    /* Set length of new type to that of the original type.  */

>    TYPE_LENGTH (ntype) = TYPE_LENGTH (type);

> @@ -675,13 +676,14 @@ make_qualified_type (struct type *type, int new_flags,

>     representations.  */

>  

>  struct type *

> -make_type_with_address_space (struct type *type, int space_flag)

> +make_type_with_address_space (struct type *type,

> +			      type_instance_flags space_flag)

>  {

> -  int new_flags = ((TYPE_INSTANCE_FLAGS (type)

> -		    & ~(TYPE_INSTANCE_FLAG_CODE_SPACE

> -			| TYPE_INSTANCE_FLAG_DATA_SPACE

> -		        | TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL))

> -		   | space_flag);

> +  type_instance_flags new_flags = ((TYPE_INSTANCE_FLAGS (type)

> +				    & ~(TYPE_INSTANCE_FLAG_CODE_SPACE

> +					| TYPE_INSTANCE_FLAG_DATA_SPACE

> +					| TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL))

> +				   | space_flag);

>  

>    return make_qualified_type (type, new_flags, NULL);

>  }

> @@ -705,9 +707,9 @@ make_cv_type (int cnst, int voltl,

>  {

>    struct type *ntype;	/* New type */

>  

> -  int new_flags = (TYPE_INSTANCE_FLAGS (type)

> -		   & ~(TYPE_INSTANCE_FLAG_CONST 

> -		       | TYPE_INSTANCE_FLAG_VOLATILE));

> +  type_instance_flags new_flags = (TYPE_INSTANCE_FLAGS (type)

> +				   & ~(TYPE_INSTANCE_FLAG_CONST

> +				       | TYPE_INSTANCE_FLAG_VOLATILE));

>  

>    if (cnst)

>      new_flags |= TYPE_INSTANCE_FLAG_CONST;

> @@ -1410,7 +1412,6 @@ void

>  make_vector_type (struct type *array_type)

>  {

>    struct type *inner_array, *elt_type;

> -  int flags;

>  

>    /* Find the innermost array type, in case the array is

>       multi-dimensional.  */

> @@ -1421,7 +1422,8 @@ make_vector_type (struct type *array_type)

>    elt_type = TYPE_TARGET_TYPE (inner_array);

>    if (elt_type->code () == TYPE_CODE_INT)

>      {

> -      flags = TYPE_INSTANCE_FLAGS (elt_type) | TYPE_INSTANCE_FLAG_NOTTEXT;

> +      type_instance_flags flags

> +	= TYPE_INSTANCE_FLAGS (elt_type) | TYPE_INSTANCE_FLAG_NOTTEXT;

>        elt_type = make_qualified_type (elt_type, flags, NULL);

>        TYPE_TARGET_TYPE (inner_array) = elt_type;

>      }

> @@ -2732,12 +2734,13 @@ struct type *

>  check_typedef (struct type *type)

>  {

>    struct type *orig_type = type;

> -  /* While we're removing typedefs, we don't want to lose qualifiers.

> -     E.g., const/volatile.  */

> -  int instance_flags = TYPE_INSTANCE_FLAGS (type);

>  

>    gdb_assert (type);

>  

> +  /* While we're removing typedefs, we don't want to lose qualifiers.

> +     E.g., const/volatile.  */

> +  type_instance_flags instance_flags = TYPE_INSTANCE_FLAGS (type);

> +

>    while (type->code () == TYPE_CODE_TYPEDEF)

>      {

>        if (!TYPE_TARGET_TYPE (type))

> @@ -2778,10 +2781,13 @@ check_typedef (struct type *type)

>  	 outer cast in a chain of casting win), instead of assuming

>  	 "it can't happen".  */

>        {

> -	const int ALL_SPACES = (TYPE_INSTANCE_FLAG_CODE_SPACE

> -				| TYPE_INSTANCE_FLAG_DATA_SPACE);

> -	const int ALL_CLASSES = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;

> -	int new_instance_flags = TYPE_INSTANCE_FLAGS (type);

> +	const type_instance_flags ALL_SPACES

> +	  = (TYPE_INSTANCE_FLAG_CODE_SPACE

> +	     | TYPE_INSTANCE_FLAG_DATA_SPACE);

> +	const type_instance_flags ALL_CLASSES

> +	  = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;

> +	type_instance_flags new_instance_flags

> +	  = TYPE_INSTANCE_FLAGS (type);

>  

>  	/* Treat code vs data spaces and address classes separately.  */

>  	if ((instance_flags & ALL_SPACES) != 0)

> @@ -5026,7 +5032,7 @@ recursive_dump_type (struct type *type, int spaces)

>    gdb_print_host_address (TYPE_CHAIN (type), gdb_stdout);

>    printf_filtered ("\n");

>    printfi_filtered (spaces, "instance_flags 0x%x", 

> -		    TYPE_INSTANCE_FLAGS (type));

> +		    (unsigned) TYPE_INSTANCE_FLAGS (type));

>    if (TYPE_CONST (type))

>      {

>        puts_filtered (" TYPE_CONST");

> @@ -5300,7 +5306,7 @@ copy_type_recursive (struct objfile *objfile,

>    if (type->name ())

>      new_type->set_name (xstrdup (type->name ()));

>  

> -  TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);

> +  SET_TYPE_INSTANCE_FLAGS (new_type, TYPE_INSTANCE_FLAGS (type));

>    TYPE_LENGTH (new_type) = TYPE_LENGTH (type);

>  

>    /* Copy the fields.  */

> @@ -5427,7 +5433,7 @@ copy_type (const struct type *type)

>    gdb_assert (TYPE_OBJFILE_OWNED (type));

>  

>    new_type = alloc_type_copy (type);

> -  TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);

> +  SET_TYPE_INSTANCE_FLAGS (new_type, TYPE_INSTANCE_FLAGS (type));

>    TYPE_LENGTH (new_type) = TYPE_LENGTH (type);

>    memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),

>  	  sizeof (struct main_type));

> 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.

Thanks,
Andrew

---

commit 77f72bf3ec4f67354bb42b99b5115001f08bd492
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Aug 26 16:19:22 2020 +0100

    gdb: use SET_TYPE_INSTANCE_FLAGS more
    
    A few places where SET_TYPE_INSTANCE_FLAGS should be used.
    
    gdb/ChangeLog:
    
            * d-lang.c (build_d_types): Use SET_TYPE_INSTANCE_FLAGS.
            * ft32-tdep.c (ft32_gdbarch_init): Likewise.
            * gdbtypes.c (gdbtypes_post_init): Likewise.
            * gdbtypes.h (struct type) <get_instance_flags>: New member
            function.
            <set_instance_flags>: Likewise.
            (TYPE_INSTANCE_FLAGS): Update.
            (SET_TYPE_INSTANCE_FLAGS): Update.

diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index 4ebb011ee9b..6b1f68b827e 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -321,10 +321,12 @@ build_d_types (struct gdbarch *gdbarch)
     = arch_float_type (gdbarch, gdbarch_long_double_bit (gdbarch),
 		       "real", gdbarch_long_double_format (gdbarch));
 
-  TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_byte)
-    |= TYPE_INSTANCE_FLAG_NOTTEXT;
-  TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_ubyte)
-    |= TYPE_INSTANCE_FLAG_NOTTEXT;
+  SET_TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_byte,
+			   (TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_byte)
+			    | TYPE_INSTANCE_FLAG_NOTTEXT));
+  SET_TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_ubyte,
+			   (TYPE_INSTANCE_FLAGS (builtin_d_type->builtin_ubyte)
+			    | TYPE_INSTANCE_FLAG_NOTTEXT));
 
   /* Imaginary and complex types.  */
   builtin_d_type->builtin_ifloat
diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 8ce16c06505..c3a83149bec 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -577,7 +577,9 @@ ft32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   func_void_type = make_function_type (void_type, NULL);
   tdep->pc_type = arch_pointer_type (gdbarch, 4 * TARGET_CHAR_BIT, NULL,
 				     func_void_type);
-  TYPE_INSTANCE_FLAGS (tdep->pc_type) |= TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1;
+  SET_TYPE_INSTANCE_FLAGS (tdep->pc_type,
+			   (TYPE_INSTANCE_FLAGS (tdep->pc_type)
+			    | TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1));
 
   set_gdbarch_num_regs (gdbarch, FT32_NUM_REGS);
   set_gdbarch_sp_regnum (gdbarch, FT32_SP_REGNUM);
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 64e44bfe23d..394abaae106 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5823,10 +5823,12 @@ gdbtypes_post_init (struct gdbarch *gdbarch)
     = arch_integer_type (gdbarch, 128, 0, "int128_t");
   builtin_type->builtin_uint128
     = arch_integer_type (gdbarch, 128, 1, "uint128_t");
-  TYPE_INSTANCE_FLAGS (builtin_type->builtin_int8) |=
-    TYPE_INSTANCE_FLAG_NOTTEXT;
-  TYPE_INSTANCE_FLAGS (builtin_type->builtin_uint8) |=
-    TYPE_INSTANCE_FLAG_NOTTEXT;
+  SET_TYPE_INSTANCE_FLAGS (builtin_type->builtin_int8,
+			   (TYPE_INSTANCE_FLAGS (builtin_type->builtin_int8)
+			    | TYPE_INSTANCE_FLAG_NOTTEXT));
+  SET_TYPE_INSTANCE_FLAGS (builtin_type->builtin_uint8,
+			   (TYPE_INSTANCE_FLAGS (builtin_type->builtin_uint8)
+			    | TYPE_INSTANCE_FLAG_NOTTEXT));
 
   /* Wide character types.  */
   builtin_type->builtin_char16
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index b42cef61371..9c0289b18b2 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1037,6 +1037,18 @@ struct type
     this->field (0).set_type (index_type);
   }
 
+  /* Return the instance flags converted to the correct type.  */
+  const type_instance_flags get_instance_flags () const
+  {
+    return type_instance_flags ((enum type_instance_flag_value) this->instance_flags);
+  }
+
+  /* Set the instance flags.  */
+  void set_instance_flags (type_instance_flags flags)
+  {
+    this->instance_flags = flags;
+  }
+
   /* Get the bounds bounds of this type.  The type must be a range type.  */
   range_bounds *bounds () const
   {
@@ -1585,10 +1597,9 @@ 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) \
-  type_instance_flags ((enum type_instance_flag_value) (thistype)->instance_flags)
-#define SET_TYPE_INSTANCE_FLAGS(thistype, flags) \
-  (thistype)->instance_flags = flags
+#define TYPE_INSTANCE_FLAGS(thistype) ((thistype)->get_instance_flags ())
+#define SET_TYPE_INSTANCE_FLAGS(thistype, flags)	\
+  ((thistype)->set_instance_flags (flags))
 #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
 #define TYPE_TARGET_TYPE(thistype) TYPE_MAIN_TYPE(thistype)->target_type
 #define TYPE_POINTER_TYPE(thistype) (thistype)->pointer_type
Tom Tromey Sept. 11, 2020, 8:21 p.m. | #4
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:


Pedro> -int
Pedro> +type_instance_flags
Pedro>  address_space_name_to_int (struct gdbarch *gdbarch,

Maybe this is misnamed after the change.

Pedro>  const char *
Pedro> -address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)
Pedro> +address_space_int_to_name (struct gdbarch *gdbarch,

Likewise.

Tom
Pedro Alves Sept. 14, 2020, 11:56 a.m. | #5
Hi Luis,

I should have replied to this promptly.  Sorry about that.

On 8/25/20 12:36 PM, Luis Machado wrote:
> Hi,

> 

> On 8/21/20 11:45 AM, Pedro Alves wrote:

>> The next patch in this series will rewrites enum_flags fixing some API

>> holes.  That would cause build failures around code using

>> type_instance_flags.  Or rather, that should be using it, but wasn't.

>>

>> This patch fixes it by using type_instance_flags throughout instead of

>> plain integers.

>>

>> Note that we can't make the seemingly obvious change to struct

>> type::instance_flags:

>>

>>   -  unsigned instance_flags : 9;

>>   +  ENUM_BITFIELD (type_instance_flag_value) instance_flags : 9;

>>

>> Because G++ complains then that 9 bits isn't sufficient for holding

>> all values of type_instance_flag_value.

> 

> I ran into the problem above the last time I used instance flags (address classes), but mostly because of the (rather silly) 9 bit (or whatever current number is there) limitation.

> 

> Isn't this more an artifact of trying to save space where there isn't a need to anymore? Wouldn't a 64-bit integer suffice here? That would give us 64 flags worth of data and virtually no problems until we ran out of bits.

> 


Well, I'm not exactly sure about struct type, but symbol related
structures are space sensitive, a few bytes here and there can make
a significant difference when you have thousands or millions of 
instances.  Once in a while someone tries to shrink these structures,
see if we can pack fields better by reordering fields, etc.

Here's what we have today, on x86-64:

 (top-gdb) ptype /o type
 /* offset    |  size */  type = struct type {
 /*    0      |     8 */    type *pointer_type;
 /*    8      |     8 */    type *reference_type;
 /*   16      |     8 */    type *rvalue_reference_type;
 /*   24      |     8 */    type *chain;
 /*   32: 0   |     4 */    unsigned int align_log2 : 8;
 /*   33: 0   |     4 */    unsigned int instance_flags : 9;
 /* XXX  7-bit hole   */
 /* XXX  5-byte hole  */
 /*   40      |     8 */    ULONGEST length;
 /*   48      |     8 */    main_type *main_type;
 
                            /* total size (bytes):   56 */
                          }

And if we blindly made instance_flags 64-bit:

 (top-gdb) ptype /o type
 /* offset    |  size */  type = struct type {
 /*    0      |     8 */    type *pointer_type;
 /*    8      |     8 */    type *reference_type;
 /*   16      |     8 */    type *rvalue_reference_type;
 /*   24      |     8 */    type *chain;
 /*   32: 0   |     4 */    unsigned int align_log2 : 8;
 /* XXX  7-byte hole  */
 /*   40      |     8 */    ULONGEST instance_flags;
 /*   48      |     8 */    ULONGEST length;
 /*   56      |     8 */    main_type *main_type;
 
                            /* total size (bytes):   64 */
                          }

Maybe we will need 64 bits for flags in the future anyway, I don't
know.

> Honestly, I don't particularly like the hackery that is going on in traits.h, just to catch something that may be easier fixed with a longer integer type.

> 

> I understand the C++ standard is evolving, but GDB's code base is still full of old code. We don't necessarily need to convert GDB's code base to use the latest and greatest out there, right? We just need to make it good quality and well maintained?


The "hackery" is basically switching to the detection idiom that's now standard in C++17.  We
can remove our implementation in traits.h once we start requiring C++17.  But the "hackery" is
used in the enum flags unit tests -- even if instance_flags was made a plain 64-bits enum, I'd
still want the "hackery".  It's orthogonal to instance_flags.  This patch could be the first
in the series, before the traits.h change.  In fact, I'll do just that.

> 

> Just an opinion from someone that has used these flags before and has been bitten by their storage-space-saving ugliness.

>
Pedro Alves Sept. 14, 2020, 12:54 p.m. | #6
On 8/26/20 11:06 AM, Andrew Burgess wrote:

> While testing this I found a few issues building with

> --enable-targets=all.  The patch below fixes all of the issues I ran

> into.  Feel free to merge this with your work of keep it as a separate

> patch, whatever works for you.


Thanks Andrew.  Sorry I forgot to --enable-targets=all in the first place.
I saw those places needed adjustment when I was grepping around, but then
forgot.

I'll merge this with my patch and add you to the ChangeLog entry.

Pedro
Pedro Alves Sept. 14, 2020, 1:53 p.m. | #7
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
Pedro Alves Sept. 14, 2020, 7:26 p.m. | #8
On 9/11/20 9:21 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

> 

> Pedro> -int

> Pedro> +type_instance_flags

> Pedro>  address_space_name_to_int (struct gdbarch *gdbarch,

> 

> Maybe this is misnamed after the change.

> 

> Pedro>  const char *

> Pedro> -address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)

> Pedro> +address_space_int_to_name (struct gdbarch *gdbarch,

> 

> Likewise.


Yeah, I wasn't sure it was worth it.

I'm adding this patch to the series to address this.

From 924d1538efdfbc2241cbab0c2ad91abbb0921fde Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>

Date: Mon, 14 Sep 2020 19:53:18 +0100
Subject: [PATCH] Rename address_space_int_to_name/address_space_name_to_int

These methods now take/return a type_instance_flags instead of a raw
integer, so rename them accordingly.

gdb/ChangeLog:

	* 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.
	(address_space_int_to_name): Rename to ...
	(address_space_type_instance_flags_to_name): ... this.
	* gdbtypes.h (address_space_name_to_int): Rename to ...
	(address_space_name_to_type_instance_flags): ... this.
	(address_space_int_to_name): Rename to ...
	(address_space_type_instance_flags_to_name): ... this.
	* type-stack.c (type_stack::insert): Adjust to rename.
	* type-stack.h (type_stack::insert): Likewise.
---
 gdb/c-typeprint.c |  5 +++--
 gdb/gdbtypes.c    | 16 ++++++++--------
 gdb/gdbtypes.h    |  8 ++++----
 gdb/type-stack.c  |  5 +++--
 gdb/type-stack.h  | 10 +++++-----
 5 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index b642c88178..a07b29a95d 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -526,8 +526,9 @@ c_type_print_modifier (struct type *type, struct ui_file *stream,
       did_print_modifier = 1;
     }
 
-  address_space_id = address_space_int_to_name (get_type_arch (type),
-						TYPE_INSTANCE_FLAGS (type));
+  address_space_id
+    = address_space_type_instance_flags_to_name (get_type_arch (type),
+						 TYPE_INSTANCE_FLAGS (type));
   if (address_space_id)
     {
       if (did_print_modifier || need_pre_space)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index b57353dec3..7ade2ccb53 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -575,12 +575,12 @@ lookup_function_type_with_arguments (struct type *type,
   return fn;
 }
 
-/* Identify address space identifier by name --
-   return the integer flag defined in gdbtypes.h.  */
+/* Identify address space identifier by name -- return a
+   type_instance_flags.  */
 
 type_instance_flags
-address_space_name_to_int (struct gdbarch *gdbarch,
-			   const char *space_identifier)
+address_space_name_to_type_instance_flags (struct gdbarch *gdbarch,
+					   const char *space_identifier)
 {
   type_instance_flags type_flags;
 
@@ -598,12 +598,12 @@ address_space_name_to_int (struct gdbarch *gdbarch,
     error (_("Unknown address space specifier: \"%s\""), space_identifier);
 }
 
-/* Identify address space identifier by integer flag as defined in 
-   gdbtypes.h -- return the string version of the adress space name.  */
+/* Identify address space identifier by type_instance_flags and return
+   the string version of the adress space name.  */
 
 const char *
-address_space_int_to_name (struct gdbarch *gdbarch,
-			   type_instance_flags space_flag)
+address_space_type_instance_flags_to_name (struct gdbarch *gdbarch,
+					   type_instance_flags space_flag)
 {
   if (space_flag & TYPE_INSTANCE_FLAG_CODE_SPACE)
     return "code";
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index cdd136ef52..6b87de307b 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2217,11 +2217,11 @@ extern struct type *make_atomic_type (struct type *);
 
 extern void replace_type (struct type *, struct type *);
 
-extern type_instance_flags address_space_name_to_int (struct gdbarch *,
-						      const char *);
+extern type_instance_flags address_space_name_to_type_instance_flags
+  (struct gdbarch *, const char *);
 
-extern const char *address_space_int_to_name (struct gdbarch *,
-					      type_instance_flags);
+extern const char *address_space_type_instance_flags_to_name
+  (struct gdbarch *, type_instance_flags);
 
 extern struct type *make_type_with_address_space
   (struct type *type, type_instance_flags space_identifier);
diff --git a/gdb/type-stack.c b/gdb/type-stack.c
index 608142c849..94ff9ba8c9 100644
--- a/gdb/type-stack.c
+++ b/gdb/type-stack.c
@@ -67,8 +67,9 @@ type_stack::insert (struct expr_builder *pstate, const char *string)
 
   element.piece = tp_space_identifier;
   insert_into (slot, element);
-  element.int_val = address_space_name_to_int (pstate->gdbarch (),
-					       string);
+  element.int_val
+    = address_space_name_to_type_instance_flags (pstate->gdbarch (),
+						 string);
   insert_into (slot, element);
 }
 
diff --git a/gdb/type-stack.h b/gdb/type-stack.h
index 8060f2fea7..265178fc37 100644
--- a/gdb/type-stack.h
+++ b/gdb/type-stack.h
@@ -157,11 +157,11 @@ struct type_stack
 
   /* Insert a tp_space_identifier and the corresponding address space
      value into the stack.  STRING is the name of an address space, as
-     recognized by address_space_name_to_int.  If the stack is empty,
-     the new elements are simply pushed.  If the stack is not empty,
-     this function assumes that the first item on the stack is a
-     tp_pointer, and the new values are inserted above the first
-     item.  */
+     recognized by address_space_name_to_type_instance_flags.  If the
+     stack is empty, the new elements are simply pushed.  If the stack
+     is not empty, this function assumes that the first item on the
+     stack is a tp_pointer, and the new values are inserted above the
+     first item.  */
 
   void insert (struct expr_builder *pstate, const char *string);
 

-- 
2.14.5

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 0ac8533263..4ced5ac02b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -17292,10 +17292,9 @@  read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
     {
       if (gdbarch_address_class_type_flags_p (gdbarch))
 	{
-	  int type_flags;
-
-	  type_flags = gdbarch_address_class_type_flags
-			 (gdbarch, byte_size, addr_class);
+	  type_instance_flags type_flags
+	    = gdbarch_address_class_type_flags (gdbarch, byte_size,
+						addr_class);
 	  gdb_assert ((type_flags & ~TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
 		      == 0);
 	  type = make_type_with_address_space (type, type_flags);
diff --git a/gdb/eval.c b/gdb/eval.c
index c62c35f318..cd300ddfef 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -659,7 +659,7 @@  fake_method::fake_method (type_instance_flags flags,
   TYPE_LENGTH (type) = 1;
   type->set_code (TYPE_CODE_METHOD);
   TYPE_CHAIN (type) = type;
-  TYPE_INSTANCE_FLAGS (type) = flags;
+  SET_TYPE_INSTANCE_FLAGS (type, flags);
   if (num_types > 0)
     {
       if (param_types[num_types - 1] == NULL)
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index f8fe03ca68..2be959ecfc 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -3501,7 +3501,7 @@  gdbarch_address_class_type_flags_p (struct gdbarch *gdbarch)
   return gdbarch->address_class_type_flags != NULL;
 }
 
-int
+type_instance_flags
 gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class)
 {
   gdb_assert (gdbarch != NULL);
@@ -3566,8 +3566,8 @@  gdbarch_address_class_name_to_type_flags_p (struct gdbarch *gdbarch)
   return gdbarch->address_class_name_to_type_flags != NULL;
 }
 
-int
-gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr)
+bool
+gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->address_class_name_to_type_flags != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 7a3060e628..8a4a384fda 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -848,8 +848,8 @@  extern void set_gdbarch_have_nonsteppable_watchpoint (struct gdbarch *gdbarch, i
 
 extern int gdbarch_address_class_type_flags_p (struct gdbarch *gdbarch);
 
-typedef int (gdbarch_address_class_type_flags_ftype) (int byte_size, int dwarf2_addr_class);
-extern int gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class);
+typedef type_instance_flags (gdbarch_address_class_type_flags_ftype) (int byte_size, int dwarf2_addr_class);
+extern type_instance_flags gdbarch_address_class_type_flags (struct gdbarch *gdbarch, int byte_size, int dwarf2_addr_class);
 extern void set_gdbarch_address_class_type_flags (struct gdbarch *gdbarch, gdbarch_address_class_type_flags_ftype *address_class_type_flags);
 
 extern int gdbarch_address_class_type_flags_to_name_p (struct gdbarch *gdbarch);
@@ -866,13 +866,13 @@  extern bool gdbarch_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdb_by
 extern void set_gdbarch_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdbarch_execute_dwarf_cfa_vendor_op_ftype *execute_dwarf_cfa_vendor_op);
 
 /* Return the appropriate type_flags for the supplied address class.
-   This function should return 1 if the address class was recognized and
-   type_flags was set, zero otherwise. */
+   This function should return true if the address class was recognized and
+   type_flags was set, false otherwise. */
 
 extern int gdbarch_address_class_name_to_type_flags_p (struct gdbarch *gdbarch);
 
-typedef int (gdbarch_address_class_name_to_type_flags_ftype) (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr);
-extern int gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, int *type_flags_ptr);
+typedef bool (gdbarch_address_class_name_to_type_flags_ftype) (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr);
+extern bool gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, const char *name, type_instance_flags *type_flags_ptr);
 extern void set_gdbarch_address_class_name_to_type_flags (struct gdbarch *gdbarch, gdbarch_address_class_name_to_type_flags_ftype *address_class_name_to_type_flags);
 
 /* Is a register in a group */
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 6d3c5c889d..7e9204119b 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -689,16 +689,16 @@  v;int;cannot_step_breakpoint;;;0;0;;0
 # See comment in target.h about continuable, steppable and
 # non-steppable watchpoints.
 v;int;have_nonsteppable_watchpoint;;;0;0;;0
-F;int;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class
+F;type_instance_flags;address_class_type_flags;int byte_size, int dwarf2_addr_class;byte_size, dwarf2_addr_class
 M;const char *;address_class_type_flags_to_name;int type_flags;type_flags
 # Execute vendor-specific DWARF Call Frame Instruction.  OP is the instruction.
 # FS are passed from the generic execute_cfa_program function.
 m;bool;execute_dwarf_cfa_vendor_op;gdb_byte op, struct dwarf2_frame_state *fs;op, fs;;default_execute_dwarf_cfa_vendor_op;;0
 
 # Return the appropriate type_flags for the supplied address class.
-# This function should return 1 if the address class was recognized and
-# type_flags was set, zero otherwise.
-M;int;address_class_name_to_type_flags;const char *name, int *type_flags_ptr;name, type_flags_ptr
+# This function should return true if the address class was recognized and
+# type_flags was set, false otherwise.
+M;bool;address_class_name_to_type_flags;const char *name, type_instance_flags *type_flags_ptr;name, type_flags_ptr
 # Is a register in a group
 m;int;register_reggroup_p;int regnum, struct reggroup *reggroup;regnum, reggroup;;default_register_reggroup_p;;0
 # Fetch the pointer to the ith function argument.
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c1eb03d898..64e44bfe23 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -574,11 +574,11 @@  lookup_function_type_with_arguments (struct type *type,
 /* Identify address space identifier by name --
    return the integer flag defined in gdbtypes.h.  */
 
-int
+type_instance_flags
 address_space_name_to_int (struct gdbarch *gdbarch,
 			   const char *space_identifier)
 {
-  int type_flags;
+  type_instance_flags type_flags;
 
   /* Check for known address space delimiters.  */
   if (!strcmp (space_identifier, "code"))
@@ -598,7 +598,8 @@  address_space_name_to_int (struct gdbarch *gdbarch,
    gdbtypes.h -- return the string version of the adress space name.  */
 
 const char *
-address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)
+address_space_int_to_name (struct gdbarch *gdbarch,
+			   type_instance_flags space_flag)
 {
   if (space_flag & TYPE_INSTANCE_FLAG_CODE_SPACE)
     return "code";
@@ -617,7 +618,7 @@  address_space_int_to_name (struct gdbarch *gdbarch, int space_flag)
    STORAGE must be in the same obstack as TYPE.  */
 
 static struct type *
-make_qualified_type (struct type *type, int new_flags,
+make_qualified_type (struct type *type, type_instance_flags new_flags,
 		     struct type *storage)
 {
   struct type *ntype;
@@ -657,7 +658,7 @@  make_qualified_type (struct type *type, int new_flags,
   TYPE_CHAIN (type) = ntype;
 
   /* Now set the instance flags and return the new type.  */
-  TYPE_INSTANCE_FLAGS (ntype) = new_flags;
+  SET_TYPE_INSTANCE_FLAGS (ntype, new_flags);
 
   /* Set length of new type to that of the original type.  */
   TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
@@ -675,13 +676,14 @@  make_qualified_type (struct type *type, int new_flags,
    representations.  */
 
 struct type *
-make_type_with_address_space (struct type *type, int space_flag)
+make_type_with_address_space (struct type *type,
+			      type_instance_flags space_flag)
 {
-  int new_flags = ((TYPE_INSTANCE_FLAGS (type)
-		    & ~(TYPE_INSTANCE_FLAG_CODE_SPACE
-			| TYPE_INSTANCE_FLAG_DATA_SPACE
-		        | TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL))
-		   | space_flag);
+  type_instance_flags new_flags = ((TYPE_INSTANCE_FLAGS (type)
+				    & ~(TYPE_INSTANCE_FLAG_CODE_SPACE
+					| TYPE_INSTANCE_FLAG_DATA_SPACE
+					| TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL))
+				   | space_flag);
 
   return make_qualified_type (type, new_flags, NULL);
 }
@@ -705,9 +707,9 @@  make_cv_type (int cnst, int voltl,
 {
   struct type *ntype;	/* New type */
 
-  int new_flags = (TYPE_INSTANCE_FLAGS (type)
-		   & ~(TYPE_INSTANCE_FLAG_CONST 
-		       | TYPE_INSTANCE_FLAG_VOLATILE));
+  type_instance_flags new_flags = (TYPE_INSTANCE_FLAGS (type)
+				   & ~(TYPE_INSTANCE_FLAG_CONST
+				       | TYPE_INSTANCE_FLAG_VOLATILE));
 
   if (cnst)
     new_flags |= TYPE_INSTANCE_FLAG_CONST;
@@ -1410,7 +1412,6 @@  void
 make_vector_type (struct type *array_type)
 {
   struct type *inner_array, *elt_type;
-  int flags;
 
   /* Find the innermost array type, in case the array is
      multi-dimensional.  */
@@ -1421,7 +1422,8 @@  make_vector_type (struct type *array_type)
   elt_type = TYPE_TARGET_TYPE (inner_array);
   if (elt_type->code () == TYPE_CODE_INT)
     {
-      flags = TYPE_INSTANCE_FLAGS (elt_type) | TYPE_INSTANCE_FLAG_NOTTEXT;
+      type_instance_flags flags
+	= TYPE_INSTANCE_FLAGS (elt_type) | TYPE_INSTANCE_FLAG_NOTTEXT;
       elt_type = make_qualified_type (elt_type, flags, NULL);
       TYPE_TARGET_TYPE (inner_array) = elt_type;
     }
@@ -2732,12 +2734,13 @@  struct type *
 check_typedef (struct type *type)
 {
   struct type *orig_type = type;
-  /* While we're removing typedefs, we don't want to lose qualifiers.
-     E.g., const/volatile.  */
-  int instance_flags = TYPE_INSTANCE_FLAGS (type);
 
   gdb_assert (type);
 
+  /* While we're removing typedefs, we don't want to lose qualifiers.
+     E.g., const/volatile.  */
+  type_instance_flags instance_flags = TYPE_INSTANCE_FLAGS (type);
+
   while (type->code () == TYPE_CODE_TYPEDEF)
     {
       if (!TYPE_TARGET_TYPE (type))
@@ -2778,10 +2781,13 @@  check_typedef (struct type *type)
 	 outer cast in a chain of casting win), instead of assuming
 	 "it can't happen".  */
       {
-	const int ALL_SPACES = (TYPE_INSTANCE_FLAG_CODE_SPACE
-				| TYPE_INSTANCE_FLAG_DATA_SPACE);
-	const int ALL_CLASSES = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;
-	int new_instance_flags = TYPE_INSTANCE_FLAGS (type);
+	const type_instance_flags ALL_SPACES
+	  = (TYPE_INSTANCE_FLAG_CODE_SPACE
+	     | TYPE_INSTANCE_FLAG_DATA_SPACE);
+	const type_instance_flags ALL_CLASSES
+	  = TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL;
+	type_instance_flags new_instance_flags
+	  = TYPE_INSTANCE_FLAGS (type);
 
 	/* Treat code vs data spaces and address classes separately.  */
 	if ((instance_flags & ALL_SPACES) != 0)
@@ -5026,7 +5032,7 @@  recursive_dump_type (struct type *type, int spaces)
   gdb_print_host_address (TYPE_CHAIN (type), gdb_stdout);
   printf_filtered ("\n");
   printfi_filtered (spaces, "instance_flags 0x%x", 
-		    TYPE_INSTANCE_FLAGS (type));
+		    (unsigned) TYPE_INSTANCE_FLAGS (type));
   if (TYPE_CONST (type))
     {
       puts_filtered (" TYPE_CONST");
@@ -5300,7 +5306,7 @@  copy_type_recursive (struct objfile *objfile,
   if (type->name ())
     new_type->set_name (xstrdup (type->name ()));
 
-  TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);
+  SET_TYPE_INSTANCE_FLAGS (new_type, TYPE_INSTANCE_FLAGS (type));
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
 
   /* Copy the fields.  */
@@ -5427,7 +5433,7 @@  copy_type (const struct type *type)
   gdb_assert (TYPE_OBJFILE_OWNED (type));
 
   new_type = alloc_type_copy (type);
-  TYPE_INSTANCE_FLAGS (new_type) = TYPE_INSTANCE_FLAGS (type);
+  SET_TYPE_INSTANCE_FLAGS (new_type, TYPE_INSTANCE_FLAGS (type));
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
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
 #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
 #define TYPE_TARGET_TYPE(thistype) TYPE_MAIN_TYPE(thistype)->target_type
 #define TYPE_POINTER_TYPE(thistype) (thistype)->pointer_type
@@ -2117,12 +2120,14 @@  extern struct type *make_atomic_type (struct type *);
 
 extern void replace_type (struct type *, struct type *);
 
-extern int address_space_name_to_int (struct gdbarch *, const char *);
+extern type_instance_flags address_space_name_to_int (struct gdbarch *,
+						      const char *);
 
-extern const char *address_space_int_to_name (struct gdbarch *, int);
+extern const char *address_space_int_to_name (struct gdbarch *,
+					      type_instance_flags);
 
-extern struct type *make_type_with_address_space (struct type *type, 
-						  int space_identifier);
+extern struct type *make_type_with_address_space
+  (struct type *type, type_instance_flags space_identifier);
 
 extern struct type *lookup_memberptr_type (struct type *, struct type *);
 
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index d2ff54a47b..ed31dc0111 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -4397,7 +4397,7 @@  cleanup_undefined_types_noname (struct objfile *objfile)
              and needs to be copied over from the reference type.
              Since replace_type expects them to be identical, we need
              to set these flags manually before hand.  */
-          TYPE_INSTANCE_FLAGS (nat.type) = TYPE_INSTANCE_FLAGS (*type);
+          SET_TYPE_INSTANCE_FLAGS (nat.type, TYPE_INSTANCE_FLAGS (*type));
           replace_type (nat.type, *type);
         }
     }
diff --git a/gdb/type-stack.c b/gdb/type-stack.c
index f8661d7565..608142c849 100644
--- a/gdb/type-stack.c
+++ b/gdb/type-stack.c
@@ -109,7 +109,7 @@  type_stack::follow_types (struct type *follow_type)
   int done = 0;
   int make_const = 0;
   int make_volatile = 0;
-  int make_addr_space = 0;
+  type_instance_flags make_addr_space = 0;
   bool make_restrict = false;
   bool make_atomic = false;
   int array_size;
@@ -128,7 +128,7 @@  type_stack::follow_types (struct type *follow_type)
 	make_volatile = 1;
 	break;
       case tp_space_identifier:
-	make_addr_space = pop_int ();
+	make_addr_space = (enum type_instance_flag_value) pop_int ();
 	break;
       case tp_atomic:
 	make_atomic = true;