[PATCH 0/4] Move dyn prop functions to be methods of struct type

[PATCH 0/4] Move dyn prop functions to be methods of struct type

[Simon Marchi]

I'd like to start cleaning up gdbtypes.h.  In particular, move some functions
and some of the TYPE_* macros to be methods of `struct type`.  To test the
waters, I started with the dynamic property stuff, which is quite isolated.

Simon Marchi (4):
  gdb: make get_dyn_prop a method of struct type
  gdb: make add_dyn_prop a method of struct type
  gdb: make remove_dyn_prop a method of struct type
  gdb: remove TYPE_DYN_PROP_LIST macro

 gdb/ada-lang.c      |  4 ++--
 gdb/ada-typeprint.c |  4 ++--
 gdb/dwarf2/read.c   | 12 +++++-----
 gdb/gdbtypes.c      | 56 ++++++++++++++++++++++-----------------------
 gdb/gdbtypes.h      | 45 ++++++++++++++++--------------------
 gdb/rust-lang.c     |  3 +--
 gdb/value.c         |  2 +-
 7 files changed, 59 insertions(+), 67 deletions(-)

-- 
2.26.2

[PATCH 1/4] gdb: make get_dyn_prop a method of struct type

[Simon Marchi]

Move get_dyn_prop, currently a free function, to be a method on struct
type.

gdb/ChangeLog:

        * gdbtypes.h (struct type) <get_dyn_prop>: New method.
	(get_dyn_prop): Remove.  Update all users to use
	type::get_dyn_prop.
	* gdbtypes.c (get_dyn_prop): Rename to...
	(type::get_dyn_prop): ... this.
---
 gdb/ada-lang.c      |  4 ++--
 gdb/ada-typeprint.c |  4 ++--
 gdb/gdbtypes.c      | 16 ++++++++--------
 gdb/gdbtypes.h      | 23 +++++++++++------------
 gdb/rust-lang.c     |  3 +--
 5 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index bfbc69084ec..6932a544ec6 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2812,7 +2812,7 @@ ada_value_slice_from_ptr (struct value *array_ptr, struct type *type,
     = create_static_range_type (NULL, base_index_type, low, high);
   struct type *slice_type = create_array_type_with_stride
 			      (NULL, TYPE_TARGET_TYPE (type0), index_type,
-			       get_dyn_prop (DYN_PROP_BYTE_STRIDE, type0),
+			       type0->get_dyn_prop (DYN_PROP_BYTE_STRIDE),
 			       TYPE_FIELD_BITSIZE (type0, 0));
   int base_low =  ada_discrete_type_low_bound (TYPE_INDEX_TYPE (type0));
   LONGEST base_low_pos, low_pos;
@@ -2842,7 +2842,7 @@ ada_value_slice (struct value *array, int low, int high)
     = create_static_range_type (NULL, TYPE_INDEX_TYPE (type), low, high);
   struct type *slice_type = create_array_type_with_stride
 			      (NULL, TYPE_TARGET_TYPE (type), index_type,
-			       get_dyn_prop (DYN_PROP_BYTE_STRIDE, type),
+			       type->get_dyn_prop (DYN_PROP_BYTE_STRIDE),
 			       TYPE_FIELD_BITSIZE (type, 0));
   LONGEST low_pos, high_pos;
 
diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index 83972fe125d..9071eeace13 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -776,13 +776,13 @@ print_record_field_types (struct type *type, struct type *outer_type,
 			  struct ui_file *stream, int show, int level,
 			  const struct type_print_options *flags)
 {
-  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS, type);
+  struct dynamic_prop *prop = type->get_dyn_prop (DYN_PROP_VARIANT_PARTS);
   if (prop != nullptr)
     {
       if (prop->kind == PROP_TYPE)
 	{
 	  type = prop->data.original_type;
-	  prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS, type);
+	  prop = type->get_dyn_prop (DYN_PROP_VARIANT_PARTS);
 	}
       gdb_assert (prop->kind == PROP_VARIANT_PARTS);
       print_record_field_types_dynamic (*prop->data.variant_parts,
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 6648dc4d678..9efcfea1ef9 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1938,7 +1938,7 @@ stub_noname_complaint (void)
 static int
 array_type_has_dynamic_stride (struct type *type)
 {
-  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_BYTE_STRIDE, type);
+  struct dynamic_prop *prop = type->get_dyn_prop (DYN_PROP_BYTE_STRIDE);
 
   return (prop != NULL && prop->kind != PROP_CONST);
 }
@@ -1971,7 +1971,7 @@ is_dynamic_type_internal (struct type *type, int top_level)
   if (TYPE_ALLOCATED_PROP (type))
     return 1;
 
-  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS, type);
+  struct dynamic_prop *prop = type->get_dyn_prop (DYN_PROP_VARIANT_PARTS);
   if (prop != nullptr && prop->kind != PROP_TYPE)
     return 1;
 
@@ -2180,7 +2180,7 @@ resolve_dynamic_array_or_string (struct type *type,
   else
     elt_type = TYPE_TARGET_TYPE (type);
 
-  prop = get_dyn_prop (DYN_PROP_BYTE_STRIDE, type);
+  prop = type->get_dyn_prop (DYN_PROP_BYTE_STRIDE);
   if (prop != NULL)
     {
       if (dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
@@ -2417,8 +2417,8 @@ resolve_dynamic_struct (struct type *type,
 
   resolved_type = copy_type (type);
 
-  struct dynamic_prop *variant_prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS,
-						    resolved_type);
+  dynamic_prop *variant_prop
+    = resolved_type->get_dyn_prop (DYN_PROP_VARIANT_PARTS);
   if (variant_prop != nullptr && variant_prop->kind == PROP_VARIANT_PARTS)
     {
       compute_variant_fields (type, resolved_type, addr_stack,
@@ -2633,10 +2633,10 @@ resolve_dynamic_type (struct type *type,
 
 /* See gdbtypes.h  */
 
-struct dynamic_prop *
-get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
+dynamic_prop *
+type::get_dyn_prop (dynamic_prop_node_kind prop_kind) const
 {
-  struct dynamic_prop_list *node = TYPE_DYN_PROP_LIST (type);
+  dynamic_prop_list *node = TYPE_DYN_PROP_LIST (this);
 
   while (node != NULL)
     {
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 87b1bca3a22..a56570726fe 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -349,15 +349,15 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
 
 /* * True if this type is allocatable.  */
 #define TYPE_IS_ALLOCATABLE(t) \
-  (get_dyn_prop (DYN_PROP_ALLOCATED, t) != NULL)
+  ((t)->get_dyn_prop (DYN_PROP_ALLOCATED) != NULL)
 
 /* * True if this type has variant parts.  */
 #define TYPE_HAS_VARIANT_PARTS(t) \
-  (get_dyn_prop (DYN_PROP_VARIANT_PARTS, t) != nullptr)
+  ((t)->get_dyn_prop (DYN_PROP_VARIANT_PARTS) != nullptr)
 
 /* * True if this type has a dynamic length.  */
 #define TYPE_HAS_DYNAMIC_LENGTH(t) \
-  (get_dyn_prop (DYN_PROP_BYTE_SIZE, t) != nullptr)
+  ((t)->get_dyn_prop (DYN_PROP_BYTE_SIZE) != nullptr)
 
 /* * Instruction-space delimited type.  This is for Harvard architectures
    which have separate instruction and data address spaces (and perhaps
@@ -886,6 +886,10 @@ struct main_type
 
 struct type
 {
+  /* * Return the dynamic property of the requested KIND from this type's
+     list of dynamic properties.  */
+  dynamic_prop *get_dyn_prop (dynamic_prop_node_kind kind) const;
+
   /* * Type that is a pointer to this type.
      NULL if no such pointer-to type is known yet.
      The debugger may add the address of such a type
@@ -1445,7 +1449,7 @@ extern bool set_type_align (struct type *, ULONGEST);
 
 /* Property accessors for the type data location.  */
 #define TYPE_DATA_LOCATION(thistype) \
-  get_dyn_prop (DYN_PROP_DATA_LOCATION, thistype)
+  ((thistype)->get_dyn_prop (DYN_PROP_DATA_LOCATION))
 #define TYPE_DATA_LOCATION_BATON(thistype) \
   TYPE_DATA_LOCATION (thistype)->data.baton
 #define TYPE_DATA_LOCATION_ADDR(thistype) \
@@ -1453,13 +1457,13 @@ extern bool set_type_align (struct type *, ULONGEST);
 #define TYPE_DATA_LOCATION_KIND(thistype) \
   TYPE_DATA_LOCATION (thistype)->kind
 #define TYPE_DYNAMIC_LENGTH(thistype) \
-  get_dyn_prop (DYN_PROP_BYTE_SIZE, thistype)
+  ((thistype)->get_dyn_prop (DYN_PROP_BYTE_SIZE))
 
 /* Property accessors for the type allocated/associated.  */
 #define TYPE_ALLOCATED_PROP(thistype) \
-  get_dyn_prop (DYN_PROP_ALLOCATED, thistype)
+  ((thistype)->get_dyn_prop (DYN_PROP_ALLOCATED))
 #define TYPE_ASSOCIATED_PROP(thistype) \
-  get_dyn_prop (DYN_PROP_ASSOCIATED, thistype)
+  ((thistype)->get_dyn_prop (DYN_PROP_ASSOCIATED))
 
 /* Attribute accessors for dynamic properties.  */
 #define TYPE_DYN_PROP_LIST(thistype) \
@@ -2105,11 +2109,6 @@ extern struct type *resolve_dynamic_type
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
-/* * Return the dynamic property of the requested KIND from TYPE's
-   list of dynamic properties.  */
-extern struct dynamic_prop *get_dyn_prop
-  (enum dynamic_prop_node_kind kind, const struct type *type);
-
 /* * Given a dynamic property PROP of a given KIND, add this dynamic
    property to the given TYPE.
 
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 20661e48d96..4233834dff3 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -708,8 +708,7 @@ rust_print_struct_def (struct type *type, const char *varstring,
       if (is_enum)
 	{
 	  fputs_filtered ("enum ", stream);
-	  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_VARIANT_PARTS,
-						    type);
+	  dynamic_prop *prop = type->get_dyn_prop (DYN_PROP_VARIANT_PARTS);
 	  if (prop != nullptr && prop->kind == PROP_TYPE)
 	    type = prop->data.original_type;
 	}
-- 
2.26.2

[PATCH 2/4] gdb: make add_dyn_prop a method of struct type

[Simon Marchi]

Move add_dyn_prop, currently a free function, to be a method of struct
type.

gdb/ChangeLog:

	* gdbtypes.h (struct type) <add_dyn_prop>: New method.
	(add_dyn_prop): Remove.  Update all users to use
	type::add_dyn_prop.
	* gdbtypes.c (add_dyn_prop): Rename to...
	(type::add_dyn_prop): ... this.
---
 gdb/dwarf2/read.c | 12 ++++++------
 gdb/gdbtypes.c    | 13 ++++++-------
 gdb/gdbtypes.h    | 14 ++++++--------
 3 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1813085d0d7..ac208991ff7 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -9218,7 +9218,7 @@ alloc_rust_variant (struct obstack *obstack, struct type *type,
   prop.kind = PROP_VARIANT_PARTS;
   prop.data.variant_parts = prop_value;
 
-  add_dyn_prop (DYN_PROP_VARIANT_PARTS, prop, type);
+  type->add_dyn_prop (DYN_PROP_VARIANT_PARTS, prop);
 }
 
 /* Some versions of rustc emitted enums in an unusual way.
@@ -14706,7 +14706,7 @@ add_variant_property (struct field_info *fip, struct type *type,
     = ((gdb::array_view<variant_part> *)
        obstack_copy (&objfile->objfile_obstack, &parts, sizeof (parts)));
 
-  add_dyn_prop (DYN_PROP_VARIANT_PARTS, prop, type);
+  type->add_dyn_prop (DYN_PROP_VARIANT_PARTS, prop);
 }
 
 /* Create the vector of fields, and attach it to the type.  */
@@ -15355,7 +15355,7 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
 	  struct dynamic_prop prop;
 	  if (attr_to_dynamic_prop (attr, die, cu, &prop,
 				    cu->per_cu->addr_type ()))
-	    add_dyn_prop (DYN_PROP_BYTE_SIZE, prop, type);
+	    type->add_dyn_prop (DYN_PROP_BYTE_SIZE, prop);
           TYPE_LENGTH (type) = 0;
 	}
     }
@@ -23605,7 +23605,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
     {
       struct type *prop_type = cu->per_cu->addr_sized_int_type (false);
       if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))
-        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
+        type->add_dyn_prop (DYN_PROP_ALLOCATED, prop);
     }
   else if (attr != NULL)
     {
@@ -23620,7 +23620,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
     {
       struct type *prop_type = cu->per_cu->addr_sized_int_type (false);
       if (attr_to_dynamic_prop (attr, die, cu, &prop, prop_type))
-        add_dyn_prop (DYN_PROP_ASSOCIATED, prop, type);
+        type->add_dyn_prop (DYN_PROP_ASSOCIATED, prop);
     }
   else if (attr != NULL)
     {
@@ -23633,7 +23633,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop,
 			    cu->per_cu->addr_type ()))
-    add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
+    type->add_dyn_prop (DYN_PROP_DATA_LOCATION, prop);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     dwarf2_per_objfile->die_type_hash
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9efcfea1ef9..3fc14e9cfb9 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1278,7 +1278,7 @@ create_array_type_with_stride (struct type *result_type,
     (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
   TYPE_INDEX_TYPE (result_type) = range_type;
   if (byte_stride_prop != NULL)
-    add_dyn_prop (DYN_PROP_BYTE_STRIDE, *byte_stride_prop, result_type);
+    result_type->add_dyn_prop (DYN_PROP_BYTE_STRIDE, *byte_stride_prop);
   else if (bit_stride > 0)
     TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
 
@@ -2650,20 +2650,19 @@ type::get_dyn_prop (dynamic_prop_node_kind prop_kind) const
 /* See gdbtypes.h  */
 
 void
-add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
-              struct type *type)
+type::add_dyn_prop (dynamic_prop_node_kind prop_kind, dynamic_prop prop)
 {
   struct dynamic_prop_list *temp;
 
-  gdb_assert (TYPE_OBJFILE_OWNED (type));
+  gdb_assert (TYPE_OBJFILE_OWNED (this));
 
-  temp = XOBNEW (&TYPE_OBJFILE (type)->objfile_obstack,
+  temp = XOBNEW (&TYPE_OBJFILE (this)->objfile_obstack,
 		 struct dynamic_prop_list);
   temp->prop_kind = prop_kind;
   temp->prop = prop;
-  temp->next = TYPE_DYN_PROP_LIST (type);
+  temp->next = TYPE_DYN_PROP_LIST (this);
 
-  TYPE_DYN_PROP_LIST (type) = temp;
+  TYPE_DYN_PROP_LIST (this) = temp;
 }
 
 /* Remove dynamic property from TYPE in case it exists.  */
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index a56570726fe..cd03f921d80 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -890,6 +890,12 @@ struct type
      list of dynamic properties.  */
   dynamic_prop *get_dyn_prop (dynamic_prop_node_kind kind) const;
 
+  /* * Given a dynamic property PROP of a given KIND, add this dynamic
+     property to this type.
+
+     This function assumes that this type is objfile-owned.  */
+  void add_dyn_prop (dynamic_prop_node_kind kind, dynamic_prop prop);
+
   /* * Type that is a pointer to this type.
      NULL if no such pointer-to type is known yet.
      The debugger may add the address of such a type
@@ -2109,14 +2115,6 @@ extern struct type *resolve_dynamic_type
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
-/* * Given a dynamic property PROP of a given KIND, add this dynamic
-   property to the given TYPE.
-
-   This function assumes that TYPE is objfile-owned.  */
-extern void add_dyn_prop
-  (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
-   struct type *type);
-
 extern void remove_dyn_prop (enum dynamic_prop_node_kind prop_kind,
                              struct type *type);
 
-- 
2.26.2

[PATCH 3/4] gdb: make remove_dyn_prop a method of struct type

[Simon Marchi]

Move remove_dyn_prop, currently a free function, to be a method of
struct type.

gdb/ChangeLog:

	* gdbtypes.h (struct type) <remove_dyn_prop>: New method.
	(remove_dyn_prop): Remove.  Update all users to use
	type::remove_dyn_prop.
	* gdbtypes.c (remove_dyn_prop): Rename to...
	(type::remove_dyn_prop): ... this.
---
 gdb/gdbtypes.c | 15 +++++++--------
 gdb/gdbtypes.h |  6 +++---
 gdb/value.c    |  2 +-
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 3fc14e9cfb9..566c6c4f063 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2185,7 +2185,7 @@ resolve_dynamic_array_or_string (struct type *type,
     {
       if (dwarf2_evaluate_property (prop, NULL, addr_stack, &value))
 	{
-	  remove_dyn_prop (DYN_PROP_BYTE_STRIDE, type);
+	  type->remove_dyn_prop (DYN_PROP_BYTE_STRIDE);
 	  bit_stride = (unsigned int) (value * 8);
 	}
       else
@@ -2603,7 +2603,7 @@ resolve_dynamic_type_internal (struct type *type,
   if (type_length.has_value ())
     {
       TYPE_LENGTH (resolved_type) = *type_length;
-      remove_dyn_prop (DYN_PROP_BYTE_SIZE, resolved_type);
+      resolved_type->remove_dyn_prop (DYN_PROP_BYTE_SIZE);
     }
 
   /* Resolve data_location attribute.  */
@@ -2665,27 +2665,26 @@ type::add_dyn_prop (dynamic_prop_node_kind prop_kind, dynamic_prop prop)
   TYPE_DYN_PROP_LIST (this) = temp;
 }
 
-/* Remove dynamic property from TYPE in case it exists.  */
+/* See gdbtypes.h.  */
 
 void
-remove_dyn_prop (enum dynamic_prop_node_kind prop_kind,
-                 struct type *type)
+type::remove_dyn_prop (dynamic_prop_node_kind kind)
 {
   struct dynamic_prop_list *prev_node, *curr_node;
 
-  curr_node = TYPE_DYN_PROP_LIST (type);
+  curr_node = TYPE_DYN_PROP_LIST (this);
   prev_node = NULL;
 
   while (NULL != curr_node)
     {
-      if (curr_node->prop_kind == prop_kind)
+      if (curr_node->prop_kind == kind)
 	{
 	  /* Update the linked list but don't free anything.
 	     The property was allocated on objstack and it is not known
 	     if we are on top of it.  Nevertheless, everything is released
 	     when the complete objstack is freed.  */
 	  if (NULL == prev_node)
-	    TYPE_DYN_PROP_LIST (type) = curr_node->next;
+	    TYPE_DYN_PROP_LIST (this) = curr_node->next;
 	  else
 	    prev_node->next = curr_node->next;
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index cd03f921d80..50edcca841c 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -896,6 +896,9 @@ struct type
      This function assumes that this type is objfile-owned.  */
   void add_dyn_prop (dynamic_prop_node_kind kind, dynamic_prop prop);
 
+  /* * Remove dynamic property of kind KIND from this type, if it exists.  */
+  void remove_dyn_prop (dynamic_prop_node_kind kind);
+
   /* * Type that is a pointer to this type.
      NULL if no such pointer-to type is known yet.
      The debugger may add the address of such a type
@@ -2115,9 +2118,6 @@ extern struct type *resolve_dynamic_type
 /* * Predicate if the type has dynamic values, which are not resolved yet.  */
 extern int is_dynamic_type (struct type *type);
 
-extern void remove_dyn_prop (enum dynamic_prop_node_kind prop_kind,
-                             struct type *type);
-
 extern struct type *check_typedef (struct type *);
 
 extern void check_stub_method_group (struct type *, int);
diff --git a/gdb/value.c b/gdb/value.c
index 7ea39af5551..aafbf0fc06b 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2291,7 +2291,7 @@ set_internalvar (struct internalvar *var, struct value *val)
          when accessing the value.
          If we keep it, we would still refer to the origin value.
          Remove the location property in case it exist.  */
-      remove_dyn_prop (DYN_PROP_DATA_LOCATION, value_type (new_data.value));
+      value_type (new_data.value)->remove_dyn_prop (DYN_PROP_DATA_LOCATION);
 
       break;
     }
-- 
2.26.2

[PATCH 4/4] gdb: remove TYPE_DYN_PROP_LIST macro

[Simon Marchi]

Remove this macro, which abstracts how to obtain the dyn_prop_list of a
given type.  We could replace it with a method on `struct type`, but I
don't think it's needed, as the only code that accesses the dynamic prop
list directly is internal gdbtypes.c code (that can be seen as code
internal to `struct type`).  So it can just refer to the field directly.

gdb/ChangeLog:

	* gdbtypes.h (TYPE_DYN_PROP_LIST): Remove.  Update all users
	access thistype->main_type->dyn_prop_list directly.
---
 gdb/gdbtypes.c | 22 +++++++++++-----------
 gdb/gdbtypes.h |  2 --
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 566c6c4f063..a7109338415 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2636,7 +2636,7 @@ resolve_dynamic_type (struct type *type,
 dynamic_prop *
 type::get_dyn_prop (dynamic_prop_node_kind prop_kind) const
 {
-  dynamic_prop_list *node = TYPE_DYN_PROP_LIST (this);
+  dynamic_prop_list *node = this->main_type->dyn_prop_list;
 
   while (node != NULL)
     {
@@ -2660,9 +2660,9 @@ type::add_dyn_prop (dynamic_prop_node_kind prop_kind, dynamic_prop prop)
 		 struct dynamic_prop_list);
   temp->prop_kind = prop_kind;
   temp->prop = prop;
-  temp->next = TYPE_DYN_PROP_LIST (this);
+  temp->next = this->main_type->dyn_prop_list;
 
-  TYPE_DYN_PROP_LIST (this) = temp;
+  this->main_type->dyn_prop_list = temp;
 }
 
 /* See gdbtypes.h.  */
@@ -2672,7 +2672,7 @@ type::remove_dyn_prop (dynamic_prop_node_kind kind)
 {
   struct dynamic_prop_list *prev_node, *curr_node;
 
-  curr_node = TYPE_DYN_PROP_LIST (this);
+  curr_node = this->main_type->dyn_prop_list;
   prev_node = NULL;
 
   while (NULL != curr_node)
@@ -2684,7 +2684,7 @@ type::remove_dyn_prop (dynamic_prop_node_kind kind)
 	     if we are on top of it.  Nevertheless, everything is released
 	     when the complete objstack is freed.  */
 	  if (NULL == prev_node)
-	    TYPE_DYN_PROP_LIST (this) = curr_node->next;
+	    this->main_type->dyn_prop_list = curr_node->next;
 	  else
 	    prev_node->next = curr_node->next;
 
@@ -5347,10 +5347,10 @@ copy_type_recursive (struct objfile *objfile,
       *TYPE_RANGE_DATA (new_type) = *TYPE_RANGE_DATA (type);
     }
 
-  if (TYPE_DYN_PROP_LIST (type) != NULL)
-    TYPE_DYN_PROP_LIST (new_type)
+  if (type->main_type->dyn_prop_list != NULL)
+    new_type->main_type->dyn_prop_list
       = copy_dynamic_prop_list (&objfile->objfile_obstack,
-				TYPE_DYN_PROP_LIST (type));
+				type->main_type->dyn_prop_list);
 
 
   /* Copy pointers to other types.  */
@@ -5415,10 +5415,10 @@ copy_type (const struct type *type)
   TYPE_LENGTH (new_type) = TYPE_LENGTH (type);
   memcpy (TYPE_MAIN_TYPE (new_type), TYPE_MAIN_TYPE (type),
 	  sizeof (struct main_type));
-  if (TYPE_DYN_PROP_LIST (type) != NULL)
-    TYPE_DYN_PROP_LIST (new_type)
+  if (type->main_type->dyn_prop_list != NULL)
+    new_type->main_type->dyn_prop_list
       = copy_dynamic_prop_list (&TYPE_OBJFILE (type) -> objfile_obstack,
-				TYPE_DYN_PROP_LIST (type));
+				type->main_type->dyn_prop_list);
 
   return new_type;
 }
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 50edcca841c..513ee5572d7 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1475,8 +1475,6 @@ extern bool set_type_align (struct type *, ULONGEST);
   ((thistype)->get_dyn_prop (DYN_PROP_ASSOCIATED))
 
 /* Attribute accessors for dynamic properties.  */
-#define TYPE_DYN_PROP_LIST(thistype) \
-  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
 #define TYPE_DYN_PROP_BATON(dynprop) \
   dynprop->data.baton
 #define TYPE_DYN_PROP_ADDR(dynprop) \
-- 
2.26.2

Re: [PATCH 4/4] gdb: remove TYPE_DYN_PROP_LIST macro

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Remove this macro, which abstracts how to obtain the dyn_prop_list of a
Simon> given type.  We could replace it with a method on `struct type`, but I
Simon> don't think it's needed, as the only code that accesses the dynamic prop
Simon> list directly is internal gdbtypes.c code (that can be seen as code
Simon> internal to `struct type`).  So it can just refer to the field directly.

Eventually this member should probably be private.

Tom

Re: [PATCH 0/4] Move dyn prop functions to be methods of struct type

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I'd like to start cleaning up gdbtypes.h.  In particular, move some functions
Simon> and some of the TYPE_* macros to be methods of `struct type`.  To test the
Simon> waters, I started with the dynamic property stuff, which is quite isolated.

Looks good to me.  I support this project in general; I don't think
macros of this form really add value.  There's been a time or two in the
past where we were able to use the macros to change the representation a
little -- but that's even easier with methods.

thanks,
Tom

Re: [PATCH 0/4] Move dyn prop functions to be methods of struct type

[Simon Marchi]

On 2020-05-07 9:59 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I'd like to start cleaning up gdbtypes.h.  In particular, move some functions
> Simon> and some of the TYPE_* macros to be methods of `struct type`.  To test the
> Simon> waters, I started with the dynamic property stuff, which is quite isolated.
> 
> Looks good to me.  I support this project in general; I don't think
> macros of this form really add value.  There's been a time or two in the
> past where we were able to use the macros to change the representation a
> little -- but that's even easier with methods.

Yeah, it might have made sense before C++.

There is one small thing I would change in this patch series.  I have progressed a bit
on that front (changed more macros to methods) and realized I probably don't want to
use the prefix `get_` in the getters.  It makes it long and verbose for nothing.

  type->get_field (0)->get_type ()->get_name ()

vs

  type->field (0)->type ()->name ()

Plus, existing getters on the objfile struct, for example, don't use get, so it would
be more consistent.  So before pushing, I would change patch 1 to rename get_dyn_prop
to just dyn_prop.

Simon

Re: [PATCH 4/4] gdb: remove TYPE_DYN_PROP_LIST macro

[Simon Marchi]

On 2020-05-07 9:58 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Remove this macro, which abstracts how to obtain the dyn_prop_list of a
> Simon> given type.  We could replace it with a method on `struct type`, but I
> Simon> don't think it's needed, as the only code that accesses the dynamic prop
> Simon> list directly is internal gdbtypes.c code (that can be seen as code
> Simon> internal to `struct type`).  So it can just refer to the field directly.
> 
> Eventually this member should probably be private.

Indeed, there are a few things to consider before:

- To make struct type non-POD, we need to re-evaluate how it's allocated/de-allocated
- Re-evaluate how the types are copied.  copy_type and copy_type_recursive are currently
  free functions.  So either they need to work only using the public API of struct type
  and struct main_type, they need to be made "friend" of these classes, or they need to
  be changed to be methods of these classes.

In the mean time, an approach I've taken with my not-submitted-yet patches is to still
rename the field to give it the `m_` prefix, even though it's not really private yet.
That still ensures that no other part of the code is unexpectedly accessing the field
directly.

Simon

Re: [PATCH 0/4] Move dyn prop functions to be methods of struct type

[Simon Marchi]

On 2020-05-07 10:07 a.m., Simon Marchi wrote:
> On 2020-05-07 9:59 a.m., Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Simon> I'd like to start cleaning up gdbtypes.h.  In particular, move some functions
>> Simon> and some of the TYPE_* macros to be methods of `struct type`.  To test the
>> Simon> waters, I started with the dynamic property stuff, which is quite isolated.
>>
>> Looks good to me.  I support this project in general; I don't think
>> macros of this form really add value.  There's been a time or two in the
>> past where we were able to use the macros to change the representation a
>> little -- but that's even easier with methods.
> 
> Yeah, it might have made sense before C++.
> 
> There is one small thing I would change in this patch series.  I have progressed a bit
> on that front (changed more macros to methods) and realized I probably don't want to
> use the prefix `get_` in the getters.  It makes it long and verbose for nothing.
> 
>   type->get_field (0)->get_type ()->get_name ()
> 
> vs
> 
>   type->field (0)->type ()->name ()
> 
> Plus, existing getters on the objfile struct, for example, don't use get, so it would
> be more consistent.  So before pushing, I would change patch 1 to rename get_dyn_prop
> to just dyn_prop.

I pushed the series with that changed.

Simon