Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis Machado <luis.machado@linaro.org>
To: Simon Marchi <simark@simark.ca>, Pedro Alves <pedro@palves.net>,
	gdb-patches@sourceware.org
Subject: Re: [pushed] gdb: remove TYPE_INSTANCE_FLAGS
Date: Tue, 15 Sep 2020 18:28:46 -0300	[thread overview]
Message-ID: <3db19da5-af04-2f7f-52ed-c496c5b3e791@linaro.org> (raw)
In-Reply-To: <c88b11ac-db17-7a92-37b3-076dc7949be1@simark.ca>

I think you missed adjusting the stale references in gdb/gdb-gdb.py.in.

On 9/14/20 11:28 PM, Simon Marchi wrote:
> On 2020-09-14 5:31 p.m., Pedro Alves wrote:
>> A later patch in this series will rewrite 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 an type::instance_flags() method, which takes care
>> of casting appropriately, and adds a separate type::set_instance_flags
>> method, following the pattern of the ongoing TYPE_XXX macro
>> elimination.  This converts uses of TYPE_INSTANCE_FLAGS to
>> type::instance_flags() in the places where the code was already being
>> touched, but there are still many references to the
>> TYPE_INSTANCE_FLAGS macro left behind.  Those could/should be fully
>> replaced at some point.
> 
> Oh, thanks for doing this.  I just pushed this to eliminate TYPE_INSTANCE_FLAGS.
> 
>>From 10242f367fe102a4d55574c930ebfb389dbd233d Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@efficios.com>
> Date: Mon, 14 Sep 2020 22:22:33 -0400
> Subject: [PATCH] gdb: remove TYPE_INSTANCE_FLAGS
> 
> Remove it, use the `type::instance_flags` method everywhere.
> 
> gdb/ChangeLog:
> 
> 	* gdbtypes.h (TYPE_INSTANCE_FLAGS): Remove, replace all uses
> 	with `type::instance_flags`.
> 
> Change-Id: I3653108b712e6186529cb0102e2b70247bbcabbe
> ---
>   gdb/ChangeLog                     |  5 +++++
>   gdb/c-typeprint.c                 |  2 +-
>   gdb/compile/compile-c-types.c     |  6 +++---
>   gdb/compile/compile-cplus-types.c |  6 +++---
>   gdb/gdbtypes.c                    | 21 +++++++++------------
>   gdb/gdbtypes.h                    | 21 ++++++++++-----------
>   gdb/stabsread.c                   |  9 ++++-----
>   7 files changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 0e6dec38b2ae..a68c1ab248a7 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-09-14  Simon Marchi  <simon.marchi@efficios.com>
> +
> +	* gdbtypes.h (TYPE_INSTANCE_FLAGS): Remove, replace all uses
> +	with `type::instance_flags`.
> +
>   2020-09-14  Michael Mullin  <masmullin@gmail.com>
> 
>   	* xml-tdesc.c [!defined(HAVE_LIBEXPAT)] (tdesc_parse_xml):
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index a07b29a95de8..d89c420add6b 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -528,7 +528,7 @@ c_type_print_modifier (struct type *type, struct ui_file *stream,
> 
>     address_space_id
>       = address_space_type_instance_flags_to_name (get_type_arch (type),
> -						 TYPE_INSTANCE_FLAGS (type));
> +						 type->instance_flags ());
>     if (address_space_id)
>       {
>         if (did_print_modifier || need_pre_space)
> diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
> index 6c9135947eac..585f6c89435b 100644
> --- a/gdb/compile/compile-c-types.c
> +++ b/gdb/compile/compile-c-types.c
> @@ -278,9 +278,9 @@ convert_type_basic (compile_c_instance *context, struct type *type)
>   {
>     /* If we are converting a qualified type, first convert the
>        unqualified type and then apply the qualifiers.  */
> -  if ((TYPE_INSTANCE_FLAGS (type) & (TYPE_INSTANCE_FLAG_CONST
> -				     | TYPE_INSTANCE_FLAG_VOLATILE
> -				     | TYPE_INSTANCE_FLAG_RESTRICT)) != 0)
> +  if ((type->instance_flags () & (TYPE_INSTANCE_FLAG_CONST
> +				  | TYPE_INSTANCE_FLAG_VOLATILE
> +				  | TYPE_INSTANCE_FLAG_RESTRICT)) != 0)
>       return convert_qualified (context, type);
> 
>     switch (type->code ())
> diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
> index 2d4352f6c2c4..a0945683e474 100644
> --- a/gdb/compile/compile-cplus-types.c
> +++ b/gdb/compile/compile-cplus-types.c
> @@ -1135,9 +1135,9 @@ convert_type_cplus_basic (compile_cplus_instance *instance,
>   {
>     /* If we are converting a qualified type, first convert the
>        unqualified type and then apply the qualifiers.  */
> -  if ((TYPE_INSTANCE_FLAGS (type) & (TYPE_INSTANCE_FLAG_CONST
> -				     | TYPE_INSTANCE_FLAG_VOLATILE
> -				     | TYPE_INSTANCE_FLAG_RESTRICT)) != 0)
> +  if ((type->instance_flags () & (TYPE_INSTANCE_FLAG_CONST
> +				  | TYPE_INSTANCE_FLAG_VOLATILE
> +				  | TYPE_INSTANCE_FLAG_RESTRICT)) != 0)
>       return compile_cplus_convert_qualified (instance, type);
> 
>     switch (type->code ())
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 7ade2ccb533b..f1f4ec52c7bb 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -630,7 +630,7 @@ make_qualified_type (struct type *type, type_instance_flags new_flags,
>     ntype = type;
>     do
>       {
> -      if (TYPE_INSTANCE_FLAGS (ntype) == new_flags)
> +      if (ntype->instance_flags () == new_flags)
>   	return ntype;
>         ntype = TYPE_CHAIN (ntype);
>       }
> @@ -753,7 +753,7 @@ struct type *
>   make_restrict_type (struct type *type)
>   {
>     return make_qualified_type (type,
> -			      (TYPE_INSTANCE_FLAGS (type)
> +			      (type->instance_flags ()
>   			       | TYPE_INSTANCE_FLAG_RESTRICT),
>   			      NULL);
>   }
> @@ -764,7 +764,7 @@ struct type *
>   make_unqualified_type (struct type *type)
>   {
>     return make_qualified_type (type,
> -			      (TYPE_INSTANCE_FLAGS (type)
> +			      (type->instance_flags ()
>   			       & ~(TYPE_INSTANCE_FLAG_CONST
>   				   | TYPE_INSTANCE_FLAG_VOLATILE
>   				   | TYPE_INSTANCE_FLAG_RESTRICT)),
> @@ -777,7 +777,7 @@ struct type *
>   make_atomic_type (struct type *type)
>   {
>     return make_qualified_type (type,
> -			      (TYPE_INSTANCE_FLAGS (type)
> +			      (type->instance_flags ()
>   			       | TYPE_INSTANCE_FLAG_ATOMIC),
>   			      NULL);
>   }
> @@ -825,7 +825,7 @@ replace_type (struct type *ntype, struct type *type)
> 
>     /* Assert that the two types have equivalent instance qualifiers.
>        This should be true for at least all of our debug readers.  */
> -  gdb_assert (TYPE_INSTANCE_FLAGS (ntype) == TYPE_INSTANCE_FLAGS (type));
> +  gdb_assert (ntype->instance_flags () == type->instance_flags ());
>   }
> 
>   /* Implement direct support for MEMBER_TYPE in GNU C++.
> @@ -2834,9 +2834,7 @@ check_typedef (struct type *type)
>   	     move over any other types NEWTYPE refers to, which could
>   	     be an unbounded amount of stuff.  */
>   	  if (TYPE_OBJFILE (newtype) == TYPE_OBJFILE (type))
> -	    type = make_qualified_type (newtype,
> -					TYPE_INSTANCE_FLAGS (type),
> -					type);
> +	    type = make_qualified_type (newtype, type->instance_flags (), type);
>   	  else
>   	    type = newtype;
>   	}
> @@ -2862,9 +2860,8 @@ check_typedef (struct type *type)
>                with the complete type only if they are in the same
>                objfile.  */
>   	  if (TYPE_OBJFILE (SYMBOL_TYPE (sym)) == TYPE_OBJFILE (type))
> -            type = make_qualified_type (SYMBOL_TYPE (sym),
> -					TYPE_INSTANCE_FLAGS (type),
> -					type);
> +	    type = make_qualified_type (SYMBOL_TYPE (sym),
> +					type->instance_flags (), type);
>   	  else
>   	    type = SYMBOL_TYPE (sym);
>           }
> @@ -4001,7 +3998,7 @@ check_types_equal (struct type *type1, struct type *type2,
>         || type1->has_varargs () != type2->has_varargs ()
>         || type1->is_vector () != type2->is_vector ()
>         || TYPE_NOTTEXT (type1) != TYPE_NOTTEXT (type2)
> -      || TYPE_INSTANCE_FLAGS (type1) != TYPE_INSTANCE_FLAGS (type2)
> +      || type1->instance_flags () != type2->instance_flags ()
>         || type1->num_fields () != type2->num_fields ())
>       return false;
> 
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 6b87de307bde..d28622d46cea 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -213,7 +213,7 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
>   /* * Not textual.  By default, GDB treats all single byte integers as
>      characters (or elements of strings) unless this flag is set.  */
> 
> -#define TYPE_NOTTEXT(t)	(TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_NOTTEXT)
> +#define TYPE_NOTTEXT(t)	(((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_NOTTEXT)
> 
>   /* * Type owner.  If TYPE_OBJFILE_OWNED is true, the type is owned by
>      the objfile retrieved as TYPE_OBJFILE.  Otherwise, the type is
> @@ -240,25 +240,25 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
>   /* * Constant type.  If this is set, the corresponding type has a
>      const modifier.  */
> 
> -#define TYPE_CONST(t) ((TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_CONST) != 0)
> +#define TYPE_CONST(t) ((((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_CONST) != 0)
> 
>   /* * Volatile type.  If this is set, the corresponding type has a
>      volatile modifier.  */
> 
>   #define TYPE_VOLATILE(t) \
> -  ((TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_VOLATILE) != 0)
> +  ((((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_VOLATILE) != 0)
> 
>   /* * Restrict type.  If this is set, the corresponding type has a
>      restrict modifier.  */
> 
>   #define TYPE_RESTRICT(t) \
> -  ((TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_RESTRICT) != 0)
> +  ((((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_RESTRICT) != 0)
> 
>   /* * Atomic type.  If this is set, the corresponding type has an
>      _Atomic modifier.  */
> 
>   #define TYPE_ATOMIC(t) \
> -  ((TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_ATOMIC) != 0)
> +  ((((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_ATOMIC) != 0)
> 
>   /* * True if this type represents either an lvalue or lvalue reference type.  */
> 
> @@ -297,10 +297,10 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
>      is instruction space, and for data objects is data memory.  */
> 
>   #define TYPE_CODE_SPACE(t) \
> -  ((TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_CODE_SPACE) != 0)
> +  ((((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_CODE_SPACE) != 0)
> 
>   #define TYPE_DATA_SPACE(t) \
> -  ((TYPE_INSTANCE_FLAGS (t) & TYPE_INSTANCE_FLAG_DATA_SPACE) != 0)
> +  ((((t)->instance_flags ()) & TYPE_INSTANCE_FLAG_DATA_SPACE) != 0)
> 
>   /* * Address class flags.  Some environments provide for pointers
>      whose size is different from that of a normal pointer or address
> @@ -309,13 +309,13 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
>      target specific ways to represent these different types of address
>      classes.  */
> 
> -#define TYPE_ADDRESS_CLASS_1(t) (TYPE_INSTANCE_FLAGS(t) \
> +#define TYPE_ADDRESS_CLASS_1(t) (((t)->instance_flags ()) \
>                                    & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1)
> -#define TYPE_ADDRESS_CLASS_2(t) (TYPE_INSTANCE_FLAGS(t) \
> +#define TYPE_ADDRESS_CLASS_2(t) (((t)->instance_flags ()) \
>   				 & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_2)
>   #define TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL \
>     (TYPE_INSTANCE_FLAG_ADDRESS_CLASS_1 | TYPE_INSTANCE_FLAG_ADDRESS_CLASS_2)
> -#define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
> +#define TYPE_ADDRESS_CLASS_ALL(t) (((t)->instance_flags ()) \
>   				   & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)
> 
>   /* * Information about a single discriminant.  */
> @@ -1684,7 +1684,6 @@ 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_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
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c
> index 02fc8ccfd2b4..4b1e3b2857aa 100644
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
> @@ -4473,12 +4473,11 @@ cleanup_undefined_types_1 (void)
> 
>   			if (SYMBOL_CLASS (sym) == LOC_TYPEDEF
>   			    && SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN
> -			    && (SYMBOL_TYPE (sym)->code () ==
> -				(*type)->code ())
> -			    && (TYPE_INSTANCE_FLAGS (*type) ==
> -				TYPE_INSTANCE_FLAGS (SYMBOL_TYPE (sym)))
> +			    && (SYMBOL_TYPE (sym)->code () == (*type)->code ())
> +			    && ((*type)->instance_flags ()
> +				== SYMBOL_TYPE (sym)->instance_flags ())
>   			    && strcmp (sym->linkage_name (), type_name) == 0)
> -                          replace_type (*type, SYMBOL_TYPE (sym));
> +			  replace_type (*type, SYMBOL_TYPE (sym));
>   		      }
>   		  }
>   	      }
> 


  reply	other threads:[~2020-09-15 21:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 21:31 [pushed v2 0/4] Rewrite enum_flags, add unit tests, fix problems Pedro Alves
2020-09-14 21:31 ` [pushed v2 1/4] Use type_instance_flags more throughout Pedro Alves
2020-09-15  2:28   ` [pushed] gdb: remove TYPE_INSTANCE_FLAGS Simon Marchi
2020-09-15 21:28     ` Luis Machado [this message]
2020-09-16  2:50       ` Simon Marchi
2020-09-16 11:01         ` Pedro Alves
2020-09-16 20:44           ` Simon Marchi
2020-09-14 21:31 ` [pushed v2 2/4] Rename address_space_int_to_name/address_space_name_to_int Pedro Alves
2020-09-14 21:31 ` [pushed v2 3/4] Rewrite valid-expr.h's internals in terms of the detection idiom (C++17/N4502) Pedro Alves
2020-09-17 10:57   ` Vaseeharan Vinayagamoorthy
2020-09-17 11:41     ` Luis Machado
2020-09-17 16:10     ` Vaseeharan Vinayagamoorthy
2020-09-17 16:23       ` Luis Machado
2020-09-14 21:31 ` [pushed v2 4/4] Rewrite enum_flags, add unit tests, fix problems Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3db19da5-af04-2f7f-52ed-c496c5b3e791@linaro.org \
    --to=luis.machado@linaro.org \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@palves.net \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox