public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c
@ 2017-10-31  1:42 Simon Marchi
  2017-10-31  1:42 ` [PATCH 08/10] Make tdesc_type::name an std::string Simon Marchi
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch series replaces VECs with std::vector and char* with std::string in
the tdesc classes.  The objective is to remove the usage of VECs and the manual
memory management.  The whole series was tested on the buildbot.  At each step,
I tried to re-generate the C target descriptions.  Only one patch causes a
visible change, which is harmless I think.

Simon Marchi (10):
  Make target_desc::properties an std::vector
  Make target_desc::compatible an std::vector
  Make target_desc::features an std::vector
  Make tdesc_feature::name an std::string
  Make tdesc_feature::registers an std::vector
  Make tdesc_reg string fields std::string
  Make tdesc_feature::types an std::vector
  Make tdesc_type::name an std::string
  Make tdesc_type::u::u::fields an std::vector
  Make tdesc_arch_data::arch_regs an std::vector

 gdb/features/arc-arcompact.c |   2 +-
 gdb/features/arc-v2.c        |   4 +-
 gdb/target-descriptions.c    | 704 ++++++++++++++++---------------------------
 3 files changed, 269 insertions(+), 441 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 04/10] Make tdesc_feature::name an std::string
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (2 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 10/10] Make tdesc_arch_data::arch_regs an std::vector Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-10-31  1:42 ` [PATCH 05/10] Make tdesc_feature::registers an std::vector Simon Marchi
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

... so we don't have to manually free it in ~tdesc_feature.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_feature) <name>: Change type to
	std::string.
	<~tdesc_feature>: Don't manually free name.
	<operator==>: Adjust.
	(tdesc_find_feature): Adjust.
	(tdesc_feature_name): Adjust.
	(class print_c_tdesc) <visit_pre>: Adjust.
	(class print_c_feature) <visit_pre>: Adjust.
---
 gdb/target-descriptions.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index eea5115..410575d 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -281,8 +281,8 @@ DEF_VEC_P(tdesc_type_p);
 
 struct tdesc_feature : tdesc_element
 {
-  tdesc_feature (const char *name_)
-    : name (xstrdup (name_))
+  tdesc_feature (const std::string &name_)
+    : name (name_)
   {}
 
   virtual ~tdesc_feature ()
@@ -298,15 +298,13 @@ struct tdesc_feature : tdesc_element
     for (ix = 0; VEC_iterate (tdesc_type_p, types, ix, type); ix++)
       delete type;
     VEC_free (tdesc_type_p, types);
-
-    xfree (name);
   }
 
   DISABLE_COPY_AND_ASSIGN (tdesc_feature);
 
   /* The name of this feature.  It may be recognized by the architecture
      support code.  */
-  char *name;
+  std::string name;
 
   /* The registers associated with this feature.  */
   VEC(tdesc_reg_p) *registers = NULL;
@@ -338,7 +336,7 @@ struct tdesc_feature : tdesc_element
 
   bool operator== (const tdesc_feature &other) const
   {
-    if (strcmp (name, other.name) != 0)
+    if (name != other.name)
       return false;
 
     if (VEC_length (tdesc_reg_p, registers)
@@ -741,7 +739,7 @@ tdesc_find_feature (const struct target_desc *target_desc,
 		    const char *name)
 {
   for (const tdesc_feature_up &feature : target_desc->features)
-    if (strcmp (feature->name, name) == 0)
+    if (feature->name == name)
       return feature.get ();
 
   return NULL;
@@ -752,7 +750,7 @@ tdesc_find_feature (const struct target_desc *target_desc,
 const char *
 tdesc_feature_name (const struct tdesc_feature *feature)
 {
-  return feature->name;
+  return feature->name.c_str ();
 }
 
 /* Predefined types.  */
@@ -1928,7 +1926,7 @@ public:
   void visit_pre (const tdesc_feature *e) override
   {
     printf_unfiltered ("\n  feature = tdesc_create_feature (result, \"%s\");\n",
-		       e->name);
+		       e->name.c_str ());
   }
 
   void visit_post (const tdesc_feature *e) override
@@ -2146,7 +2144,7 @@ public:
 
     printf_unfiltered
       ("\n  feature = tdesc_create_feature (result, \"%s\", \"%s\");\n",
-       e->name, lbasename (m_filename_after_features.c_str ()));
+       e->name.c_str (), lbasename (m_filename_after_features.c_str ()));
   }
 
   void visit_post (const tdesc_feature *e) override
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 07/10] Make tdesc_feature::types an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (4 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 05/10] Make tdesc_feature::registers an std::vector Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-10-31  1:42 ` [PATCH 01/10] Make target_desc::properties " Simon Marchi
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes tdesc_feature::types an std::vector of unique_ptr of
tdesc_type.  This way, we don't need to manually free the objects and
the vector in ~tdesc_feature.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_type_p): Remove typedef.
	(DEF_VEC_P (tdesc_type_p)): Remove.
	(struct tdesc_feature) <types>: Change type to std::vector.
	<~tdesc_feature>: Replace with default implementation.
	<accept>: Adjust.
	(tdesc_named_type): Adjust.
	(tdesc_create_vector): Adjust.
	(tdesc_create_struct): Adjust.
	(tdesc_create_union): Adjust.
	(tdesc_create_flags): Adjust.
	(tdesc_create_enum): Adjust.
---
 gdb/target-descriptions.c | 60 ++++++++++++++++-------------------------------
 1 file changed, 20 insertions(+), 40 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 647d42d..13d6884 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -192,7 +192,7 @@ enum tdesc_type_kind
   TDESC_TYPE_ENUM
 };
 
-typedef struct tdesc_type : tdesc_element
+struct tdesc_type : tdesc_element
 {
   tdesc_type (const char *name_, enum tdesc_type_kind kind_)
     : name (xstrdup (name_)), kind (kind_)
@@ -269,8 +269,9 @@ typedef struct tdesc_type : tdesc_element
   {
     return !(*this == other);
   }
-} *tdesc_type_p;
-DEF_VEC_P(tdesc_type_p);
+};
+
+typedef std::unique_ptr<tdesc_type> tdesc_type_up;
 
 /* A feature from a target description.  Each feature is a collection
    of other elements, e.g. registers and types.  */
@@ -281,15 +282,7 @@ struct tdesc_feature : tdesc_element
     : name (name_)
   {}
 
-  virtual ~tdesc_feature ()
-  {
-    struct tdesc_type *type;
-    int ix;
-
-    for (ix = 0; VEC_iterate (tdesc_type_p, types, ix, type); ix++)
-      delete type;
-    VEC_free (tdesc_type_p, types);
-  }
+  virtual ~tdesc_feature () = default;
 
   DISABLE_COPY_AND_ASSIGN (tdesc_feature);
 
@@ -301,17 +294,13 @@ struct tdesc_feature : tdesc_element
   std::vector<std::unique_ptr<tdesc_reg>> registers;
 
   /* The types associated with this feature.  */
-  VEC(tdesc_type_p) *types = NULL;
+  std::vector<tdesc_type_up> types;
 
   void accept (tdesc_element_visitor &v) const override
   {
     v.visit_pre (this);
 
-    struct tdesc_type *type;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_type_p, types, ix, type);
-	 ix++)
+    for (const tdesc_type_up &type : types)
       type->accept (v);
 
     for (const tdesc_reg_up &reg : registers)
@@ -337,20 +326,15 @@ struct tdesc_feature : tdesc_element
 	  return false;
       }
 
-    if (VEC_length (tdesc_type_p, types)
-	!= VEC_length (tdesc_type_p, other.types))
+    if (types.size () != other.types.size ())
       return false;
 
-    tdesc_type *type;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_type_p, types, ix, type);
-	 ix++)
+    for (int ix = 0; ix < types.size (); ix++)
       {
-	tdesc_type *type2
-	  = VEC_index (tdesc_type_p, other.types, ix);
+	const tdesc_type_up &type1 = types[ix];
+	const tdesc_type_up &type2 = other.types[ix];
 
-	if (type != type2 && *type != *type2)
+	if (type1 != type2 && *type1 != *type2)
 	  return false;
       }
 
@@ -361,7 +345,6 @@ struct tdesc_feature : tdesc_element
   {
     return !(*this == other);
   }
-
 };
 
 typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
@@ -776,16 +759,13 @@ tdesc_predefined_type (enum tdesc_type_kind kind)
 struct tdesc_type *
 tdesc_named_type (const struct tdesc_feature *feature, const char *id)
 {
-  int ix;
-  struct tdesc_type *type;
-
   /* First try target-defined types.  */
-  for (ix = 0; VEC_iterate (tdesc_type_p, feature->types, ix, type); ix++)
+  for (const tdesc_type_up &type : feature->types)
     if (strcmp (type->name, id) == 0)
-      return type;
+      return type.get ();
 
   /* Next try the predefined types.  */
-  for (ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
     if (strcmp (tdesc_predefined_types[ix].name, id) == 0)
       return &tdesc_predefined_types[ix];
 
@@ -1499,7 +1479,7 @@ tdesc_create_vector (struct tdesc_feature *feature, const char *name,
   type->u.v.type = field_type;
   type->u.v.count = count;
 
-  VEC_safe_push (tdesc_type_p, feature->types, type);
+  feature->types.emplace_back (type);
   return type;
 }
 
@@ -1510,7 +1490,7 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *name)
 {
   struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_STRUCT);
 
-  VEC_safe_push (tdesc_type_p, feature->types, type);
+  feature->types.emplace_back (type);
   return type;
 }
 
@@ -1531,7 +1511,7 @@ tdesc_create_union (struct tdesc_feature *feature, const char *name)
 {
   struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_UNION);
 
-  VEC_safe_push (tdesc_type_p, feature->types, type);
+  feature->types.emplace_back (type);
   return type;
 }
 
@@ -1547,7 +1527,7 @@ tdesc_create_flags (struct tdesc_feature *feature, const char *name,
 
   type->u.u.size = size;
 
-  VEC_safe_push (tdesc_type_p, feature->types, type);
+  feature->types.emplace_back (type);
   return type;
 }
 
@@ -1561,7 +1541,7 @@ tdesc_create_enum (struct tdesc_feature *feature, const char *name,
 
   type->u.u.size = size;
 
-  VEC_safe_push (tdesc_type_p, feature->types, type);
+  feature->types.emplace_back (type);
   return type;
 }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/10] Make target_desc::properties an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (5 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 07/10] Make tdesc_feature::types " Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-10-31  1:42 ` [PATCH 06/10] Make tdesc_reg string fields std::string Simon Marchi
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch changes target_desc::properties to be a vector of property
objects.  This way, we don't need to manually free the property members
as well as the property objects themselves.

gdb/ChangeLog:

	* target-descriptions.c (property_s): Remove typedef.
	(DEF_VEC_O (property_s)): Remove.
	(struct target_desc) <properties>: Make an std::vector.
	<~target_desc>: Don't manually free properties.
	(tdesc_property): Adjust.
	(set_tdesc_property): Adjust.
	(class print_c_tdesc) <visit_pre>: Adjust.
---
 gdb/target-descriptions.c | 60 ++++++++++++++++-------------------------------
 1 file changed, 20 insertions(+), 40 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 4f5e9d6..6f8a1c9 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -61,12 +61,15 @@ public:
 
 /* Types.  */
 
-typedef struct property
+struct property
 {
-  char *key;
-  char *value;
-} property_s;
-DEF_VEC_O(property_s);
+  property (const std::string &key_, const std::string &value_)
+  : key (key_), value (value_)
+  {}
+
+  std::string key;
+  std::string value;
+};
 
 /* An individual register from a target description.  */
 
@@ -397,7 +400,6 @@ struct target_desc : tdesc_element
   virtual ~target_desc ()
   {
     struct tdesc_feature *feature;
-    struct property *prop;
     int ix;
 
     for (ix = 0;
@@ -406,15 +408,6 @@ struct target_desc : tdesc_element
       delete feature;
     VEC_free (tdesc_feature_p, features);
 
-    for (ix = 0;
-	 VEC_iterate (property_s, properties, ix, prop);
-	 ix++)
-      {
-	xfree (prop->key);
-	xfree (prop->value);
-      }
-
-    VEC_free (property_s, properties);
     VEC_free (arch_p, compatible);
   }
 
@@ -432,7 +425,7 @@ struct target_desc : tdesc_element
   VEC(arch_p) *compatible = NULL;
 
   /* Any architecture-specific properties specified by the target.  */
-  VEC(property_s) *properties = NULL;
+  std::vector<property> properties;
 
   /* The features associated with this target.  */
   VEC(tdesc_feature_p) *features = NULL;
@@ -726,13 +719,9 @@ tdesc_compatible_p (const struct target_desc *target_desc,
 const char *
 tdesc_property (const struct target_desc *target_desc, const char *key)
 {
-  struct property *prop;
-  int ix;
-
-  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
-       ix++)
-    if (strcmp (prop->key, key) == 0)
-      return prop->value;
+  for (const property &prop : target_desc->properties)
+    if (prop.key == key)
+      return prop.value.c_str ();
 
   return NULL;
 }
@@ -1803,20 +1792,13 @@ void
 set_tdesc_property (struct target_desc *target_desc,
 		    const char *key, const char *value)
 {
-  struct property *prop, new_prop;
-  int ix;
-
   gdb_assert (key != NULL && value != NULL);
 
-  for (ix = 0; VEC_iterate (property_s, target_desc->properties, ix, prop);
-       ix++)
-    if (strcmp (prop->key, key) == 0)
-      internal_error (__FILE__, __LINE__,
-		      _("Attempted to add duplicate property \"%s\""), key);
+  if (tdesc_property (target_desc, key) != NULL)
+    internal_error (__FILE__, __LINE__,
+		    _("Attempted to add duplicate property \"%s\""), key);
 
-  new_prop.key = xstrdup (key);
-  new_prop.value = xstrdup (value);
-  VEC_safe_push (property_s, target_desc->properties, &new_prop);
+  target_desc->properties.emplace_back (key, value);
 }
 
 /* See arch/tdesc.h.  */
@@ -1989,12 +1971,10 @@ public:
     if (ix)
       printf_unfiltered ("\n");
 
-    for (ix = 0; VEC_iterate (property_s, e->properties, ix, prop);
-	 ix++)
-      {
-	printf_unfiltered ("  set_tdesc_property (result, \"%s\", \"%s\");\n",
-			   prop->key, prop->value);
-      }
+    for (const property &prop : e->properties)
+      printf_unfiltered ("  set_tdesc_property (result, \"%s\", \"%s\");\n",
+			 prop.key.c_str (), prop.value.c_str ());
+
     printf_unfiltered ("  struct tdesc_feature *feature;\n");
   }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 08/10] Make tdesc_type::name an std::string
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-10-31  1:42 ` [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector Simon Marchi
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes tdesc_type::name an std::string.  This way, we don't
need to free it manually in ~tdesc_type.  I think the comment on top of
name is not correct, the string is always malloc'ed.

gdb/ChangeLog:

	* target-descriptions.c (struct tdesc_type) <name>: Change type
	to std::string.
	<~tdesc_type>: Don't manually free name.
	<operator==>: Adjust.
	(tdesc_named_type): Adjust.
	(tdesc_find_type): Adjust.
	(tdesc_gdb_type): Adjust.
	(class print_c_tdesc) <visit>: Adjust.
---
 gdb/target-descriptions.c | 51 ++++++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 13d6884..77a9c1c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -194,8 +194,8 @@ enum tdesc_type_kind
 
 struct tdesc_type : tdesc_element
 {
-  tdesc_type (const char *name_, enum tdesc_type_kind kind_)
-    : name (xstrdup (name_)), kind (kind_)
+  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
+    : name (name_), kind (kind_)
   {
     memset (&u, 0, sizeof (u));
   }
@@ -224,15 +224,12 @@ struct tdesc_type : tdesc_element
       default:
 	break;
       }
-    xfree ((char *) name);
   }
 
   DISABLE_COPY_AND_ASSIGN (tdesc_type);
 
-  /* The name of this type.  If this type is a built-in type, this is
-     a pointer to a constant string.  Otherwise, it's a
-     malloc-allocated string (and thus must be freed).  */
-  const char *name;
+  /* The name of this type.   */
+  std::string name;
 
   /* Identify the kind of this type.  */
   enum tdesc_type_kind kind;
@@ -262,7 +259,7 @@ struct tdesc_type : tdesc_element
 
   bool operator== (const tdesc_type &other) const
   {
-    return (streq (name, other.name) && kind == other.kind);
+    return name == other.name && kind == other.kind;
   }
 
   bool operator!= (const tdesc_type &other) const
@@ -761,12 +758,12 @@ tdesc_named_type (const struct tdesc_feature *feature, const char *id)
 {
   /* First try target-defined types.  */
   for (const tdesc_type_up &type : feature->types)
-    if (strcmp (type->name, id) == 0)
+    if (type->name == id)
       return type.get ();
 
   /* Next try the predefined types.  */
   for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
-    if (strcmp (tdesc_predefined_types[ix].name, id) == 0)
+    if (tdesc_predefined_types[ix].name == id)
       return &tdesc_predefined_types[ix];
 
   return NULL;
@@ -789,7 +786,7 @@ tdesc_find_type (struct gdbarch *gdbarch, const char *id)
       if (reg->reg
 	  && reg->reg->tdesc_type
 	  && reg->type
-	  && strcmp (id, reg->reg->tdesc_type->name) == 0)
+	  && reg->reg->tdesc_type->name == id)
 	return reg->type;
     }
 
@@ -850,7 +847,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
       break;
     }
 
-  type = tdesc_find_type (gdbarch, tdesc_type->name);
+  type = tdesc_find_type (gdbarch, tdesc_type->name.c_str ());
   if (type)
     return type;
 
@@ -879,7 +876,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 
 	field_type = tdesc_gdb_type (gdbarch, tdesc_type->u.v.type);
 	type = init_vector_type (field_type, tdesc_type->u.v.count);
-	TYPE_NAME (type) = xstrdup (tdesc_type->name);
+	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
 
 	return type;
       }
@@ -891,7 +888,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 	int ix;
 
 	type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
-	TYPE_NAME (type) = xstrdup (tdesc_type->name);
+	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
 	TYPE_TAG_NAME (type) = TYPE_NAME (type);
 
 	for (ix = 0;
@@ -953,7 +950,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 	int ix;
 
 	type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
-	TYPE_NAME (type) = xstrdup (tdesc_type->name);
+	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
 
 	for (ix = 0;
 	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
@@ -976,7 +973,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 	struct tdesc_type_field *f;
 	int ix;
 
-	type = arch_flags_type (gdbarch, tdesc_type->name,
+	type = arch_flags_type (gdbarch, tdesc_type->name.c_str (),
 				tdesc_type->u.u.size * TARGET_CHAR_BIT);
 	for (ix = 0;
 	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
@@ -1001,7 +998,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 
 	type = arch_type (gdbarch, TYPE_CODE_ENUM,
 			  tdesc_type->u.u.size * TARGET_CHAR_BIT,
-			  tdesc_type->name);
+			  tdesc_type->name.c_str ());
 	TYPE_UNSIGNED (type) = 1;
 	for (ix = 0;
 	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
@@ -1020,7 +1017,7 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 
   internal_error (__FILE__, __LINE__,
 		  "Type \"%s\" has an unknown kind %d",
-		  tdesc_type->name, tdesc_type->kind);
+		  tdesc_type->name.c_str (), tdesc_type->kind);
 }
 \f
 
@@ -1917,10 +1914,10 @@ public:
       case TDESC_TYPE_VECTOR:
 	printf_unfiltered
 	  ("  field_type = tdesc_named_type (feature, \"%s\");\n",
-	   type->u.v.type->name);
+	   type->u.v.type->name.c_str ());
 	printf_unfiltered
 	  ("  tdesc_create_vector (feature, \"%s\", field_type, %d);\n",
-	   type->name, type->u.v.count);
+	   type->name.c_str (), type->u.v.count);
 	break;
       case TDESC_TYPE_STRUCT:
       case TDESC_TYPE_FLAGS:
@@ -1928,7 +1925,7 @@ public:
 	  {
 	    printf_unfiltered
 	      ("  type = tdesc_create_struct (feature, \"%s\");\n",
-	       type->name);
+	       type->name.c_str ());
 	    if (type->u.u.size != 0)
 	      printf_unfiltered
 		("  tdesc_set_struct_size (type, %d);\n",
@@ -1938,7 +1935,7 @@ public:
 	  {
 	    printf_unfiltered
 	      ("  type = tdesc_create_flags (feature, \"%s\", %d);\n",
-	       type->name, type->u.u.size);
+	       type->name.c_str (), type->u.u.size);
 	  }
 	for (int ix3 = 0;
 	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
@@ -1947,7 +1944,7 @@ public:
 	    const char *type_name;
 
 	    gdb_assert (f->type != NULL);
-	    type_name = f->type->name;
+	    type_name = f->type->name.c_str ();
 
 	    /* To minimize changes to generated files, don't emit type
 	       info for fields that have defaulted types.  */
@@ -1999,14 +1996,14 @@ public:
       case TDESC_TYPE_UNION:
 	printf_unfiltered
 	  ("  type = tdesc_create_union (feature, \"%s\");\n",
-	   type->name);
+	   type->name.c_str ());
 	for (int ix3 = 0;
 	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
 	     ix3++)
 	  {
 	    printf_unfiltered
 	      ("  field_type = tdesc_named_type (feature, \"%s\");\n",
-	       f->type->name);
+	       f->type->name.c_str ());
 	    printf_unfiltered
 	      ("  tdesc_add_field (type, \"%s\", field_type);\n",
 	       f->name);
@@ -2015,7 +2012,7 @@ public:
       case TDESC_TYPE_ENUM:
 	printf_unfiltered
 	  ("  type = tdesc_create_enum (feature, \"%s\", %d);\n",
-	   type->name, type->u.u.size);
+	   type->name.c_str (), type->u.u.size);
 	for (int ix3 = 0;
 	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
 	     ix3++)
@@ -2024,7 +2021,7 @@ public:
 	     f->start, f->name);
 	break;
       default:
-	error (_("C output is not supported type \"%s\"."), type->name);
+	error (_("C output is not supported type \"%s\"."), type->name.c_str ());
       }
     printf_unfiltered ("\n");
   }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 10/10] Make tdesc_arch_data::arch_regs an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
  2017-10-31  1:42 ` [PATCH 08/10] Make tdesc_type::name an std::string Simon Marchi
  2017-10-31  1:42 ` [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-10-31  1:42 ` [PATCH 04/10] Make tdesc_feature::name an std::string Simon Marchi
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Make tdesc_arch_data::arch_regs be an std::vector of tdesc_arch_reg
objects.

On particularity is that the tdesc_arch_data linked to a gdbarch is
allocated on the gdbarch's obstack.  To be safe, I did not change it and
called placement-new on the area returned by OBSTACK_ZALLOC.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_arch_reg): Remove typedef.
	(struct tdesc_arch_reg): Add constructor.
	(DEF_VEC_O (tdesc_arch_reg)): Remove.
	(struct tdesc_arch_data): Initialize fields.
	<arch_regs>: Change type to std::vector.
	(target_find_description): Adjust.
	(tdesc_find_type): Adjust.
	(tdesc_data_init): Call tdesc_arch_data constructor.
	(tdesc_data_alloc): Allocate tdesc_arch_data with new.
	(tdesc_data_cleanup): Free data with delete.
	(tdesc_numbered_register): Adjust.
	(tdesc_find_arch_register): Adjust.
	(tdesc_use_registers): Adjust.
---
 gdb/target-descriptions.c | 80 ++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 42 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 8e07a65..139f8af 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -424,12 +424,15 @@ struct target_desc : tdesc_element
    target description may be shared by multiple architectures, but
    this data is private to one gdbarch.  */
 
-typedef struct tdesc_arch_reg
+struct tdesc_arch_reg
 {
+  tdesc_arch_reg (tdesc_reg *reg_, struct type *type_)
+  : reg (reg_), type (type_)
+  {}
+
   struct tdesc_reg *reg;
   struct type *type;
-} tdesc_arch_reg;
-DEF_VEC_O(tdesc_arch_reg);
+};
 
 struct tdesc_arch_data
 {
@@ -439,13 +442,13 @@ struct tdesc_arch_data
      Registers which are NULL in this array, or off the end, are
      treated as zero-sized and nameless (i.e. placeholders in the
      numbering).  */
-  VEC(tdesc_arch_reg) *arch_regs;
+  std::vector<tdesc_arch_reg> arch_regs;
 
   /* Functions which report the register name, type, and reggroups for
      pseudo-registers.  */
-  gdbarch_register_name_ftype *pseudo_register_name;
-  gdbarch_register_type_ftype *pseudo_register_type;
-  gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p;
+  gdbarch_register_name_ftype *pseudo_register_name = NULL;
+  gdbarch_register_type_ftype *pseudo_register_type = NULL;
+  gdbarch_register_reggroup_p_ftype *pseudo_register_reggroup_p = NULL;
 };
 
 /* Info about an inferior's target description.  There's one of these
@@ -586,7 +589,7 @@ target_find_description (void)
 	  data = ((struct tdesc_arch_data *)
 		  gdbarch_data (target_gdbarch (), tdesc_data));
 	  if (tdesc_has_registers (current_target_desc)
-	      && data->arch_regs == NULL)
+	      && data->arch_regs.empty ())
 	    warning (_("Target-supplied registers are not supported "
 		       "by the current architecture"));
 	}
@@ -781,20 +784,16 @@ tdesc_named_type (const struct tdesc_feature *feature, const char *id)
 struct type *
 tdesc_find_type (struct gdbarch *gdbarch, const char *id)
 {
-  struct tdesc_arch_reg *reg;
-  struct tdesc_arch_data *data;
-  int i, num_regs;
+  tdesc_arch_data *data
+    = (struct tdesc_arch_data *) gdbarch_data (gdbarch, tdesc_data);
 
-  data = (struct tdesc_arch_data *) gdbarch_data (gdbarch, tdesc_data);
-  num_regs = VEC_length (tdesc_arch_reg, data->arch_regs);
-  for (i = 0; i < num_regs; i++)
+  for (const tdesc_arch_reg &reg : data->arch_regs)
     {
-      reg = VEC_index (tdesc_arch_reg, data->arch_regs, i);
-      if (reg->reg
-	  && reg->reg->tdesc_type
-	  && reg->type
-	  && reg->reg->tdesc_type->name == id)
-	return reg->type;
+      if (reg.reg
+	  && reg.reg->tdesc_type
+	  && reg.type
+	  && reg.reg->tdesc_type->name == id)
+	return reg.type;
     }
 
   return NULL;
@@ -1022,6 +1021,8 @@ tdesc_data_init (struct obstack *obstack)
   struct tdesc_arch_data *data;
 
   data = OBSTACK_ZALLOC (obstack, struct tdesc_arch_data);
+  new (data) tdesc_arch_data ();
+
   return data;
 }
 
@@ -1031,7 +1032,7 @@ tdesc_data_init (struct obstack *obstack)
 struct tdesc_arch_data *
 tdesc_data_alloc (void)
 {
-  return XCNEW (struct tdesc_arch_data);
+  return new tdesc_arch_data ();
 }
 
 /* Free something allocated by tdesc_data_alloc, if it is not going
@@ -1043,8 +1044,7 @@ tdesc_data_cleanup (void *data_untyped)
 {
   struct tdesc_arch_data *data = (struct tdesc_arch_data *) data_untyped;
 
-  VEC_free (tdesc_arch_reg, data->arch_regs);
-  xfree (data);
+  delete data;
 }
 
 /* Search FEATURE for a register named NAME.  */
@@ -1067,18 +1067,17 @@ tdesc_numbered_register (const struct tdesc_feature *feature,
 			 struct tdesc_arch_data *data,
 			 int regno, const char *name)
 {
-  struct tdesc_arch_reg arch_reg = { 0 };
   struct tdesc_reg *reg = tdesc_find_register_early (feature, name);
 
   if (reg == NULL)
     return 0;
 
   /* Make sure the vector includes a REGNO'th element.  */
-  while (regno >= VEC_length (tdesc_arch_reg, data->arch_regs))
-    VEC_safe_push (tdesc_arch_reg, data->arch_regs, &arch_reg);
+  while (regno >= data->arch_regs.size ())
+    data->arch_regs.emplace_back (nullptr, nullptr);
+
+  data->arch_regs[regno] = tdesc_arch_reg (reg, NULL);
 
-  arch_reg.reg = reg;
-  VEC_replace (tdesc_arch_reg, data->arch_regs, regno, &arch_reg);
   return 1;
 }
 
@@ -1135,8 +1134,8 @@ tdesc_find_arch_register (struct gdbarch *gdbarch, int regno)
   struct tdesc_arch_data *data;
 
   data = (struct tdesc_arch_data *) gdbarch_data (gdbarch, tdesc_data);
-  if (regno < VEC_length (tdesc_arch_reg, data->arch_regs))
-    return VEC_index (tdesc_arch_reg, data->arch_regs, regno);
+  if (regno < data->arch_regs.size ())
+    return &data->arch_regs[regno];
   else
     return NULL;
 }
@@ -1381,7 +1380,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 {
   int num_regs = gdbarch_num_regs (gdbarch);
   struct tdesc_arch_data *data;
-  struct tdesc_arch_reg *arch_reg, new_arch_reg = { 0 };
   htab_t reg_hash;
 
   /* We can't use the description for registers if it doesn't describe
@@ -1392,7 +1390,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 
   data = (struct tdesc_arch_data *) gdbarch_data (gdbarch, tdesc_data);
   data->arch_regs = early_data->arch_regs;
-  xfree (early_data);
+  delete early_data;
 
   /* Build up a set of all registers, so that we can assign register
      numbers where needed.  The hash table expands as necessary, so
@@ -1408,26 +1406,24 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 
   /* Remove any registers which were assigned numbers by the
      architecture.  */
-  for (int ixr = 0;
-       VEC_iterate (tdesc_arch_reg, data->arch_regs, ixr, arch_reg);
-       ixr++)
-    if (arch_reg->reg)
-      htab_remove_elt (reg_hash, arch_reg->reg);
+  for (const tdesc_arch_reg &arch_reg : data->arch_regs)
+    if (arch_reg.reg != NULL)
+      htab_remove_elt (reg_hash, arch_reg.reg);
 
   /* Assign numbers to the remaining registers and add them to the
      list of registers.  The new numbers are always above gdbarch_num_regs.
      Iterate over the features, not the hash table, so that the order
      matches that in the target description.  */
 
-  gdb_assert (VEC_length (tdesc_arch_reg, data->arch_regs) <= num_regs);
-  while (VEC_length (tdesc_arch_reg, data->arch_regs) < num_regs)
-    VEC_safe_push (tdesc_arch_reg, data->arch_regs, &new_arch_reg);
+  gdb_assert (data->arch_regs.size () <= num_regs);
+  while (data->arch_regs.size () < num_regs)
+    data->arch_regs.emplace_back (nullptr, nullptr);
+
   for (const tdesc_feature_up &feature : target_desc->features)
     for (const tdesc_reg_up &reg : feature->registers)
       if (htab_find (reg_hash, reg.get ()) != NULL)
 	{
-	  new_arch_reg.reg = reg.get ();
-	  VEC_safe_push (tdesc_arch_reg, data->arch_regs, &new_arch_reg);
+	  data->arch_regs.emplace_back (reg.get (), nullptr);
 	  num_regs++;
 	}
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 05/10] Make tdesc_feature::registers an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (3 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 04/10] Make tdesc_feature::name an std::string Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-11-02  9:32   ` Yao Qi
  2017-10-31  1:42 ` [PATCH 07/10] Make tdesc_feature::types " Simon Marchi
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes tdesc_feature::registers an std::vector of unique_ptr
to tdesc_reg.  This way, we don't have to manually free the tdesc_reg
objects and the vector in the tdesc_feature destructor.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_reg_p): Remove typedef.
	(DEF_VEC_P (tdesc_reg_p)): Remove.
	(struct tdesc_feature) <registers>: Change type to std::vector.
	<~tdesc_feature>: Don't manually free registers.
	<accept>: Adjust.
	<operator==>: Adjust.
	(tdesc_has_registers): Adjust.
	(tdesc_find_register_early): Adjust.
	(tdesc_use_registers): Adjust.
	(tdesc_create_reg): Adjust.
---
 gdb/target-descriptions.c | 69 +++++++++++++++--------------------------------
 1 file changed, 22 insertions(+), 47 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 410575d..4293996 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -73,7 +73,7 @@ struct property
 
 /* An individual register from a target description.  */
 
-typedef struct tdesc_reg : tdesc_element
+struct tdesc_reg : tdesc_element
 {
   tdesc_reg (struct tdesc_feature *feature, const char *name_,
 	     int regnum, int save_restore_, const char *group_,
@@ -151,8 +151,9 @@ typedef struct tdesc_reg : tdesc_element
   {
     return !(*this == other);
   }
-} *tdesc_reg_p;
-DEF_VEC_P(tdesc_reg_p);
+};
+
+typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
 
 /* A named type from a target description.  */
 
@@ -287,14 +288,9 @@ struct tdesc_feature : tdesc_element
 
   virtual ~tdesc_feature ()
   {
-    struct tdesc_reg *reg;
     struct tdesc_type *type;
     int ix;
 
-    for (ix = 0; VEC_iterate (tdesc_reg_p, registers, ix, reg); ix++)
-      delete reg;
-    VEC_free (tdesc_reg_p, registers);
-
     for (ix = 0; VEC_iterate (tdesc_type_p, types, ix, type); ix++)
       delete type;
     VEC_free (tdesc_type_p, types);
@@ -307,7 +303,7 @@ struct tdesc_feature : tdesc_element
   std::string name;
 
   /* The registers associated with this feature.  */
-  VEC(tdesc_reg_p) *registers = NULL;
+  std::vector<std::unique_ptr<tdesc_reg>> registers;
 
   /* The types associated with this feature.  */
   VEC(tdesc_type_p) *types = NULL;
@@ -323,14 +319,9 @@ struct tdesc_feature : tdesc_element
 	 ix++)
       type->accept (v);
 
-    struct tdesc_reg *reg;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_reg_p, registers, ix, reg);
-	 ix++)
+    for (const tdesc_reg_up &reg : registers)
       reg->accept (v);
 
-
     v.visit_post (this);
   }
 
@@ -339,20 +330,15 @@ struct tdesc_feature : tdesc_element
     if (name != other.name)
       return false;
 
-    if (VEC_length (tdesc_reg_p, registers)
-	!= VEC_length (tdesc_reg_p, other.registers))
+    if (registers.size () != other.registers.size ())
       return false;
 
-    struct tdesc_reg *reg;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_reg_p, registers, ix, reg);
-	 ix++)
+    for (int ix = 0; ix < registers.size (); ix++)
       {
-	tdesc_reg *reg2
-	  = VEC_index (tdesc_reg_p, other.registers, ix);
+	const tdesc_reg_up &reg1 = registers[ix];
+	const tdesc_reg_up &reg2 = other.registers[ix];
 
-	if (reg != reg2 && *reg != *reg2)
+	if (reg1 != reg2 && *reg1 != *reg2)
 	  return false;
       }
 
@@ -725,7 +711,7 @@ tdesc_has_registers (const struct target_desc *target_desc)
     return 0;
 
   for (const tdesc_feature_up &feature : target_desc->features)
-    if (! VEC_empty (tdesc_reg_p, feature->registers))
+    if (!feature->registers.empty ())
       return 1;
 
   return 0;
@@ -1104,14 +1090,9 @@ static struct tdesc_reg *
 tdesc_find_register_early (const struct tdesc_feature *feature,
 			   const char *name)
 {
-  int ixr;
-  struct tdesc_reg *reg;
-
-  for (ixr = 0;
-       VEC_iterate (tdesc_reg_p, feature->registers, ixr, reg);
-       ixr++)
+  for (const tdesc_reg_up &reg : feature->registers)
     if (strcasecmp (reg->name, name) == 0)
-      return reg;
+      return reg.get ();
 
   return NULL;
 }
@@ -1436,8 +1417,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 		     struct tdesc_arch_data *early_data)
 {
   int num_regs = gdbarch_num_regs (gdbarch);
-  int ixr;
-  struct tdesc_reg *reg;
   struct tdesc_arch_data *data;
   struct tdesc_arch_reg *arch_reg, new_arch_reg = { 0 };
   htab_t reg_hash;
@@ -1457,18 +1436,16 @@ tdesc_use_registers (struct gdbarch *gdbarch,
      the initial size is arbitrary.  */
   reg_hash = htab_create (37, htab_hash_pointer, htab_eq_pointer, NULL);
   for (const tdesc_feature_up &feature : target_desc->features)
-    for (ixr = 0;
-	 VEC_iterate (tdesc_reg_p, feature->registers, ixr, reg);
-	 ixr++)
+    for (const tdesc_reg_up &reg : feature->registers)
       {
-	void **slot = htab_find_slot (reg_hash, reg, INSERT);
+	void **slot = htab_find_slot (reg_hash, reg.get (), INSERT);
 
-	*slot = reg;
+	*slot = reg.get ();
       }
 
   /* Remove any registers which were assigned numbers by the
      architecture.  */
-  for (ixr = 0;
+  for (int ixr = 0;
        VEC_iterate (tdesc_arch_reg, data->arch_regs, ixr, arch_reg);
        ixr++)
     if (arch_reg->reg)
@@ -1483,12 +1460,10 @@ tdesc_use_registers (struct gdbarch *gdbarch,
   while (VEC_length (tdesc_arch_reg, data->arch_regs) < num_regs)
     VEC_safe_push (tdesc_arch_reg, data->arch_regs, &new_arch_reg);
   for (const tdesc_feature_up &feature : target_desc->features)
-    for (ixr = 0;
-	 VEC_iterate (tdesc_reg_p, feature->registers, ixr, reg);
-	 ixr++)
-      if (htab_find (reg_hash, reg) != NULL)
+    for (const tdesc_reg_up &reg : feature->registers)
+      if (htab_find (reg_hash, reg.get ()) != NULL)
 	{
-	  new_arch_reg.reg = reg;
+	  new_arch_reg.reg = reg.get ();
 	  VEC_safe_push (tdesc_arch_reg, data->arch_regs, &new_arch_reg);
 	  num_regs++;
 	}
@@ -1515,7 +1490,7 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
   tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
 				  group, bitsize, type);
 
-  VEC_safe_push (tdesc_reg_p, feature->registers, reg);
+  feature->registers.emplace_back (reg);
 }
 
 /* See arch/tdesc.h.  */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 03/10] Make target_desc::features an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (7 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 06/10] Make tdesc_reg string fields std::string Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-11-02  9:29   ` Yao Qi
  2017-10-31  1:43 ` [PATCH 02/10] Make target_desc::compatible " Simon Marchi
  2017-11-02 10:16 ` [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Yao Qi
  10 siblings, 1 reply; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes target_desc to be a vector of unique_ptr to
tdesc_feature objects.  This way, we don't have to manually free the
features and the vector in the target_desc destructor.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_feature_p): Remove typedef.
	(DEF_VEC_P (tdesc_feature_p)): Remove.
	(struct target_desc) <features>: Change type to std::vector.
	<~target_desc>: Replace with default implementation.
	<accept>: Adjust.
	<operator==>: Adjust.
	(tdesc_has_registers): Adjust.
	(tdesc_find_feature): Adjust.
	(tdesc_use_registers): Adjust.
	(tdesc_create_feature): Adjust.
---
 gdb/target-descriptions.c | 72 +++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 52 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 309480c..eea5115 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -279,7 +279,7 @@ DEF_VEC_P(tdesc_type_p);
 /* A feature from a target description.  Each feature is a collection
    of other elements, e.g. registers and types.  */
 
-typedef struct tdesc_feature : tdesc_element
+struct tdesc_feature : tdesc_element
 {
   tdesc_feature (const char *name_)
     : name (xstrdup (name_))
@@ -383,8 +383,9 @@ typedef struct tdesc_feature : tdesc_element
     return !(*this == other);
   }
 
-} *tdesc_feature_p;
-DEF_VEC_P(tdesc_feature_p);
+};
+
+typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
 
 /* A target description.  */
 
@@ -393,17 +394,7 @@ struct target_desc : tdesc_element
   target_desc ()
   {}
 
-  virtual ~target_desc ()
-  {
-    struct tdesc_feature *feature;
-    int ix;
-
-    for (ix = 0;
-	 VEC_iterate (tdesc_feature_p, features, ix, feature);
-	 ix++)
-      delete feature;
-    VEC_free (tdesc_feature_p, features);
-  }
+  virtual ~target_desc () = default;
 
   target_desc (const target_desc &) = delete;
   void operator= (const target_desc &) = delete;
@@ -422,17 +413,13 @@ struct target_desc : tdesc_element
   std::vector<property> properties;
 
   /* The features associated with this target.  */
-  VEC(tdesc_feature_p) *features = NULL;
+  std::vector<std::unique_ptr<tdesc_feature>> features;
 
   void accept (tdesc_element_visitor &v) const override
   {
     v.visit_pre (this);
 
-    struct tdesc_feature *feature;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_feature_p, features, ix, feature);
-	 ix++)
+    for (const tdesc_feature_up &feature : features)
       feature->accept (v);
 
     v.visit_post (this);
@@ -446,20 +433,15 @@ struct target_desc : tdesc_element
     if (osabi != other.osabi)
       return false;
 
-    if (VEC_length (tdesc_feature_p, features)
-	!= VEC_length (tdesc_feature_p, other.features))
+    if (features.size () != other.features.size ())
       return false;
 
-    struct tdesc_feature *feature;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_feature_p, features, ix, feature);
-	 ix++)
+    for (int ix = 0; ix < features.size (); ix++)
       {
-	struct tdesc_feature *feature2
-	  = VEC_index (tdesc_feature_p, other.features, ix);
+	const tdesc_feature_up &feature1 = features[ix];
+	const tdesc_feature_up &feature2 = other.features[ix];
 
-	if (feature != feature2 && *feature != *feature2)
+	if (feature1 != feature2 && *feature1 != *feature2)
 	  return false;
       }
 
@@ -741,15 +723,10 @@ tdesc_osabi (const struct target_desc *target_desc)
 int
 tdesc_has_registers (const struct target_desc *target_desc)
 {
-  int ix;
-  struct tdesc_feature *feature;
-
   if (target_desc == NULL)
     return 0;
 
-  for (ix = 0;
-       VEC_iterate (tdesc_feature_p, target_desc->features, ix, feature);
-       ix++)
+  for (const tdesc_feature_up &feature : target_desc->features)
     if (! VEC_empty (tdesc_reg_p, feature->registers))
       return 1;
 
@@ -763,14 +740,9 @@ const struct tdesc_feature *
 tdesc_find_feature (const struct target_desc *target_desc,
 		    const char *name)
 {
-  int ix;
-  struct tdesc_feature *feature;
-
-  for (ix = 0;
-       VEC_iterate (tdesc_feature_p, target_desc->features, ix, feature);
-       ix++)
+  for (const tdesc_feature_up &feature : target_desc->features)
     if (strcmp (feature->name, name) == 0)
-      return feature;
+      return feature.get ();
 
   return NULL;
 }
@@ -1466,8 +1438,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 		     struct tdesc_arch_data *early_data)
 {
   int num_regs = gdbarch_num_regs (gdbarch);
-  int ixf, ixr;
-  struct tdesc_feature *feature;
+  int ixr;
   struct tdesc_reg *reg;
   struct tdesc_arch_data *data;
   struct tdesc_arch_reg *arch_reg, new_arch_reg = { 0 };
@@ -1487,9 +1458,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
      numbers where needed.  The hash table expands as necessary, so
      the initial size is arbitrary.  */
   reg_hash = htab_create (37, htab_hash_pointer, htab_eq_pointer, NULL);
-  for (ixf = 0;
-       VEC_iterate (tdesc_feature_p, target_desc->features, ixf, feature);
-       ixf++)
+  for (const tdesc_feature_up &feature : target_desc->features)
     for (ixr = 0;
 	 VEC_iterate (tdesc_reg_p, feature->registers, ixr, reg);
 	 ixr++)
@@ -1515,9 +1484,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
   gdb_assert (VEC_length (tdesc_arch_reg, data->arch_regs) <= num_regs);
   while (VEC_length (tdesc_arch_reg, data->arch_regs) < num_regs)
     VEC_safe_push (tdesc_arch_reg, data->arch_regs, &new_arch_reg);
-  for (ixf = 0;
-       VEC_iterate (tdesc_feature_p, target_desc->features, ixf, feature);
-       ixf++)
+  for (const tdesc_feature_up &feature : target_desc->features)
     for (ixr = 0;
 	 VEC_iterate (tdesc_reg_p, feature->registers, ixr, reg);
 	 ixr++)
@@ -1730,7 +1697,8 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
 {
   struct tdesc_feature *new_feature = new tdesc_feature (name);
 
-  VEC_safe_push (tdesc_feature_p, tdesc->features, new_feature);
+  tdesc->features.emplace_back (new_feature);
+
   return new_feature;
 }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 06/10] Make tdesc_reg string fields std::string
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (6 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 01/10] Make target_desc::properties " Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-11-02  9:43   ` Yao Qi
  2017-10-31  1:42 ` [PATCH 03/10] Make target_desc::features an std::vector Simon Marchi
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Make the name, group and type fields of tdesc_reg std::strings.  This
way, we don't have to manually free them in ~tdesc_reg.

Doing so results in a small change in the generated tdesc.  Instead of
passing an empty string for the group parameter of tdesc_create_reg, the
two modified tdesc now pass NULL.  The end result should be the same.

gdb/ChangeLog:

	* target-descriptions.c (struct tdesc_reg) <tdesc_reg>: Change
	type of name_ parameter, adjust to std::string change.
	<name, group, type>: Change type to std::string.
	<~tdesc_reg>: Replace with default implementation.
	<operator==>: Adjust.
	(tdesc_find_register_early): Adjust.
	(tdesc_register_name): Adjust.
	(tdesc_register_type): Adjust.
	(tdesc_register_in_reggroup_p): Adjust.
	(class print_c_tdesc) <visit>: Adjust.
	(class print_c_feature) <visit>: Adjust.
---
 gdb/features/arc-arcompact.c |  2 +-
 gdb/features/arc-v2.c        |  4 +--
 gdb/target-descriptions.c    | 72 +++++++++++++++++++++-----------------------
 3 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/gdb/features/arc-arcompact.c b/gdb/features/arc-arcompact.c
index ea84a40..fd11e31 100644
--- a/gdb/features/arc-arcompact.c
+++ b/gdb/features/arc-arcompact.c
@@ -48,7 +48,7 @@ initialize_tdesc_arc_arcompact (void)
   tdesc_create_reg (feature, "ilink2", 30, 1, NULL, 32, "code_ptr");
   tdesc_create_reg (feature, "blink", 31, 1, NULL, 32, "code_ptr");
   tdesc_create_reg (feature, "lp_count", 32, 1, NULL, 32, "uint32");
-  tdesc_create_reg (feature, "pcl", 33, 1, "", 32, "code_ptr");
+  tdesc_create_reg (feature, "pcl", 33, 1, NULL, 32, "code_ptr");
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
   struct tdesc_type *field_type;
diff --git a/gdb/features/arc-v2.c b/gdb/features/arc-v2.c
index 1eefc24..6eeefdb 100644
--- a/gdb/features/arc-v2.c
+++ b/gdb/features/arc-v2.c
@@ -45,10 +45,10 @@ initialize_tdesc_arc_v2 (void)
   tdesc_create_reg (feature, "fp", 27, 1, NULL, 32, "data_ptr");
   tdesc_create_reg (feature, "sp", 28, 1, NULL, 32, "data_ptr");
   tdesc_create_reg (feature, "ilink", 29, 1, NULL, 32, "code_ptr");
-  tdesc_create_reg (feature, "r30", 30, 1, "", 32, "int");
+  tdesc_create_reg (feature, "r30", 30, 1, NULL, 32, "int");
   tdesc_create_reg (feature, "blink", 31, 1, NULL, 32, "code_ptr");
   tdesc_create_reg (feature, "lp_count", 32, 1, NULL, 32, "uint32");
-  tdesc_create_reg (feature, "pcl", 33, 1, "", 32, "code_ptr");
+  tdesc_create_reg (feature, "pcl", 33, 1, NULL, 32, "code_ptr");
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
   struct tdesc_type *field_type;
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 4293996..647d42d 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -75,33 +75,28 @@ struct property
 
 struct tdesc_reg : tdesc_element
 {
-  tdesc_reg (struct tdesc_feature *feature, const char *name_,
+  tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
 	     int regnum, int save_restore_, const char *group_,
 	     int bitsize_, const char *type_)
-    : name (xstrdup (name_)), target_regnum (regnum),
+    : name (name_), target_regnum (regnum),
       save_restore (save_restore_),
-      group (group_ != NULL ? xstrdup (group_) : NULL),
+      group (group_ != NULL ? group_ : ""),
       bitsize (bitsize_),
-      type (type_ != NULL ? xstrdup (type_) : xstrdup ("<unknown>"))
+      type (type_ != NULL ? type_ : "<unknown>")
   {
     /* If the register's type is target-defined, look it up now.  We may not
        have easy access to the containing feature when we want it later.  */
-    tdesc_type = tdesc_named_type (feature, type);
+    tdesc_type = tdesc_named_type (feature, type.c_str ());
   }
 
-  virtual ~tdesc_reg ()
-  {
-    xfree (name);
-    xfree (type);
-    xfree (group);
-  }
+  virtual ~tdesc_reg () = default;
 
   DISABLE_COPY_AND_ASSIGN (tdesc_reg);
 
   /* The name of this register.  In standard features, it may be
      recognized by the architecture support code, or it may be purely
      for the user.  */
-  char *name;
+  std::string name;
 
   /* The register number used by this target to refer to this
      register.  This is used for remote p/P packets and to determine
@@ -112,14 +107,14 @@ struct tdesc_reg : tdesc_element
      around calls to an inferior function.  */
   int save_restore;
 
-  /* The name of the register group containing this register, or NULL
+  /* The name of the register group containing this register, or empty
      if the group should be automatically determined from the
      register's type.  If this is "general", "float", or "vector", the
      corresponding "info" command should display this register's
      value.  It can be an arbitrary string, but should be limited to
      alphanumeric characters and internal hyphens.  Currently other
-     strings are ignored (treated as NULL).  */
-  char *group;
+     strings are ignored (treated as empty).  */
+  std::string group;
 
   /* The size of the register, in bits.  */
   int bitsize;
@@ -127,7 +122,7 @@ struct tdesc_reg : tdesc_element
   /* The type of the register.  This string corresponds to either
      a named type from the target description or a predefined
      type from GDB.  */
-  char *type;
+  std::string type;
 
   /* The target-described type corresponding to TYPE, if found.  */
   struct tdesc_type *tdesc_type;
@@ -139,12 +134,12 @@ struct tdesc_reg : tdesc_element
 
   bool operator== (const tdesc_reg &other) const
   {
-    return (streq (name, other.name)
+    return (name == other.name
 	    && target_regnum == other.target_regnum
 	    && save_restore == other.save_restore
 	    && bitsize == other.bitsize
-	    && (group == other.group || streq (group, other.group))
-	    && streq (type, other.type));
+	    && group == other.group
+	    && type == other.type);
   }
 
   bool operator!= (const tdesc_reg &other) const
@@ -1091,7 +1086,7 @@ tdesc_find_register_early (const struct tdesc_feature *feature,
 			   const char *name)
 {
   for (const tdesc_reg_up &reg : feature->registers)
-    if (strcasecmp (reg->name, name) == 0)
+    if (strcasecmp (reg->name.c_str (), name) == 0)
       return reg.get ();
 
   return NULL;
@@ -1197,7 +1192,7 @@ tdesc_register_name (struct gdbarch *gdbarch, int regno)
   int num_pseudo_regs = gdbarch_num_pseudo_regs (gdbarch);
 
   if (reg != NULL)
-    return reg->name;
+    return reg->name.c_str ();
 
   if (regno >= num_regs && regno < num_regs + num_pseudo_regs)
     {
@@ -1239,7 +1234,7 @@ tdesc_register_type (struct gdbarch *gdbarch, int regno)
         arch_reg->type = tdesc_gdb_type (gdbarch, reg->tdesc_type);
 
       /* Next try size-sensitive type shortcuts.  */
-      else if (strcmp (reg->type, "float") == 0)
+      else if (reg->type == "float")
 	{
 	  if (reg->bitsize == gdbarch_float_bit (gdbarch))
 	    arch_reg->type = builtin_type (gdbarch)->builtin_float;
@@ -1250,11 +1245,11 @@ tdesc_register_type (struct gdbarch *gdbarch, int regno)
 	  else
 	    {
 	      warning (_("Register \"%s\" has an unsupported size (%d bits)"),
-		       reg->name, reg->bitsize);
+		       reg->name.c_str (), reg->bitsize);
 	      arch_reg->type = builtin_type (gdbarch)->builtin_double;
 	    }
 	}
-      else if (strcmp (reg->type, "int") == 0)
+      else if (reg->type == "int")
 	{
 	  if (reg->bitsize == gdbarch_long_bit (gdbarch))
 	    arch_reg->type = builtin_type (gdbarch)->builtin_long;
@@ -1272,7 +1267,7 @@ tdesc_register_type (struct gdbarch *gdbarch, int regno)
 	  else
 	    {
 	      warning (_("Register \"%s\" has an unsupported size (%d bits)"),
-		       reg->name, reg->bitsize);
+		       reg->name.c_str (), reg->bitsize);
 	      arch_reg->type = builtin_type (gdbarch)->builtin_long;
 	    }
 	}
@@ -1280,7 +1275,7 @@ tdesc_register_type (struct gdbarch *gdbarch, int regno)
       if (arch_reg->type == NULL)
 	internal_error (__FILE__, __LINE__,
 			"Register \"%s\" has an unknown type \"%s\"",
-			reg->name, reg->type);
+			reg->name.c_str (), reg->type.c_str ());
     }
 
   return arch_reg->type;
@@ -1318,15 +1313,15 @@ tdesc_register_in_reggroup_p (struct gdbarch *gdbarch, int regno,
 {
   struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno);
 
-  if (reg != NULL && reg->group != NULL)
+  if (reg != NULL && !reg->group.empty ())
     {
       int general_p = 0, float_p = 0, vector_p = 0;
 
-      if (strcmp (reg->group, "general") == 0)
+      if (reg->group == "general")
 	general_p = 1;
-      else if (strcmp (reg->group, "float") == 0)
+      else if (reg->group == "float")
 	float_p = 1;
-      else if (strcmp (reg->group, "vector") == 0)
+      else if (reg->group == "vector")
 	vector_p = 1;
 
       if (reggroup == float_reggroup)
@@ -2057,12 +2052,13 @@ public:
   void visit (const tdesc_reg *reg) override
   {
     printf_unfiltered ("  tdesc_create_reg (feature, \"%s\", %ld, %d, ",
-		       reg->name, reg->target_regnum, reg->save_restore);
-    if (reg->group)
-      printf_unfiltered ("\"%s\", ", reg->group);
+		       reg->name.c_str (), reg->target_regnum,
+		       reg->save_restore);
+    if (!reg->group.empty ())
+      printf_unfiltered ("\"%s\", ", reg->group.c_str ());
     else
       printf_unfiltered ("NULL, ");
-    printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
+    printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type.c_str ());
   }
 
 protected:
@@ -2173,12 +2169,12 @@ public:
       }
 
     printf_unfiltered ("  tdesc_create_reg (feature, \"%s\", regnum++, %d, ",
-		       reg->name, reg->save_restore);
-    if (reg->group)
-      printf_unfiltered ("\"%s\", ", reg->group);
+		       reg->name.c_str (), reg->save_restore);
+    if (!reg->group.empty ())
+      printf_unfiltered ("\"%s\", ", reg->group.c_str ());
     else
       printf_unfiltered ("NULL, ");
-    printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
+    printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type.c_str ());
 
     m_next_regnum++;
   }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
  2017-10-31  1:42 ` [PATCH 08/10] Make tdesc_type::name an std::string Simon Marchi
@ 2017-10-31  1:42 ` Simon Marchi
  2017-11-02 10:02   ` Yao Qi
  2017-10-31  1:42 ` [PATCH 10/10] Make tdesc_arch_data::arch_regs an std::vector Simon Marchi
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes the tdesc_type::u::u::fields an std::vector of
tdesc_type_field.   The difficulty here is that the vector is part of a
union.  Because of this, I made fields a pointer to a vector, and
instantiate/destroy the vector if the type is one that uses this member
of the union

The field tdesc_type_field::name is changed to an std::string at the
same time.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_type_field): Remove typedef.
	(DEF_VEC_O (tdesc_type_field)): Remove.
	(struct tdesc_type_field): Add constructor.
	<name>: Change type to std::string.
	(struct tdesc_type) <tdesc_type>: Instantiate vector if the type
	kind uses it.
	<~tdesc_type>: Destroy vector if the type kind uses it.
	<u::u::fields>: Change type to std::vector.
	(tdesc_gdb_type): Adjust.
	(tdesc_add_field): Adjust.
	(tdesc_add_typed_bitfield): Adjust.
	(tdesc_add_field): Adjust.
	(tdesc_add_enum_value): Adjust.
	(class print_c_tdesc) <visit>: Adjust.
---
 gdb/target-descriptions.c | 197 ++++++++++++++++++----------------------------
 1 file changed, 78 insertions(+), 119 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 77a9c1c..8e07a65 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -152,16 +152,20 @@ typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
 
 /* A named type from a target description.  */
 
-typedef struct tdesc_type_field
+struct tdesc_type_field
 {
-  char *name;
+  tdesc_type_field (const std::string &name_, tdesc_type *type_,
+		    int start_, int end_)
+  : name (name_), type (type_), start (start_), end (end_)
+  {}
+
+  std::string name;
   struct tdesc_type *type;
   /* For non-enum-values, either both are -1 (non-bitfield), or both are
      not -1 (bitfield).  For enum values, start is the value (which could be
      -1), end is -1.  */
   int start, end;
-} tdesc_type_field;
-DEF_VEC_O(tdesc_type_field);
+};
 
 enum tdesc_type_kind
 {
@@ -198,6 +202,19 @@ struct tdesc_type : tdesc_element
     : name (name_), kind (kind_)
   {
     memset (&u, 0, sizeof (u));
+
+    switch (kind)
+      {
+      case TDESC_TYPE_STRUCT:
+      case TDESC_TYPE_UNION:
+      case TDESC_TYPE_FLAGS:
+      case TDESC_TYPE_ENUM:
+	u.u.fields = new std::vector<tdesc_type_field> ();
+	break;
+
+      default:
+	break;
+      }
   }
 
   virtual ~tdesc_type ()
@@ -208,17 +225,7 @@ struct tdesc_type : tdesc_element
       case TDESC_TYPE_UNION:
       case TDESC_TYPE_FLAGS:
       case TDESC_TYPE_ENUM:
-	{
-	  struct tdesc_type_field *f;
-	  int ix;
-
-	  for (ix = 0;
-	       VEC_iterate (tdesc_type_field, u.u.fields, ix, f);
-	       ix++)
-	    xfree (f->name);
-
-	  VEC_free (tdesc_type_field, u.u.fields);
-	}
+	delete u.u.fields;
 	break;
 
       default:
@@ -247,7 +254,7 @@ struct tdesc_type : tdesc_element
     /* Struct, union, flags, or enum type.  */
     struct
     {
-      VEC(tdesc_type_field) *fields;
+      std::vector<tdesc_type_field> *fields;
       int size;
     } u;
   } u;
@@ -884,18 +891,14 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
     case TDESC_TYPE_STRUCT:
       {
 	struct type *type, *field_type;
-	struct tdesc_type_field *f;
-	int ix;
 
 	type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
 	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
 	TYPE_TAG_NAME (type) = TYPE_NAME (type);
 
-	for (ix = 0;
-	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
-	     ix++)
+	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
 	  {
-	    if (f->start != -1 && f->end != -1)
+	    if (f.start != -1 && f.end != -1)
 	      {
 		/* Bitfield.  */
 		struct field *fld;
@@ -904,15 +907,15 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 
 		/* This invariant should be preserved while creating types.  */
 		gdb_assert (tdesc_type->u.u.size != 0);
-		if (f->type != NULL)
-		  field_type = tdesc_gdb_type (gdbarch, f->type);
+		if (f.type != NULL)
+		  field_type = tdesc_gdb_type (gdbarch, f.type);
 		else if (tdesc_type->u.u.size > 4)
 		  field_type = builtin_type (gdbarch)->builtin_uint64;
 		else
 		  field_type = builtin_type (gdbarch)->builtin_uint32;
 
-		fld = append_composite_type_field_raw (type, xstrdup (f->name),
-						       field_type);
+		fld = append_composite_type_field_raw
+		  (type, xstrdup (f.name.c_str ()), field_type);
 
 		/* For little-endian, BITPOS counts from the LSB of
 		   the structure and marks the LSB of the field.  For
@@ -921,19 +924,19 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 		   way, it is the number of bits to the "left" of the
 		   field.  To calculate this in big-endian, we need
 		   the total size of the structure.  */
-		bitsize = f->end - f->start + 1;
+		bitsize = f.end - f.start + 1;
 		total_size = tdesc_type->u.u.size * TARGET_CHAR_BIT;
 		if (gdbarch_bits_big_endian (gdbarch))
-		  SET_FIELD_BITPOS (fld[0], total_size - f->start - bitsize);
+		  SET_FIELD_BITPOS (fld[0], total_size - f.start - bitsize);
 		else
-		  SET_FIELD_BITPOS (fld[0], f->start);
+		  SET_FIELD_BITPOS (fld[0], f.start);
 		FIELD_BITSIZE (fld[0]) = bitsize;
 	      }
 	    else
 	      {
-		gdb_assert (f->start == -1 && f->end == -1);
-		field_type = tdesc_gdb_type (gdbarch, f->type);
-		append_composite_type_field (type, xstrdup (f->name),
+		gdb_assert (f.start == -1 && f.end == -1);
+		field_type = tdesc_gdb_type (gdbarch, f.type);
+		append_composite_type_field (type, xstrdup (f.name.c_str ()),
 					     field_type);
 	      }
 	  }
@@ -946,18 +949,15 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
     case TDESC_TYPE_UNION:
       {
 	struct type *type, *field_type;
-	struct tdesc_type_field *f;
-	int ix;
 
 	type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
 	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
 
-	for (ix = 0;
-	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
-	     ix++)
+	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
 	  {
-	    field_type = tdesc_gdb_type (gdbarch, f->type);
-	    append_composite_type_field (type, xstrdup (f->name), field_type);
+	    field_type = tdesc_gdb_type (gdbarch, f.type);
+	    append_composite_type_field (type, xstrdup (f.name.c_str ()),
+					 field_type);
 
 	    /* If any of the children of a union are vectors, flag the
 	       union as a vector also.  This allows e.g. a union of two
@@ -970,22 +970,17 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 
     case TDESC_TYPE_FLAGS:
       {
-	struct tdesc_type_field *f;
-	int ix;
-
 	type = arch_flags_type (gdbarch, tdesc_type->name.c_str (),
 				tdesc_type->u.u.size * TARGET_CHAR_BIT);
-	for (ix = 0;
-	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
-	     ix++)
+	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
 	  {
 	    struct type *field_type;
-	    int bitsize = f->end - f->start + 1;
+	    int bitsize = f.end - f.start + 1;
 
-	    gdb_assert (f->type != NULL);
-	    field_type = tdesc_gdb_type (gdbarch, f->type);
-	    append_flags_type_field (type, f->start, bitsize,
-				     field_type, f->name);
+	    gdb_assert (f.type != NULL);
+	    field_type = tdesc_gdb_type (gdbarch, f.type);
+	    append_flags_type_field (type, f.start, bitsize,
+				     field_type, f.name.c_str ());
 	  }
 
 	return type;
@@ -993,22 +988,18 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
 
     case TDESC_TYPE_ENUM:
       {
-	struct tdesc_type_field *f;
-	int ix;
-
 	type = arch_type (gdbarch, TYPE_CODE_ENUM,
 			  tdesc_type->u.u.size * TARGET_CHAR_BIT,
 			  tdesc_type->name.c_str ());
 	TYPE_UNSIGNED (type) = 1;
-	for (ix = 0;
-	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
-	     ix++)
+	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
 	  {
 	    struct field *fld
-	      = append_composite_type_field_raw (type, xstrdup (f->name),
+	      = append_composite_type_field_raw (type,
+						 xstrdup (f.name.c_str ()),
 						 NULL);
 
-	    SET_FIELD_BITPOS (fld[0], f->start);
+	    SET_FIELD_BITPOS (fld[0], f.start);
 	  }
 
 	return type;
@@ -1548,37 +1539,23 @@ void
 tdesc_add_field (struct tdesc_type *type, const char *field_name,
 		 struct tdesc_type *field_type)
 {
-  struct tdesc_type_field f = { 0 };
-
   gdb_assert (type->kind == TDESC_TYPE_UNION
 	      || type->kind == TDESC_TYPE_STRUCT);
 
-  f.name = xstrdup (field_name);
-  f.type = field_type;
-  /* Initialize these values so we know this is not a bit-field
+  /* Initialize start and end so we know this is not a bit-field
      when we print-c-tdesc.  */
-  f.start = -1;
-  f.end = -1;
-
-  VEC_safe_push (tdesc_type_field, type->u.u.fields, &f);
+  type->u.u.fields->emplace_back (field_name, field_type, -1, -1);
 }
 
 void
 tdesc_add_typed_bitfield (struct tdesc_type *type, const char *field_name,
 			  int start, int end, struct tdesc_type *field_type)
 {
-  struct tdesc_type_field f = { 0 };
-
   gdb_assert (type->kind == TDESC_TYPE_STRUCT
 	      || type->kind == TDESC_TYPE_FLAGS);
   gdb_assert (start >= 0 && end >= start);
 
-  f.name = xstrdup (field_name);
-  f.start = start;
-  f.end = end;
-  f.type = field_type;
-
-  VEC_safe_push (tdesc_type_field, type->u.u.fields, &f);
+  type->u.u.fields->emplace_back (field_name, field_type, start, end);
 }
 
 /* See arch/tdesc.h.  */
@@ -1605,33 +1582,23 @@ void
 tdesc_add_flag (struct tdesc_type *type, int start,
 		const char *flag_name)
 {
-  struct tdesc_type_field f = { 0 };
-
   gdb_assert (type->kind == TDESC_TYPE_FLAGS
 	      || type->kind == TDESC_TYPE_STRUCT);
 
-  f.name = xstrdup (flag_name);
-  f.start = start;
-  f.end = start;
-  f.type = tdesc_predefined_type (TDESC_TYPE_BOOL);
-
-  VEC_safe_push (tdesc_type_field, type->u.u.fields, &f);
+  type->u.u.fields->emplace_back (flag_name,
+				   tdesc_predefined_type (TDESC_TYPE_BOOL),
+				   start, start);
 }
 
 void
 tdesc_add_enum_value (struct tdesc_type *type, int value,
 		      const char *name)
 {
-  struct tdesc_type_field f = { 0 };
-
   gdb_assert (type->kind == TDESC_TYPE_ENUM);
 
-  f.name = xstrdup (name);
-  f.start = value;
-  f.end = -1;
-  f.type = tdesc_predefined_type (TDESC_TYPE_INT32);
-
-  VEC_safe_push (tdesc_type_field, type->u.u.fields, &f);
+  type->u.u.fields->emplace_back (name,
+				   tdesc_predefined_type (TDESC_TYPE_INT32),
+				   value, -1);
 }
 
 /* See arch/tdesc.h.  */
@@ -1887,8 +1854,6 @@ public:
 
   void visit (const tdesc_type *type) override
   {
-    struct tdesc_type_field *f;
-
     /* Now we do some "filtering" in order to know which variables to
        declare.  This is needed because otherwise we would declare unused
        variables `field_type' and `type'.  */
@@ -1902,7 +1867,7 @@ public:
 	 || type->kind == TDESC_TYPE_STRUCT
 	 || type->kind == TDESC_TYPE_FLAGS
 	 || type->kind == TDESC_TYPE_ENUM)
-	&& VEC_length (tdesc_type_field, type->u.u.fields) > 0
+	&& !type->u.u.fields->empty ()
 	&& !m_printed_type)
       {
 	printf_unfiltered ("  struct tdesc_type *type;\n");
@@ -1937,35 +1902,33 @@ public:
 	      ("  type = tdesc_create_flags (feature, \"%s\", %d);\n",
 	       type->name.c_str (), type->u.u.size);
 	  }
-	for (int ix3 = 0;
-	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
-	     ix3++)
+	for (const tdesc_type_field &f : *type->u.u.fields)
 	  {
 	    const char *type_name;
 
-	    gdb_assert (f->type != NULL);
-	    type_name = f->type->name.c_str ();
+	    gdb_assert (f.type != NULL);
+	    type_name = f.type->name.c_str ();
 
 	    /* To minimize changes to generated files, don't emit type
 	       info for fields that have defaulted types.  */
-	    if (f->start != -1)
+	    if (f.start != -1)
 	      {
-		gdb_assert (f->end != -1);
-		if (f->type->kind == TDESC_TYPE_BOOL)
+		gdb_assert (f.end != -1);
+		if (f.type->kind == TDESC_TYPE_BOOL)
 		  {
-		    gdb_assert (f->start == f->end);
+		    gdb_assert (f.start == f.end);
 		    printf_unfiltered
 		      ("  tdesc_add_flag (type, %d, \"%s\");\n",
-		       f->start, f->name);
+		       f.start, f.name.c_str ());
 		  }
 		else if ((type->u.u.size == 4
-			  && f->type->kind == TDESC_TYPE_UINT32)
+			  && f.type->kind == TDESC_TYPE_UINT32)
 			 || (type->u.u.size == 8
-			     && f->type->kind == TDESC_TYPE_UINT64))
+			     && f.type->kind == TDESC_TYPE_UINT64))
 		  {
 		    printf_unfiltered
 		      ("  tdesc_add_bitfield (type, \"%s\", %d, %d);\n",
-		       f->name, f->start, f->end);
+		       f.name.c_str (), f.start, f.end);
 		  }
 		else
 		  {
@@ -1976,12 +1939,12 @@ public:
 		    printf_unfiltered
 		      ("  tdesc_add_typed_bitfield (type, \"%s\","
 		       " %d, %d, field_type);\n",
-		       f->name, f->start, f->end);
+		       f.name.c_str (), f.start, f.end);
 		  }
 	      }
 	    else /* Not a bitfield.  */
 	      {
-		gdb_assert (f->end == -1);
+		gdb_assert (f.end == -1);
 		gdb_assert (type->kind == TDESC_TYPE_STRUCT);
 		printf_unfiltered
 		  ("  field_type = tdesc_named_type (feature,"
@@ -1989,7 +1952,7 @@ public:
 		   type_name);
 		printf_unfiltered
 		  ("  tdesc_add_field (type, \"%s\", field_type);\n",
-		   f->name);
+		   f.name.c_str ());
 	      }
 	  }
 	break;
@@ -1997,28 +1960,24 @@ public:
 	printf_unfiltered
 	  ("  type = tdesc_create_union (feature, \"%s\");\n",
 	   type->name.c_str ());
-	for (int ix3 = 0;
-	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
-	     ix3++)
+	for (const tdesc_type_field &f : *type->u.u.fields)
 	  {
 	    printf_unfiltered
 	      ("  field_type = tdesc_named_type (feature, \"%s\");\n",
-	       f->type->name.c_str ());
+	       f.type->name.c_str ());
 	    printf_unfiltered
 	      ("  tdesc_add_field (type, \"%s\", field_type);\n",
-	       f->name);
+	       f.name.c_str ());
 	  }
 	break;
       case TDESC_TYPE_ENUM:
 	printf_unfiltered
 	  ("  type = tdesc_create_enum (feature, \"%s\", %d);\n",
 	   type->name.c_str (), type->u.u.size);
-	for (int ix3 = 0;
-	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
-	     ix3++)
+	for (const tdesc_type_field &f : *type->u.u.fields)
 	  printf_unfiltered
 	    ("  tdesc_add_enum_value (type, %d, \"%s\");\n",
-	     f->start, f->name);
+	     f.start, f.name.c_str ());
 	break;
       default:
 	error (_("C output is not supported type \"%s\"."), type->name.c_str ());
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 02/10] Make target_desc::compatible an std::vector
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (8 preceding siblings ...)
  2017-10-31  1:42 ` [PATCH 03/10] Make target_desc::features an std::vector Simon Marchi
@ 2017-10-31  1:43 ` Simon Marchi
  2017-11-02 10:16 ` [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Yao Qi
  10 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-10-31  1:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch changes target_desc::compatible to be a vector of
bfd_arch_info *.  This way, we don't need to manually free the vector in
the target_desc destructor.

gdb/ChangeLog:

	* target-descriptions.c (arch_p): Remove typedef.
	(DEF_VEC_P (arch_p)): Remove.
	(struct target_desc) <compatible>: Change type to std::vector.
	<~target_desc>: Don't manually free compatible.
	(tdesc_compatible_p): Adjust.
	(tdesc_add_compatible): Adjust.
	(class print_c_tdesc) <visit_pre>: Adjust.
---
 gdb/target-descriptions.c | 39 +++++++++------------------------------
 1 file changed, 9 insertions(+), 30 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 6f8a1c9..309480c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -386,10 +386,6 @@ typedef struct tdesc_feature : tdesc_element
 } *tdesc_feature_p;
 DEF_VEC_P(tdesc_feature_p);
 
-/* A compatible architecture from a target description.  */
-typedef const struct bfd_arch_info *arch_p;
-DEF_VEC_P(arch_p);
-
 /* A target description.  */
 
 struct target_desc : tdesc_element
@@ -407,8 +403,6 @@ struct target_desc : tdesc_element
 	 ix++)
       delete feature;
     VEC_free (tdesc_feature_p, features);
-
-    VEC_free (arch_p, compatible);
   }
 
   target_desc (const target_desc &) = delete;
@@ -422,7 +416,7 @@ struct target_desc : tdesc_element
   enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
 
   /* The list of compatible architectures reported by the target.  */
-  VEC(arch_p) *compatible = NULL;
+  std::vector<const bfd_arch_info *> compatible;
 
   /* Any architecture-specific properties specified by the target.  */
   std::vector<property> properties;
@@ -695,11 +689,7 @@ int
 tdesc_compatible_p (const struct target_desc *target_desc,
 		    const struct bfd_arch_info *arch)
 {
-  const struct bfd_arch_info *compat;
-  int ix;
-
-  for (ix = 0; VEC_iterate (arch_p, target_desc->compatible, ix, compat);
-       ix++)
+  for (const bfd_arch_info *compat : target_desc->compatible)
     {
       if (compat == arch
 	  || arch->compatible (arch, compat)
@@ -1768,24 +1758,20 @@ void
 tdesc_add_compatible (struct target_desc *target_desc,
 		      const struct bfd_arch_info *compatible)
 {
-  const struct bfd_arch_info *compat;
-  int ix;
-
   /* If this instance of GDB is compiled without BFD support for the
      compatible architecture, simply ignore it -- we would not be able
      to handle it anyway.  */
   if (compatible == NULL)
     return;
 
-  for (ix = 0; VEC_iterate (arch_p, target_desc->compatible, ix, compat);
-       ix++)
+  for (const bfd_arch_info *compat : target_desc->compatible)
     if (compat == compatible)
       internal_error (__FILE__, __LINE__,
 		      _("Attempted to add duplicate "
 			"compatible architecture \"%s\""),
 		      compatible->printable_name);
 
-  VEC_safe_push (arch_p, target_desc->compatible, compatible);
+  target_desc->compatible.push_back (compatible);
 }
 
 void
@@ -1956,19 +1942,12 @@ public:
 	printf_unfiltered ("\n");
       }
 
-    int ix;
-    const struct bfd_arch_info *compatible;
-    struct property *prop;
-
-    for (ix = 0; VEC_iterate (arch_p, e->compatible, ix, compatible);
-	 ix++)
-      {
-	printf_unfiltered
-	  ("  tdesc_add_compatible (result, bfd_scan_arch (\"%s\"));\n",
-	   compatible->printable_name);
-      }
+    for (const struct bfd_arch_info *compatible : e->compatible)
+      printf_unfiltered
+	("  tdesc_add_compatible (result, bfd_scan_arch (\"%s\"));\n",
+	 compatible->printable_name);
 
-    if (ix)
+    if (!e->compatible.empty ())
       printf_unfiltered ("\n");
 
     for (const property &prop : e->properties)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/10] Make target_desc::features an std::vector
  2017-10-31  1:42 ` [PATCH 03/10] Make target_desc::features an std::vector Simon Marchi
@ 2017-11-02  9:29   ` Yao Qi
  2017-11-02 13:20     ` Simon Marchi
  0 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2017-11-02  9:29 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Simon Marchi

Simon Marchi <simon.marchi@ericsson.com> writes:

Patch is good to me, two comments below,

> +typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
>  
>  /* A target description.  */
>  
> @@ -393,17 +394,7 @@ struct target_desc : tdesc_element
>    target_desc ()
>    {}
>  
> -  virtual ~target_desc ()
> -  {
> -    struct tdesc_feature *feature;
> -    int ix;
> -
> -    for (ix = 0;
> -	 VEC_iterate (tdesc_feature_p, features, ix, feature);
> -	 ix++)
> -      delete feature;
> -    VEC_free (tdesc_feature_p, features);
> -  }
> +  virtual ~target_desc () = default;
>  

Can't we remove this line and use default (compiler generated)
dtor?

>    target_desc (const target_desc &) = delete;
>    void operator= (const target_desc &) = delete;
> @@ -422,17 +413,13 @@ struct target_desc : tdesc_element
>    std::vector<property> properties;
>  
>    /* The features associated with this target.  */
> -  VEC(tdesc_feature_p) *features = NULL;
> +  std::vector<std::unique_ptr<tdesc_feature>> features;
>  

std::vector<tdesc_feature_up> features;

shorten the code.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/10] Make tdesc_feature::registers an std::vector
  2017-10-31  1:42 ` [PATCH 05/10] Make tdesc_feature::registers an std::vector Simon Marchi
@ 2017-11-02  9:32   ` Yao Qi
  2017-11-02 13:22     ` Simon Marchi
  0 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2017-11-02  9:32 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@ericsson.com> writes:

>    /* The registers associated with this feature.  */
> -  VEC(tdesc_reg_p) *registers = NULL;
> +  std::vector<std::unique_ptr<tdesc_reg>> registers;

Use tdesc_reg_up.

Patch is good to me.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/10] Make tdesc_reg string fields std::string
  2017-10-31  1:42 ` [PATCH 06/10] Make tdesc_reg string fields std::string Simon Marchi
@ 2017-11-02  9:43   ` Yao Qi
  2017-11-02 13:24     ` Simon Marchi
  0 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2017-11-02  9:43 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@ericsson.com> writes:

> Doing so results in a small change in the generated tdesc.  Instead of
> passing an empty string for the group parameter of tdesc_create_reg, the
> two modified tdesc now pass NULL.  The end result should be the same.

We should mention them (arc-*.c) in ChangeLog too.  Patch is good to me.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector
  2017-10-31  1:42 ` [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector Simon Marchi
@ 2017-11-02 10:02   ` Yao Qi
  2017-11-02 13:27     ` Simon Marchi
  0 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2017-11-02 10:02 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Simon Marchi

Simon Marchi <simon.marchi@ericsson.com> writes:

> This patch makes the tdesc_type::u::u::fields an std::vector of
> tdesc_type_field.   The difficulty here is that the vector is part of a
> union.  Because of this, I made fields a pointer to a vector, and
> instantiate/destroy the vector if the type is one that uses this member
> of the union

Hi Simon,
Did you consider remove that union by sub-class tdesc_type.  We can add
to new sub-classes, tdesc_type_with_fields and tdesc_type_vector, for
example?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c
  2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
                   ` (9 preceding siblings ...)
  2017-10-31  1:43 ` [PATCH 02/10] Make target_desc::compatible " Simon Marchi
@ 2017-11-02 10:16 ` Yao Qi
  10 siblings, 0 replies; 23+ messages in thread
From: Yao Qi @ 2017-11-02 10:16 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@ericsson.com> writes:

> This patch series replaces VECs with std::vector and char* with std::string in
> the tdesc classes.  The objective is to remove the usage of VECs and the manual
> memory management.  The whole series was tested on the buildbot.  At each step,
> I tried to re-generate the C target descriptions.  Only one patch causes a
> visible change, which is harmless I think.

I go through all of them.  Patches I don't give comments look good to me.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/10] Make target_desc::features an std::vector
  2017-11-02  9:29   ` Yao Qi
@ 2017-11-02 13:20     ` Simon Marchi
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-11-02 13:20 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Simon Marchi

On 2017-11-02 05:29 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Patch is good to me, two comments below,
> 
>> +typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
>>  
>>  /* A target description.  */
>>  
>> @@ -393,17 +394,7 @@ struct target_desc : tdesc_element
>>    target_desc ()
>>    {}
>>  
>> -  virtual ~target_desc ()
>> -  {
>> -    struct tdesc_feature *feature;
>> -    int ix;
>> -
>> -    for (ix = 0;
>> -	 VEC_iterate (tdesc_feature_p, features, ix, feature);
>> -	 ix++)
>> -      delete feature;
>> -    VEC_free (tdesc_feature_p, features);
>> -  }
>> +  virtual ~target_desc () = default;
>>  
> 
> Can't we remove this line and use default (compiler generated)
> dtor?

If I do, I get the following error:

/home/emaisin/src/binutils-gdb/gdb/target-descriptions.c: In function ‘void free_target_description(void*)’:
/home/emaisin/src/binutils-gdb/gdb/target-descriptions.c:1714:10: error: deleting object of polymorphic class type ‘target_desc’ which has non-virtual destructor might cause undefined behaviour [-Werror=delete-non-virtual-dtor]
Makefile:1929: recipe for target 'target-descriptions.o' failed
   delete target_desc;
          ^
>>    target_desc (const target_desc &) = delete;
>>    void operator= (const target_desc &) = delete;
>> @@ -422,17 +413,13 @@ struct target_desc : tdesc_element
>>    std::vector<property> properties;
>>  
>>    /* The features associated with this target.  */
>> -  VEC(tdesc_feature_p) *features = NULL;
>> +  std::vector<std::unique_ptr<tdesc_feature>> features;
>>  
> 
> std::vector<tdesc_feature_up> features;
> 
> shorten the code.

Yes, I added the typedef later and forgot that instance.

Thanks, I fixed that one locally.

Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/10] Make tdesc_feature::registers an std::vector
  2017-11-02  9:32   ` Yao Qi
@ 2017-11-02 13:22     ` Simon Marchi
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-11-02 13:22 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 2017-11-02 05:32 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>>    /* The registers associated with this feature.  */
>> -  VEC(tdesc_reg_p) *registers = NULL;
>> +  std::vector<std::unique_ptr<tdesc_reg>> registers;
> 
> Use tdesc_reg_up.
> 
> Patch is good to me.

Thanks, fixed.

Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/10] Make tdesc_reg string fields std::string
  2017-11-02  9:43   ` Yao Qi
@ 2017-11-02 13:24     ` Simon Marchi
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-11-02 13:24 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 2017-11-02 05:43 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> Doing so results in a small change in the generated tdesc.  Instead of
>> passing an empty string for the group parameter of tdesc_create_reg, the
>> two modified tdesc now pass NULL.  The end result should be the same.
> 
> We should mention them (arc-*.c) in ChangeLog too.  Patch is good to me.

Oops, you're right, fixed.

Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector
  2017-11-02 10:02   ` Yao Qi
@ 2017-11-02 13:27     ` Simon Marchi
  2017-12-03 16:04       ` [PATCH 11/10] Split tdesc_type into multiple classes Simon Marchi
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Marchi @ 2017-11-02 13:27 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Simon Marchi

On 2017-11-02 06:02 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> This patch makes the tdesc_type::u::u::fields an std::vector of
>> tdesc_type_field.   The difficulty here is that the vector is part of a
>> union.  Because of this, I made fields a pointer to a vector, and
>> instantiate/destroy the vector if the type is one that uses this member
>> of the union
> 
> Hi Simon,
> Did you consider remove that union by sub-class tdesc_type.  We can add
> to new sub-classes, tdesc_type_with_fields and tdesc_type_vector, for
> example?

I tried, but didn't get to something I thought was nice.  It should be the right
way to do it with sub-classes though, I'll try a bit harder :)

Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 11/10] Split tdesc_type into multiple classes
  2017-11-02 13:27     ` Simon Marchi
@ 2017-12-03 16:04       ` Simon Marchi
  2017-12-05 16:46         ` Yao Qi
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Marchi @ 2017-12-03 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Yao Qi, Simon Marchi

Hi Yao,

I revisited this patch, and implemented as best as I could what you
suggested.  This patch applies on top of the current series.

This patch makes tdesc_type an abstract base class and creates three
subclasses:

- tdesc_type_builtin, for builtin types
- tdesc_type_vector, for vector types
- tdesc_type_with_fields, for struct, union, flag and enum types

This allows getting rid of the union in tdesc_type and to not allow the
std::vector separately.  I tried to go further and create separate
classes for struct, union, flag and enum, but it proved too difficult.
One problem is that from the point of the of the target description
code, the types tdesc_type_* are opaque (only forward-declared).
Therefore, it doesn't know about inheritance relationship between those
classes.  This makes it impossible to make functions that accept a
pointer to a base class and pass a pointer to a derived class, for
example.  I think this patch here is a good compromise, and if somebody
wants to improve things further, the door is open.

A make_gdb_type virtual pure method is added to tdesc_type, which
replaces the current tdesc_gdb_type function.  Calling this method on a
tdesc_type returns the corresponding built gdb type.

This patch was regtested on the buildbot.

gdb/ChangeLog:

	* target-descriptions.c (struct tdesc_type): Use default
	destructor.
	<u>: Remove.
	<accept>: Remove.
	(struct tdesc_type_builtin): New.
	(struct tdesc_type_vector): New.
	(struct tdesc_type_with_fields): New.
	(tdesc_predefined_types): Change type to tdesc_type_builtin[].
	(tdesc_gdb_type): Remove.
	(tdesc_register_type): Adjust.
	(tdesc_create_vector): Create tdesc_type_vector.
	(tdesc_create_struct): Create tdesc_type_with_fields.
	(tdesc_set_struct_size): Change parameter type.
	(tdesc_create_union): Create tdesc_type_with_fields.
	(tdesc_create_flags): Likewise.
	(tdesc_create_enum): Likewise.
	(tdesc_add_field): Change parameter type.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_flag): Likewise.
	(tdesc_add_enum_value): Likewise.
	(print_c_tdesc) <visit>: Remove overload with tdesc_type
	parameter, add overloads for tdesc_type_builtin,
	tdesc_type_with_fields and tdesc_type_vector.
	* target-descriptions.h (tdesc_create_enum): Change return type.
	(tdesc_add_typed_bitfield): Change parameter type.
	(tdesc_add_enum_value): Change parameter type.
	* xml-tdesc.c (struct tdesc_parsing_data) <current_type>: Change
	type to tdesc_type_with_fields.
	(tdesc_start_struct): Adjust.
	(tdesc_start_flags): Adjust.
	(tdesc_start_enum): Adjust.
	(tdesc_start_field): Adjust.
	* arch/tdesc.h (struct tdesc_type_builtin): Forward-declare.
	(struct tdesc_type_vector): Forward-declare.
	(struct tdesc_type_with_fields): Forward-declare.
	(tdesc_create_struct): Change return type.
	(tdesc_create_union): Likewise.
	(tdesc_create_flags): Likewise.
	(tdesc_add_field): Change parameter type.
	(tdesc_set_struct_size): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_flag): Likewise.
	* features: Re-generate C files.

gdb/gdbserver/ChangeLog:

	* tdesc.c (struct tdesc_type): Change return type.
	(tdesc_add_flag): Change parameter type.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_set_struct_size): Likewise.
---
 gdb/arch/tdesc.h                                |  25 +-
 gdb/features/aarch64-core.c                     |   2 +-
 gdb/features/aarch64-fpu.c                      |   2 +-
 gdb/features/arc-arcompact.c                    |   2 +-
 gdb/features/arc-v2.c                           |   2 +-
 gdb/features/arm/arm-with-iwmmxt.c              |   2 +-
 gdb/features/i386/32bit-core.c                  |   2 +-
 gdb/features/i386/32bit-mpx.c                   |   2 +-
 gdb/features/i386/32bit-sse.c                   |   2 +-
 gdb/features/i386/64bit-avx512.c                |   2 +-
 gdb/features/i386/64bit-core.c                  |   2 +-
 gdb/features/i386/64bit-mpx.c                   |   2 +-
 gdb/features/i386/64bit-sse.c                   |   2 +-
 gdb/features/i386/x32-core.c                    |   2 +-
 gdb/features/rs6000/powerpc-7400.c              |   2 +-
 gdb/features/rs6000/powerpc-altivec32.c         |   2 +-
 gdb/features/rs6000/powerpc-altivec32l.c        |   2 +-
 gdb/features/rs6000/powerpc-altivec64.c         |   2 +-
 gdb/features/rs6000/powerpc-altivec64l.c        |   2 +-
 gdb/features/rs6000/powerpc-cell32l.c           |   2 +-
 gdb/features/rs6000/powerpc-cell64l.c           |   2 +-
 gdb/features/rs6000/powerpc-isa205-altivec32l.c |   2 +-
 gdb/features/rs6000/powerpc-isa205-altivec64l.c |   2 +-
 gdb/features/rs6000/powerpc-isa205-vsx32l.c     |   2 +-
 gdb/features/rs6000/powerpc-isa205-vsx64l.c     |   2 +-
 gdb/features/rs6000/powerpc-vsx32.c             |   2 +-
 gdb/features/rs6000/powerpc-vsx32l.c            |   2 +-
 gdb/features/rs6000/powerpc-vsx64.c             |   2 +-
 gdb/features/rs6000/powerpc-vsx64l.c            |   2 +-
 gdb/features/s390-gs-linux64.c                  |   2 +-
 gdb/features/s390-tevx-linux64.c                |   2 +-
 gdb/features/s390-vx-linux64.c                  |   2 +-
 gdb/features/s390x-gs-linux64.c                 |   2 +-
 gdb/features/s390x-tevx-linux64.c               |   2 +-
 gdb/features/s390x-vx-linux64.c                 |   2 +-
 gdb/gdbserver/tdesc.c                           |  14 +-
 gdb/target-descriptions.c                       | 694 ++++++++++++------------
 gdb/target-descriptions.h                       |  10 +-
 gdb/xml-tdesc.c                                 |  20 +-
 39 files changed, 429 insertions(+), 402 deletions(-)

diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
index 78bb0fb152..2240df6741 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/tdesc.h
@@ -20,6 +20,9 @@
 
 struct tdesc_feature;
 struct tdesc_type;
+struct tdesc_type_builtin;
+struct tdesc_type_vector;
+struct tdesc_type_with_fields;
 struct tdesc_reg;
 struct target_desc;
 
@@ -51,37 +54,37 @@ struct tdesc_type *tdesc_create_vector (struct tdesc_feature *feature,
 					int count);
 
 /* Return the created struct tdesc_type named NAME in FEATURE.  */
-struct tdesc_type *tdesc_create_struct (struct tdesc_feature *feature,
-					const char *name);
+tdesc_type_with_fields *tdesc_create_struct (struct tdesc_feature *feature,
+					     const char *name);
 
 /* Return the created union tdesc_type named NAME in FEATURE.  */
-struct tdesc_type *tdesc_create_union (struct tdesc_feature *feature,
-				       const char *name);
+tdesc_type_with_fields *tdesc_create_union (struct tdesc_feature *feature,
+					    const char *name);
 
 /* Return the created flags tdesc_type named NAME in FEATURE.  */
-struct tdesc_type *tdesc_create_flags (struct tdesc_feature *feature,
-				       const char *name,
-				       int size);
+tdesc_type_with_fields *tdesc_create_flags (struct tdesc_feature *feature,
+					    const char *name,
+					    int size);
 
 /* Add a new field to TYPE.  FIELD_NAME is its name, and FIELD_TYPE is
    its type.  */
-void tdesc_add_field (struct tdesc_type *type, const char *field_name,
+void tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
 		      struct tdesc_type *field_type);
 
 /* Set the total length of TYPE.  Structs which contain bitfields may
    omit the reserved bits, so the end of the last field may not
    suffice.  */
-void tdesc_set_struct_size (struct tdesc_type *type, int size);
+void tdesc_set_struct_size (tdesc_type_with_fields *type, int size);
 
 /* Add a new untyped bitfield to TYPE.
    Untyped bitfields become either uint32 or uint64 depending on the size
    of the underlying type.  */
-void tdesc_add_bitfield (struct tdesc_type *type, const char *field_name,
+void tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
 			 int start, int end);
 
 /* A flag is just a typed(bool) single-bit bitfield.
    This function is kept to minimize changes in generated files.  */
-void tdesc_add_flag (struct tdesc_type *type, int start,
+void tdesc_add_flag (tdesc_type_with_fields *type, int start,
 		     const char *flag_name);
 
 /* Create a register in feature FEATURE.  */
diff --git a/gdb/features/aarch64-core.c b/gdb/features/aarch64-core.c
index f3fad40133..419539866f 100644
--- a/gdb/features/aarch64-core.c
+++ b/gdb/features/aarch64-core.c
@@ -9,8 +9,8 @@ create_feature_aarch64_core (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core", "aarch64-core.xml");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_flags (feature, "cpsr_flags", 4);
   tdesc_add_flag (type, 0, "SP");
   tdesc_add_flag (type, 1, "");
diff --git a/gdb/features/aarch64-fpu.c b/gdb/features/aarch64-fpu.c
index 3672f2541e..7e15252a61 100644
--- a/gdb/features/aarch64-fpu.c
+++ b/gdb/features/aarch64-fpu.c
@@ -46,7 +46,7 @@ create_feature_aarch64_fpu (struct target_desc *result, long regnum)
   field_type = tdesc_named_type (feature, "int128");
   tdesc_create_vector (feature, "v1i", field_type, 1);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vnd");
   field_type = tdesc_named_type (feature, "v2d");
   tdesc_add_field (type, "f", field_type);
diff --git a/gdb/features/arc-arcompact.c b/gdb/features/arc-arcompact.c
index fd11e31447..cab1cea2a2 100644
--- a/gdb/features/arc-arcompact.c
+++ b/gdb/features/arc-arcompact.c
@@ -51,8 +51,8 @@ initialize_tdesc_arc_arcompact (void)
   tdesc_create_reg (feature, "pcl", 33, 1, NULL, 32, "code_ptr");
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_flags (feature, "status32_type", 4);
   tdesc_add_flag (type, 0, "H");
   tdesc_add_bitfield (type, "E", 1, 2);
diff --git a/gdb/features/arc-v2.c b/gdb/features/arc-v2.c
index 6eeefdb984..8129ee5945 100644
--- a/gdb/features/arc-v2.c
+++ b/gdb/features/arc-v2.c
@@ -51,8 +51,8 @@ initialize_tdesc_arc_v2 (void)
   tdesc_create_reg (feature, "pcl", 33, 1, NULL, 32, "code_ptr");
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_flags (feature, "status32_type", 4);
   tdesc_add_flag (type, 0, "H");
   tdesc_add_bitfield (type, "E", 1, 4);
diff --git a/gdb/features/arm/arm-with-iwmmxt.c b/gdb/features/arm/arm-with-iwmmxt.c
index 5f839a000d..c0066caed1 100644
--- a/gdb/features/arm/arm-with-iwmmxt.c
+++ b/gdb/features/arm/arm-with-iwmmxt.c
@@ -44,7 +44,7 @@ initialize_tdesc_arm_with_iwmmxt (void)
   field_type = tdesc_named_type (feature, "uint32");
   tdesc_create_vector (feature, "iwmmxt_v2u32", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "iwmmxt_vec64i");
   field_type = tdesc_named_type (feature, "iwmmxt_v8u8");
   tdesc_add_field (type, "u8", field_type);
diff --git a/gdb/features/i386/32bit-core.c b/gdb/features/i386/32bit-core.c
index ec903f3218..17db53df67 100644
--- a/gdb/features/i386/32bit-core.c
+++ b/gdb/features/i386/32bit-core.c
@@ -9,8 +9,8 @@ create_feature_i386_32bit_core (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "32bit-core.xml");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type, 0, "CF");
   tdesc_add_flag (type, 1, "");
diff --git a/gdb/features/i386/32bit-mpx.c b/gdb/features/i386/32bit-mpx.c
index 25a3fb145a..c6cf5fb51b 100644
--- a/gdb/features/i386/32bit-mpx.c
+++ b/gdb/features/i386/32bit-mpx.c
@@ -9,8 +9,8 @@ create_feature_i386_32bit_mpx (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx", "32bit-mpx.xml");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_struct (feature, "br128");
   field_type = tdesc_named_type (feature, "uint64");
   tdesc_add_field (type, "lbound", field_type);
diff --git a/gdb/features/i386/32bit-sse.c b/gdb/features/i386/32bit-sse.c
index 01b2058af6..ea7fe83c60 100644
--- a/gdb/features/i386/32bit-sse.c
+++ b/gdb/features/i386/32bit-sse.c
@@ -28,7 +28,7 @@ create_feature_i386_32bit_sse (struct target_desc *result, long regnum)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/i386/64bit-avx512.c b/gdb/features/i386/64bit-avx512.c
index fb50960896..02510de4d5 100644
--- a/gdb/features/i386/64bit-avx512.c
+++ b/gdb/features/i386/64bit-avx512.c
@@ -28,7 +28,7 @@ create_feature_i386_64bit_avx512 (struct target_desc *result, long regnum)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/i386/64bit-core.c b/gdb/features/i386/64bit-core.c
index 14d4a19d67..10f0209c88 100644
--- a/gdb/features/i386/64bit-core.c
+++ b/gdb/features/i386/64bit-core.c
@@ -9,8 +9,8 @@ create_feature_i386_64bit_core (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "64bit-core.xml");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type, 0, "CF");
   tdesc_add_flag (type, 1, "");
diff --git a/gdb/features/i386/64bit-mpx.c b/gdb/features/i386/64bit-mpx.c
index 2751e03221..ff71f8e6aa 100644
--- a/gdb/features/i386/64bit-mpx.c
+++ b/gdb/features/i386/64bit-mpx.c
@@ -9,8 +9,8 @@ create_feature_i386_64bit_mpx (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx", "64bit-mpx.xml");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_struct (feature, "br128");
   field_type = tdesc_named_type (feature, "uint64");
   tdesc_add_field (type, "lbound", field_type);
diff --git a/gdb/features/i386/64bit-sse.c b/gdb/features/i386/64bit-sse.c
index bc384988e7..251a825e34 100644
--- a/gdb/features/i386/64bit-sse.c
+++ b/gdb/features/i386/64bit-sse.c
@@ -28,7 +28,7 @@ create_feature_i386_64bit_sse (struct target_desc *result, long regnum)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/i386/x32-core.c b/gdb/features/i386/x32-core.c
index 3939abc694..ce1bd1a248 100644
--- a/gdb/features/i386/x32-core.c
+++ b/gdb/features/i386/x32-core.c
@@ -9,8 +9,8 @@ create_feature_i386_x32_core (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "x32-core.xml");
+  tdesc_type_with_fields *type;
   struct tdesc_type *field_type;
-  struct tdesc_type *type;
   type = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type, 0, "CF");
   tdesc_add_flag (type, 1, "");
diff --git a/gdb/features/rs6000/powerpc-7400.c b/gdb/features/rs6000/powerpc-7400.c
index 32b6995fd0..f494973dfc 100644
--- a/gdb/features/rs6000/powerpc-7400.c
+++ b/gdb/features/rs6000/powerpc-7400.c
@@ -151,7 +151,7 @@ initialize_tdesc_powerpc_7400 (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-altivec32.c b/gdb/features/rs6000/powerpc-altivec32.c
index e97132ec75..593958d1d8 100644
--- a/gdb/features/rs6000/powerpc-altivec32.c
+++ b/gdb/features/rs6000/powerpc-altivec32.c
@@ -103,7 +103,7 @@ initialize_tdesc_powerpc_altivec32 (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-altivec32l.c b/gdb/features/rs6000/powerpc-altivec32l.c
index a9445d96d0..d713615138 100644
--- a/gdb/features/rs6000/powerpc-altivec32l.c
+++ b/gdb/features/rs6000/powerpc-altivec32l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_altivec32l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-altivec64.c b/gdb/features/rs6000/powerpc-altivec64.c
index 3b626a9441..a7b2229b3a 100644
--- a/gdb/features/rs6000/powerpc-altivec64.c
+++ b/gdb/features/rs6000/powerpc-altivec64.c
@@ -103,7 +103,7 @@ initialize_tdesc_powerpc_altivec64 (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-altivec64l.c b/gdb/features/rs6000/powerpc-altivec64l.c
index cca5353772..50415ee8e6 100644
--- a/gdb/features/rs6000/powerpc-altivec64l.c
+++ b/gdb/features/rs6000/powerpc-altivec64l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_altivec64l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-cell32l.c b/gdb/features/rs6000/powerpc-cell32l.c
index c615b8d8d6..0137e49244 100644
--- a/gdb/features/rs6000/powerpc-cell32l.c
+++ b/gdb/features/rs6000/powerpc-cell32l.c
@@ -109,7 +109,7 @@ initialize_tdesc_powerpc_cell32l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-cell64l.c b/gdb/features/rs6000/powerpc-cell64l.c
index 5040e5e929..f3f7374e17 100644
--- a/gdb/features/rs6000/powerpc-cell64l.c
+++ b/gdb/features/rs6000/powerpc-cell64l.c
@@ -109,7 +109,7 @@ initialize_tdesc_powerpc_cell64l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-isa205-altivec32l.c b/gdb/features/rs6000/powerpc-isa205-altivec32l.c
index 943d02db46..2a8657728f 100644
--- a/gdb/features/rs6000/powerpc-isa205-altivec32l.c
+++ b/gdb/features/rs6000/powerpc-isa205-altivec32l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_isa205_altivec32l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-isa205-altivec64l.c b/gdb/features/rs6000/powerpc-isa205-altivec64l.c
index d454bac1b8..1aad4470c8 100644
--- a/gdb/features/rs6000/powerpc-isa205-altivec64l.c
+++ b/gdb/features/rs6000/powerpc-isa205-altivec64l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_isa205_altivec64l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-isa205-vsx32l.c b/gdb/features/rs6000/powerpc-isa205-vsx32l.c
index 09fd5cee29..76b29a9cf9 100644
--- a/gdb/features/rs6000/powerpc-isa205-vsx32l.c
+++ b/gdb/features/rs6000/powerpc-isa205-vsx32l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_isa205_vsx32l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-isa205-vsx64l.c b/gdb/features/rs6000/powerpc-isa205-vsx64l.c
index d295ab7726..2ce2228501 100644
--- a/gdb/features/rs6000/powerpc-isa205-vsx64l.c
+++ b/gdb/features/rs6000/powerpc-isa205-vsx64l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_isa205_vsx64l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-vsx32.c b/gdb/features/rs6000/powerpc-vsx32.c
index 8cf7562468..c0dadd0c91 100644
--- a/gdb/features/rs6000/powerpc-vsx32.c
+++ b/gdb/features/rs6000/powerpc-vsx32.c
@@ -103,7 +103,7 @@ initialize_tdesc_powerpc_vsx32 (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-vsx32l.c b/gdb/features/rs6000/powerpc-vsx32l.c
index e8c1881553..29fbe62ae4 100644
--- a/gdb/features/rs6000/powerpc-vsx32l.c
+++ b/gdb/features/rs6000/powerpc-vsx32l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_vsx32l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-vsx64.c b/gdb/features/rs6000/powerpc-vsx64.c
index 30953c539c..0295df6ff9 100644
--- a/gdb/features/rs6000/powerpc-vsx64.c
+++ b/gdb/features/rs6000/powerpc-vsx64.c
@@ -103,7 +103,7 @@ initialize_tdesc_powerpc_vsx64 (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/rs6000/powerpc-vsx64l.c b/gdb/features/rs6000/powerpc-vsx64l.c
index 5f12650547..e254ef00dc 100644
--- a/gdb/features/rs6000/powerpc-vsx64l.c
+++ b/gdb/features/rs6000/powerpc-vsx64l.c
@@ -107,7 +107,7 @@ initialize_tdesc_powerpc_vsx64l (void)
   field_type = tdesc_named_type (feature, "int8");
   tdesc_create_vector (feature, "v16i8", field_type, 16);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "uint128");
   tdesc_add_field (type, "uint128", field_type);
diff --git a/gdb/features/s390-gs-linux64.c b/gdb/features/s390-gs-linux64.c
index 39e70436a5..545a333375 100644
--- a/gdb/features/s390-gs-linux64.c
+++ b/gdb/features/s390-gs-linux64.c
@@ -134,7 +134,7 @@ initialize_tdesc_s390_gs_linux64 (void)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/s390-tevx-linux64.c b/gdb/features/s390-tevx-linux64.c
index d9b18d3850..1e376b4438 100644
--- a/gdb/features/s390-tevx-linux64.c
+++ b/gdb/features/s390-tevx-linux64.c
@@ -134,7 +134,7 @@ initialize_tdesc_s390_tevx_linux64 (void)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/s390-vx-linux64.c b/gdb/features/s390-vx-linux64.c
index b2138dd90a..46c6bbe14f 100644
--- a/gdb/features/s390-vx-linux64.c
+++ b/gdb/features/s390-vx-linux64.c
@@ -112,7 +112,7 @@ initialize_tdesc_s390_vx_linux64 (void)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/s390x-gs-linux64.c b/gdb/features/s390x-gs-linux64.c
index 652e6cfe02..f9e31bdec4 100644
--- a/gdb/features/s390x-gs-linux64.c
+++ b/gdb/features/s390x-gs-linux64.c
@@ -118,7 +118,7 @@ initialize_tdesc_s390x_gs_linux64 (void)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/s390x-tevx-linux64.c b/gdb/features/s390x-tevx-linux64.c
index 02afd3a5c2..31952d1f91 100644
--- a/gdb/features/s390x-tevx-linux64.c
+++ b/gdb/features/s390x-tevx-linux64.c
@@ -118,7 +118,7 @@ initialize_tdesc_s390x_tevx_linux64 (void)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/features/s390x-vx-linux64.c b/gdb/features/s390x-vx-linux64.c
index 120db53a33..a4ffd627c2 100644
--- a/gdb/features/s390x-vx-linux64.c
+++ b/gdb/features/s390x-vx-linux64.c
@@ -96,7 +96,7 @@ initialize_tdesc_s390x_vx_linux64 (void)
   field_type = tdesc_named_type (feature, "int64");
   tdesc_create_vector (feature, "v2i64", field_type, 2);
 
-  struct tdesc_type *type;
+  tdesc_type_with_fields *type;
   type = tdesc_create_union (feature, "vec128");
   field_type = tdesc_named_type (feature, "v4f");
   tdesc_add_field (type, "v4_float", field_type);
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e2c4288efb..5ea234145f 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -146,7 +146,7 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
 
 /* See arch/tdesc.h.  */
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_flags (struct tdesc_feature *feature, const char *name,
 		    int size)
 {
@@ -156,7 +156,7 @@ tdesc_create_flags (struct tdesc_feature *feature, const char *name,
 /* See arch/tdesc.h.  */
 
 void
-tdesc_add_flag (struct tdesc_type *type, int start,
+tdesc_add_flag (tdesc_type_with_fields *type, int start,
 		const char *flag_name)
 {}
 
@@ -170,7 +170,7 @@ tdesc_named_type (const struct tdesc_feature *feature, const char *id)
 
 /* See arch/tdesc.h.  */
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_union (struct tdesc_feature *feature, const char *id)
 {
   return NULL;
@@ -178,7 +178,7 @@ tdesc_create_union (struct tdesc_feature *feature, const char *id)
 
 /* See arch/tdesc.h.  */
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_struct (struct tdesc_feature *feature, const char *id)
 {
   return NULL;
@@ -222,20 +222,20 @@ tdesc_create_vector (struct tdesc_feature *feature, const char *name,
 }
 
 void
-tdesc_add_bitfield (struct tdesc_type *type, const char *field_name,
+tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
 		    int start, int end)
 {}
 
 /* See arch/tdesc.h.  */
 
 void
-tdesc_add_field (struct tdesc_type *type, const char *field_name,
+tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
 		 struct tdesc_type *field_type)
 {}
 
 /* See arch/tdesc.h.  */
 
 void
-tdesc_set_struct_size (struct tdesc_type *type, int size)
+tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
 {
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index fdb45f374f..5c48296323 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -49,7 +49,10 @@ public:
   virtual void visit_pre (const tdesc_feature *e) = 0;
   virtual void visit_post (const tdesc_feature *e) = 0;
 
-  virtual void visit (const tdesc_type *e) = 0;
+  virtual void visit (const tdesc_type_builtin *e) = 0;
+  virtual void visit (const tdesc_type_vector *e) = 0;
+  virtual void visit (const tdesc_type_with_fields *e) = 0;
+
   virtual void visit (const tdesc_reg *e) = 0;
 };
 
@@ -200,82 +203,313 @@ struct tdesc_type : tdesc_element
 {
   tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
     : name (name_), kind (kind_)
+  {}
+
+  virtual ~tdesc_type () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_type);
+
+  /* The name of this type.   */
+  std::string name;
+
+  /* Identify the kind of this type.  */
+  enum tdesc_type_kind kind;
+
+  bool operator== (const tdesc_type &other) const
   {
-    memset (&u, 0, sizeof (u));
+    return name == other.name && kind == other.kind;
+  }
 
-    switch (kind)
-      {
-      case TDESC_TYPE_STRUCT:
-      case TDESC_TYPE_UNION:
-      case TDESC_TYPE_FLAGS:
-      case TDESC_TYPE_ENUM:
-	u.u.fields = new std::vector<tdesc_type_field> ();
-	break;
+  bool operator!= (const tdesc_type &other) const
+  {
+    return !(*this == other);
+  }
 
-      default:
-	break;
-      }
+  /* Construct, if necessary, and return the GDB type implementing this
+     target type for architecture GDBARCH.  */
+
+  virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
+};
+
+typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+
+struct tdesc_type_builtin : tdesc_type
+{
+  tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
+  : tdesc_type (name, kind)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
   }
 
-  virtual ~tdesc_type ()
+  type *make_gdb_type (struct gdbarch *gdbarch) const override
   {
-    switch (kind)
+    switch (this->kind)
       {
-      case TDESC_TYPE_STRUCT:
-      case TDESC_TYPE_UNION:
-      case TDESC_TYPE_FLAGS:
-      case TDESC_TYPE_ENUM:
-	delete u.u.fields;
-	break;
+      /* Predefined types.  */
+      case TDESC_TYPE_BOOL:
+        return builtin_type (gdbarch)->builtin_bool;
 
-      default:
-	break;
+      case TDESC_TYPE_INT8:
+        return builtin_type (gdbarch)->builtin_int8;
+
+      case TDESC_TYPE_INT16:
+        return builtin_type (gdbarch)->builtin_int16;
+
+      case TDESC_TYPE_INT32:
+        return builtin_type (gdbarch)->builtin_int32;
+
+      case TDESC_TYPE_INT64:
+        return builtin_type (gdbarch)->builtin_int64;
+
+      case TDESC_TYPE_INT128:
+        return builtin_type (gdbarch)->builtin_int128;
+
+      case TDESC_TYPE_UINT8:
+        return builtin_type (gdbarch)->builtin_uint8;
+
+      case TDESC_TYPE_UINT16:
+        return builtin_type (gdbarch)->builtin_uint16;
+
+      case TDESC_TYPE_UINT32:
+        return builtin_type (gdbarch)->builtin_uint32;
+
+      case TDESC_TYPE_UINT64:
+        return builtin_type (gdbarch)->builtin_uint64;
+
+      case TDESC_TYPE_UINT128:
+        return builtin_type (gdbarch)->builtin_uint128;
+
+      case TDESC_TYPE_CODE_PTR:
+        return builtin_type (gdbarch)->builtin_func_ptr;
+
+      case TDESC_TYPE_DATA_PTR:
+        return builtin_type (gdbarch)->builtin_data_ptr;
+      }
+
+    type *gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
+    if (gdb_type != NULL)
+      return gdb_type;
+
+    switch (this->kind)
+      {
+      case TDESC_TYPE_IEEE_SINGLE:
+        return arch_float_type (gdbarch, -1, "builtin_type_ieee_single",
+				floatformats_ieee_single);
+
+      case TDESC_TYPE_IEEE_DOUBLE:
+        return arch_float_type (gdbarch, -1, "builtin_type_ieee_double",
+				floatformats_ieee_double);
+
+      case TDESC_TYPE_ARM_FPA_EXT:
+        return arch_float_type (gdbarch, -1, "builtin_type_arm_ext",
+				floatformats_arm_ext);
+
+      case TDESC_TYPE_I387_EXT:
+        return arch_float_type (gdbarch, -1, "builtin_type_i387_ext",
+				floatformats_i387_ext);
       }
+
+    internal_error (__FILE__, __LINE__,
+		    "Type \"%s\" has an unknown kind %d",
+		    this->name.c_str (), this->kind);
+
+    return NULL;
   }
+};
 
-  DISABLE_COPY_AND_ASSIGN (tdesc_type);
+/* tdesc_type for vector types.  */
 
-  /* The name of this type.   */
-  std::string name;
+struct tdesc_type_vector : tdesc_type
+{
+  tdesc_type_vector (const std::string &name, tdesc_type *element_type_, int count_)
+  : tdesc_type (name, TDESC_TYPE_VECTOR),
+    element_type (element_type_), count (count_)
+  {}
 
-  /* Identify the kind of this type.  */
-  enum tdesc_type_kind kind;
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
 
-  /* Kind-specific data.  */
-  union
+  type *make_gdb_type (struct gdbarch *gdbarch) const override
   {
-    /* Vector type.  */
-    struct
-    {
-      struct tdesc_type *type;
-      int count;
-    } v;
+    type *vector_gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
+    if (vector_gdb_type != NULL)
+      return vector_gdb_type;
 
-    /* Struct, union, flags, or enum type.  */
-    struct
-    {
-      std::vector<tdesc_type_field> *fields;
-      int size;
-    } u;
-  } u;
+    type *element_gdb_type = this->element_type->make_gdb_type (gdbarch);
+    vector_gdb_type = init_vector_type (element_gdb_type, this->count);
+    TYPE_NAME (vector_gdb_type) = xstrdup (this->name.c_str ());
+
+    return vector_gdb_type;
+  }
+
+  struct tdesc_type *element_type;
+  int count;
+};
+
+/* tdesc_type for struct, union, flags, and enum types.  */
+
+struct tdesc_type_with_fields : tdesc_type
+{
+  tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
+			  int size_ = 0)
+  : tdesc_type (name, kind), size (size_)
+  {}
 
   void accept (tdesc_element_visitor &v) const override
   {
     v.visit (this);
   }
 
-  bool operator== (const tdesc_type &other) const
+  type *make_gdb_type_struct (struct gdbarch *gdbarch) const
   {
-    return name == other.name && kind == other.kind;
+    type *struct_gdb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
+    TYPE_NAME (struct_gdb_type) = xstrdup (this->name.c_str ());
+    TYPE_TAG_NAME (struct_gdb_type) = TYPE_NAME (struct_gdb_type);
+
+    for (const tdesc_type_field &f : this->fields)
+      {
+	if (f.start != -1 && f.end != -1)
+	  {
+	    /* Bitfield.  */
+	    struct field *fld;
+	    struct type *field_gdb_type;
+	    int bitsize, total_size;
+
+	    /* This invariant should be preserved while creating types.  */
+	    gdb_assert (this->size != 0);
+	    if (f.type != NULL)
+	      field_gdb_type = f.type->make_gdb_type (gdbarch);
+	    else if (this->size > 4)
+	      field_gdb_type = builtin_type (gdbarch)->builtin_uint64;
+	    else
+	      field_gdb_type = builtin_type (gdbarch)->builtin_uint32;
+
+	    fld = append_composite_type_field_raw
+	      (struct_gdb_type, xstrdup (f.name.c_str ()), field_gdb_type);
+
+	    /* For little-endian, BITPOS counts from the LSB of
+	       the structure and marks the LSB of the field.  For
+	       big-endian, BITPOS counts from the MSB of the
+	       structure and marks the MSB of the field.  Either
+	       way, it is the number of bits to the "left" of the
+	       field.  To calculate this in big-endian, we need
+	       the total size of the structure.  */
+	    bitsize = f.end - f.start + 1;
+	    total_size = this->size * TARGET_CHAR_BIT;
+	    if (gdbarch_bits_big_endian (gdbarch))
+	      SET_FIELD_BITPOS (fld[0], total_size - f.start - bitsize);
+	    else
+	      SET_FIELD_BITPOS (fld[0], f.start);
+	    FIELD_BITSIZE (fld[0]) = bitsize;
+	  }
+	else
+	  {
+	    gdb_assert (f.start == -1 && f.end == -1);
+	    type *field_gdb_type = f.type->make_gdb_type (gdbarch);
+	    append_composite_type_field (struct_gdb_type,
+					 xstrdup (f.name.c_str ()),
+					 field_gdb_type);
+	  }
+      }
+
+    if (this->size != 0)
+      TYPE_LENGTH (struct_gdb_type) = this->size;
+
+    return struct_gdb_type;
   }
 
-  bool operator!= (const tdesc_type &other) const
+  type *make_gdb_type_union (struct gdbarch *gdbarch) const
   {
-    return !(*this == other);
+    type *union_gdb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
+    TYPE_NAME (union_gdb_type) = xstrdup (this->name.c_str ());
+
+    for (const tdesc_type_field &f : this->fields)
+      {
+	type* field_gdb_type = f.type->make_gdb_type (gdbarch);
+	append_composite_type_field (union_gdb_type, xstrdup (f.name.c_str ()),
+				     field_gdb_type);
+
+	/* If any of the children of a union are vectors, flag the
+	   union as a vector also.  This allows e.g. a union of two
+	   vector types to show up automatically in "info vector".  */
+	if (TYPE_VECTOR (field_gdb_type))
+	  TYPE_VECTOR (union_gdb_type) = 1;
+      }
+
+    return union_gdb_type;
   }
-};
 
-typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+  type *make_gdb_type_flags (struct gdbarch *gdbarch) const
+  {
+    type *flags_gdb_type = arch_flags_type (gdbarch, this->name.c_str (),
+					  this->size * TARGET_CHAR_BIT);
+
+    for (const tdesc_type_field &f : this->fields)
+      {
+      int bitsize = f.end - f.start + 1;
+
+      gdb_assert (f.type != NULL);
+      type *field_gdb_type = f.type->make_gdb_type (gdbarch);
+      append_flags_type_field (flags_gdb_type, f.start, bitsize,
+			       field_gdb_type, f.name.c_str ());
+      }
+
+    return flags_gdb_type;
+  }
+
+  type *make_gdb_type_enum (struct gdbarch *gdbarch) const
+  {
+    type *enum_gdb_type = arch_type (gdbarch, TYPE_CODE_ENUM,
+				   this->size * TARGET_CHAR_BIT,
+				   this->name.c_str ());
+
+    TYPE_UNSIGNED (enum_gdb_type) = 1;
+    for (const tdesc_type_field &f : this->fields)
+      {
+      struct field *fld
+	= append_composite_type_field_raw (enum_gdb_type,
+					   xstrdup (f.name.c_str ()),
+					   NULL);
+
+      SET_FIELD_BITPOS (fld[0], f.start);
+      }
+
+    return enum_gdb_type;
+  }
+
+  type *make_gdb_type (struct gdbarch *gdbarch) const override
+  {
+    type *gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
+    if (gdb_type != NULL)
+      return gdb_type;
+
+    switch (this->kind)
+    {
+      case TDESC_TYPE_STRUCT:
+	return make_gdb_type_struct (gdbarch);
+      case TDESC_TYPE_UNION:
+	return make_gdb_type_union (gdbarch);
+      case TDESC_TYPE_FLAGS:
+	return make_gdb_type_flags (gdbarch);
+      case TDESC_TYPE_ENUM:
+	return make_gdb_type_enum (gdbarch);
+    }
+
+    internal_error (__FILE__, __LINE__,
+		    "Type \"%s\" has an unknown kind %d",
+		    this->name.c_str (), this->kind);
+
+    return NULL;
+  }
+
+  std::vector<tdesc_type_field> fields;
+  int size;
+};
 
 /* A feature from a target description.  Each feature is a collection
    of other elements, e.g. registers and types.  */
@@ -725,7 +959,7 @@ tdesc_feature_name (const struct tdesc_feature *feature)
 }
 
 /* Predefined types.  */
-static struct tdesc_type tdesc_predefined_types[] =
+static tdesc_type_builtin tdesc_predefined_types[] =
 {
   { "bool", TDESC_TYPE_BOOL },
   { "int8", TDESC_TYPE_INT8 },
@@ -799,218 +1033,6 @@ tdesc_find_type (struct gdbarch *gdbarch, const char *id)
   return NULL;
 }
 
-/* Construct, if necessary, and return the GDB type implementing target
-   type TDESC_TYPE for architecture GDBARCH.  */
-
-static struct type *
-tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
-{
-  struct type *type;
-
-  switch (tdesc_type->kind)
-    {
-    /* Predefined types.  */
-    case TDESC_TYPE_BOOL:
-      return builtin_type (gdbarch)->builtin_bool;
-
-    case TDESC_TYPE_INT8:
-      return builtin_type (gdbarch)->builtin_int8;
-
-    case TDESC_TYPE_INT16:
-      return builtin_type (gdbarch)->builtin_int16;
-
-    case TDESC_TYPE_INT32:
-      return builtin_type (gdbarch)->builtin_int32;
-
-    case TDESC_TYPE_INT64:
-      return builtin_type (gdbarch)->builtin_int64;
-
-    case TDESC_TYPE_INT128:
-      return builtin_type (gdbarch)->builtin_int128;
-
-    case TDESC_TYPE_UINT8:
-      return builtin_type (gdbarch)->builtin_uint8;
-
-    case TDESC_TYPE_UINT16:
-      return builtin_type (gdbarch)->builtin_uint16;
-
-    case TDESC_TYPE_UINT32:
-      return builtin_type (gdbarch)->builtin_uint32;
-
-    case TDESC_TYPE_UINT64:
-      return builtin_type (gdbarch)->builtin_uint64;
-
-    case TDESC_TYPE_UINT128:
-      return builtin_type (gdbarch)->builtin_uint128;
-
-    case TDESC_TYPE_CODE_PTR:
-      return builtin_type (gdbarch)->builtin_func_ptr;
-
-    case TDESC_TYPE_DATA_PTR:
-      return builtin_type (gdbarch)->builtin_data_ptr;
-
-    default:
-      break;
-    }
-
-  type = tdesc_find_type (gdbarch, tdesc_type->name.c_str ());
-  if (type)
-    return type;
-
-  switch (tdesc_type->kind)
-    {
-    case TDESC_TYPE_IEEE_SINGLE:
-      return arch_float_type (gdbarch, -1, "builtin_type_ieee_single",
-			      floatformats_ieee_single);
-
-    case TDESC_TYPE_IEEE_DOUBLE:
-      return arch_float_type (gdbarch, -1, "builtin_type_ieee_double",
-			      floatformats_ieee_double);
-
-    case TDESC_TYPE_ARM_FPA_EXT:
-      return arch_float_type (gdbarch, -1, "builtin_type_arm_ext",
-			      floatformats_arm_ext);
-
-    case TDESC_TYPE_I387_EXT:
-      return arch_float_type (gdbarch, -1, "builtin_type_i387_ext",
-			      floatformats_i387_ext);
-
-    /* Types defined by a target feature.  */
-    case TDESC_TYPE_VECTOR:
-      {
-	struct type *type, *field_type;
-
-	field_type = tdesc_gdb_type (gdbarch, tdesc_type->u.v.type);
-	type = init_vector_type (field_type, tdesc_type->u.v.count);
-	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
-
-	return type;
-      }
-
-    case TDESC_TYPE_STRUCT:
-      {
-	struct type *type, *field_type;
-
-	type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
-	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
-	TYPE_TAG_NAME (type) = TYPE_NAME (type);
-
-	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
-	  {
-	    if (f.start != -1 && f.end != -1)
-	      {
-		/* Bitfield.  */
-		struct field *fld;
-		struct type *field_type;
-		int bitsize, total_size;
-
-		/* This invariant should be preserved while creating types.  */
-		gdb_assert (tdesc_type->u.u.size != 0);
-		if (f.type != NULL)
-		  field_type = tdesc_gdb_type (gdbarch, f.type);
-		else if (tdesc_type->u.u.size > 4)
-		  field_type = builtin_type (gdbarch)->builtin_uint64;
-		else
-		  field_type = builtin_type (gdbarch)->builtin_uint32;
-
-		fld = append_composite_type_field_raw
-		  (type, xstrdup (f.name.c_str ()), field_type);
-
-		/* For little-endian, BITPOS counts from the LSB of
-		   the structure and marks the LSB of the field.  For
-		   big-endian, BITPOS counts from the MSB of the
-		   structure and marks the MSB of the field.  Either
-		   way, it is the number of bits to the "left" of the
-		   field.  To calculate this in big-endian, we need
-		   the total size of the structure.  */
-		bitsize = f.end - f.start + 1;
-		total_size = tdesc_type->u.u.size * TARGET_CHAR_BIT;
-		if (gdbarch_bits_big_endian (gdbarch))
-		  SET_FIELD_BITPOS (fld[0], total_size - f.start - bitsize);
-		else
-		  SET_FIELD_BITPOS (fld[0], f.start);
-		FIELD_BITSIZE (fld[0]) = bitsize;
-	      }
-	    else
-	      {
-		gdb_assert (f.start == -1 && f.end == -1);
-		field_type = tdesc_gdb_type (gdbarch, f.type);
-		append_composite_type_field (type, xstrdup (f.name.c_str ()),
-					     field_type);
-	      }
-	  }
-
-	if (tdesc_type->u.u.size != 0)
-	  TYPE_LENGTH (type) = tdesc_type->u.u.size;
-	return type;
-      }
-
-    case TDESC_TYPE_UNION:
-      {
-	struct type *type, *field_type;
-
-	type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
-	TYPE_NAME (type) = xstrdup (tdesc_type->name.c_str ());
-
-	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
-	  {
-	    field_type = tdesc_gdb_type (gdbarch, f.type);
-	    append_composite_type_field (type, xstrdup (f.name.c_str ()),
-					 field_type);
-
-	    /* If any of the children of a union are vectors, flag the
-	       union as a vector also.  This allows e.g. a union of two
-	       vector types to show up automatically in "info vector".  */
-	    if (TYPE_VECTOR (field_type))
-	      TYPE_VECTOR (type) = 1;
-	  }
-	return type;
-      }
-
-    case TDESC_TYPE_FLAGS:
-      {
-	type = arch_flags_type (gdbarch, tdesc_type->name.c_str (),
-				tdesc_type->u.u.size * TARGET_CHAR_BIT);
-	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
-	  {
-	    struct type *field_type;
-	    int bitsize = f.end - f.start + 1;
-
-	    gdb_assert (f.type != NULL);
-	    field_type = tdesc_gdb_type (gdbarch, f.type);
-	    append_flags_type_field (type, f.start, bitsize,
-				     field_type, f.name.c_str ());
-	  }
-
-	return type;
-      }
-
-    case TDESC_TYPE_ENUM:
-      {
-	type = arch_type (gdbarch, TYPE_CODE_ENUM,
-			  tdesc_type->u.u.size * TARGET_CHAR_BIT,
-			  tdesc_type->name.c_str ());
-	TYPE_UNSIGNED (type) = 1;
-	for (const tdesc_type_field &f : *tdesc_type->u.u.fields)
-	  {
-	    struct field *fld
-	      = append_composite_type_field_raw (type,
-						 xstrdup (f.name.c_str ()),
-						 NULL);
-
-	    SET_FIELD_BITPOS (fld[0], f.start);
-	  }
-
-	return type;
-      }
-    }
-
-  internal_error (__FILE__, __LINE__,
-		  "Type \"%s\" has an unknown kind %d",
-		  tdesc_type->name.c_str (), tdesc_type->kind);
-}
-\f
-
 /* Support for registers from target descriptions.  */
 
 /* Construct the per-gdbarch data.  */
@@ -1198,7 +1220,7 @@ tdesc_register_type (struct gdbarch *gdbarch, int regno)
     {
       /* First check for a predefined or target defined type.  */
       if (reg->tdesc_type)
-        arch_reg->type = tdesc_gdb_type (gdbarch, reg->tdesc_type);
+        arch_reg->type = reg->tdesc_type->make_gdb_type (gdbarch);
 
       /* Next try size-sensitive type shortcuts.  */
       else if (reg->type == "float")
@@ -1458,81 +1480,78 @@ struct tdesc_type *
 tdesc_create_vector (struct tdesc_feature *feature, const char *name,
 		     struct tdesc_type *field_type, int count)
 {
-  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_VECTOR);
-
-  type->u.v.type = field_type;
-  type->u.v.count = count;
-
+  tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
   feature->types.emplace_back (type);
+
   return type;
 }
 
 /* See arch/tdesc.h.  */
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_struct (struct tdesc_feature *feature, const char *name)
 {
-  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_STRUCT);
-
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
   feature->types.emplace_back (type);
+
   return type;
 }
 
 /* See arch/tdesc.h.  */
 
 void
-tdesc_set_struct_size (struct tdesc_type *type, int size)
+tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
 {
   gdb_assert (type->kind == TDESC_TYPE_STRUCT);
   gdb_assert (size > 0);
-  type->u.u.size = size;
+  type->size = size;
 }
 
 /* See arch/tdesc.h.  */
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_union (struct tdesc_feature *feature, const char *name)
 {
-  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_UNION);
-
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
   feature->types.emplace_back (type);
+
   return type;
 }
 
 /* See arch/tdesc.h.  */
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_flags (struct tdesc_feature *feature, const char *name,
 		    int size)
 {
-  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_FLAGS);
-
   gdb_assert (size > 0);
 
-  type->u.u.size = size;
-
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
   feature->types.emplace_back (type);
+
   return type;
 }
 
-struct tdesc_type *
+tdesc_type_with_fields *
 tdesc_create_enum (struct tdesc_feature *feature, const char *name,
 		   int size)
 {
-  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_ENUM);
-
   gdb_assert (size > 0);
 
-  type->u.u.size = size;
-
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
   feature->types.emplace_back (type);
+
   return type;
 }
 
 /* See arch/tdesc.h.  */
 
 void
-tdesc_add_field (struct tdesc_type *type, const char *field_name,
+tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
 		 struct tdesc_type *field_type)
 {
   gdb_assert (type->kind == TDESC_TYPE_UNION
@@ -1540,31 +1559,31 @@ tdesc_add_field (struct tdesc_type *type, const char *field_name,
 
   /* Initialize start and end so we know this is not a bit-field
      when we print-c-tdesc.  */
-  type->u.u.fields->emplace_back (field_name, field_type, -1, -1);
+  type->fields.emplace_back (field_name, field_type, -1, -1);
 }
 
 void
-tdesc_add_typed_bitfield (struct tdesc_type *type, const char *field_name,
+tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
 			  int start, int end, struct tdesc_type *field_type)
 {
   gdb_assert (type->kind == TDESC_TYPE_STRUCT
 	      || type->kind == TDESC_TYPE_FLAGS);
   gdb_assert (start >= 0 && end >= start);
 
-  type->u.u.fields->emplace_back (field_name, field_type, start, end);
+  type->fields.emplace_back (field_name, field_type, start, end);
 }
 
 /* See arch/tdesc.h.  */
 
 void
-tdesc_add_bitfield (struct tdesc_type *type, const char *field_name,
+tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
 		    int start, int end)
 {
   struct tdesc_type *field_type;
 
   gdb_assert (start >= 0 && end >= start);
 
-  if (type->u.u.size > 4)
+  if (type->size > 4)
     field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
   else
     field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
@@ -1575,26 +1594,25 @@ tdesc_add_bitfield (struct tdesc_type *type, const char *field_name,
 /* See arch/tdesc.h.  */
 
 void
-tdesc_add_flag (struct tdesc_type *type, int start,
+tdesc_add_flag (tdesc_type_with_fields *type, int start,
 		const char *flag_name)
 {
   gdb_assert (type->kind == TDESC_TYPE_FLAGS
 	      || type->kind == TDESC_TYPE_STRUCT);
 
-  type->u.u.fields->emplace_back (flag_name,
-				   tdesc_predefined_type (TDESC_TYPE_BOOL),
-				   start, start);
+  type->fields.emplace_back (flag_name,
+			     tdesc_predefined_type (TDESC_TYPE_BOOL),
+			     start, start);
 }
 
 void
-tdesc_add_enum_value (struct tdesc_type *type, int value,
+tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
 		      const char *name)
 {
   gdb_assert (type->kind == TDESC_TYPE_ENUM);
-
-  type->u.u.fields->emplace_back (name,
-				   tdesc_predefined_type (TDESC_TYPE_INT32),
-				   value, -1);
+  type->fields.emplace_back (name,
+			     tdesc_predefined_type (TDESC_TYPE_INT32),
+			     value, -1);
 }
 
 /* See arch/tdesc.h.  */
@@ -1848,38 +1866,46 @@ public:
     printf_unfiltered ("}\n");
   }
 
-  void visit (const tdesc_type *type) override
+  void visit (const tdesc_type_builtin *type) override
+  {
+    error (_("C output is not supported type \"%s\"."), type->name.c_str ());
+  }
+
+  void visit (const tdesc_type_vector *type) override
   {
-    /* Now we do some "filtering" in order to know which variables to
-       declare.  This is needed because otherwise we would declare unused
-       variables `field_type' and `type'.  */
     if (!m_printed_field_type)
       {
 	printf_unfiltered ("  struct tdesc_type *field_type;\n");
 	m_printed_field_type = true;
       }
 
-    if ((type->kind == TDESC_TYPE_UNION
-	 || type->kind == TDESC_TYPE_STRUCT
-	 || type->kind == TDESC_TYPE_FLAGS
-	 || type->kind == TDESC_TYPE_ENUM)
-	&& !type->u.u.fields->empty ()
-	&& !m_printed_type)
+    printf_unfiltered
+      ("  field_type = tdesc_named_type (feature, \"%s\");\n",
+       type->element_type->name.c_str ());
+    printf_unfiltered
+      ("  tdesc_create_vector (feature, \"%s\", field_type, %d);\n",
+       type->name.c_str (), type->count);
+
+    printf_unfiltered ("\n");
+  }
+
+  void visit (const tdesc_type_with_fields *type) override
+  {
+    if (!m_printed_type)
       {
-	printf_unfiltered ("  struct tdesc_type *type;\n");
+	printf_unfiltered ("  tdesc_type_with_fields *type;\n");
 	m_printed_type = true;
       }
 
+    if (!type->fields.empty ()
+	&& !m_printed_field_type)
+      {
+	printf_unfiltered ("  struct tdesc_type *field_type;\n");
+	m_printed_field_type = true;
+      }
+
     switch (type->kind)
       {
-      case TDESC_TYPE_VECTOR:
-	printf_unfiltered
-	  ("  field_type = tdesc_named_type (feature, \"%s\");\n",
-	   type->u.v.type->name.c_str ());
-	printf_unfiltered
-	  ("  tdesc_create_vector (feature, \"%s\", field_type, %d);\n",
-	   type->name.c_str (), type->u.v.count);
-	break;
       case TDESC_TYPE_STRUCT:
       case TDESC_TYPE_FLAGS:
 	if (type->kind == TDESC_TYPE_STRUCT)
@@ -1887,18 +1913,17 @@ public:
 	    printf_unfiltered
 	      ("  type = tdesc_create_struct (feature, \"%s\");\n",
 	       type->name.c_str ());
-	    if (type->u.u.size != 0)
+	    if (type->size != 0)
 	      printf_unfiltered
-		("  tdesc_set_struct_size (type, %d);\n",
-		 type->u.u.size);
+		("  tdesc_set_struct_size (type, %d);\n", type->size);
 	  }
 	else
 	  {
 	    printf_unfiltered
 	      ("  type = tdesc_create_flags (feature, \"%s\", %d);\n",
-	       type->name.c_str (), type->u.u.size);
+	       type->name.c_str (), type->size);
 	  }
-	for (const tdesc_type_field &f : *type->u.u.fields)
+	for (const tdesc_type_field &f : type->fields)
 	  {
 	    const char *type_name;
 
@@ -1917,9 +1942,8 @@ public:
 		      ("  tdesc_add_flag (type, %d, \"%s\");\n",
 		       f.start, f.name.c_str ());
 		  }
-		else if ((type->u.u.size == 4
-			  && f.type->kind == TDESC_TYPE_UINT32)
-			 || (type->u.u.size == 8
+		else if ((type->size == 4 && f.type->kind == TDESC_TYPE_UINT32)
+			 || (type->size == 8
 			     && f.type->kind == TDESC_TYPE_UINT64))
 		  {
 		    printf_unfiltered
@@ -1929,8 +1953,7 @@ public:
 		else
 		  {
 		    printf_unfiltered
-		      ("  field_type = tdesc_named_type (feature,"
-		       " \"%s\");\n",
+		      ("  field_type = tdesc_named_type (feature, \"%s\");\n",
 		       type_name);
 		    printf_unfiltered
 		      ("  tdesc_add_typed_bitfield (type, \"%s\","
@@ -1956,7 +1979,7 @@ public:
 	printf_unfiltered
 	  ("  type = tdesc_create_union (feature, \"%s\");\n",
 	   type->name.c_str ());
-	for (const tdesc_type_field &f : *type->u.u.fields)
+	for (const tdesc_type_field &f : type->fields)
 	  {
 	    printf_unfiltered
 	      ("  field_type = tdesc_named_type (feature, \"%s\");\n",
@@ -1969,8 +1992,8 @@ public:
       case TDESC_TYPE_ENUM:
 	printf_unfiltered
 	  ("  type = tdesc_create_enum (feature, \"%s\", %d);\n",
-	   type->name.c_str (), type->u.u.size);
-	for (const tdesc_type_field &f : *type->u.u.fields)
+	   type->name.c_str (), type->size);
+	for (const tdesc_type_field &f : type->fields)
 	  printf_unfiltered
 	    ("  tdesc_add_enum_value (type, %d, \"%s\");\n",
 	     f.start, f.name.c_str ());
@@ -1978,6 +2001,7 @@ public:
       default:
 	error (_("C output is not supported type \"%s\"."), type->name.c_str ());
       }
+
     printf_unfiltered ("\n");
   }
 
@@ -1998,7 +2022,11 @@ protected:
 
 private:
   char *m_function;
+
+  /* Did we print "struct tdesc_type *field_type;" yet?  */
   bool m_printed_field_type = false;
+
+  /* Did we print "struct tdesc_type_with_fields *type;" yet?  */
   bool m_printed_type = false;
 };
 
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index e97a3738e2..8465c9ea69 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -209,13 +209,13 @@ void set_tdesc_property (struct target_desc *,
 			 const char *key, const char *value);
 void tdesc_add_compatible (struct target_desc *,
 			   const struct bfd_arch_info *);
-struct tdesc_type *tdesc_create_enum (struct tdesc_feature *feature,
-				      const char *name,
-				      int size);
-void tdesc_add_typed_bitfield (struct tdesc_type *type, const char *field_name,
+tdesc_type_with_fields *tdesc_create_enum (struct tdesc_feature *feature,
+					   const char *name,
+					   int size);
+void tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
 			       int start, int end,
 			       struct tdesc_type *field_type);
-void tdesc_add_enum_value (struct tdesc_type *type, int value,
+void tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
 			   const char *name);
 
 #if GDB_SELF_TEST
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 5a1f459f98..6baeadfdff 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -83,7 +83,7 @@ struct tdesc_parsing_data
   int next_regnum;
 
   /* The struct or union we are currently parsing, or last parsed.  */
-  struct tdesc_type *current_type;
+  tdesc_type_with_fields *current_type;
 
   /* The byte size of the current struct/flags type, if specified.  Zero
      if not specified.  Flags values must specify a size.  */
@@ -244,11 +244,11 @@ tdesc_start_struct (struct gdb_xml_parser *parser,
 {
   struct tdesc_parsing_data *data = (struct tdesc_parsing_data *) user_data;
   char *id = (char *) xml_find_attribute (attributes, "id")->value;
-  struct tdesc_type *type;
   struct gdb_xml_value *attr;
 
-  type = tdesc_create_struct (data->current_feature, id);
-  data->current_type = type;
+  tdesc_type_with_fields *type_with_fields
+    = tdesc_create_struct (data->current_feature, id);
+  data->current_type = type_with_fields;
   data->current_type_size = 0;
 
   attr = xml_find_attribute (attributes, "size");
@@ -262,7 +262,7 @@ tdesc_start_struct (struct gdb_xml_parser *parser,
 			 _("Struct size %s is larger than maximum (%d)"),
 			 pulongest (size), MAX_FIELD_SIZE);
 	}
-      tdesc_set_struct_size (type, size);
+      tdesc_set_struct_size (type_with_fields, size);
       data->current_type_size = size;
     }
 }
@@ -276,7 +276,6 @@ tdesc_start_flags (struct gdb_xml_parser *parser,
   char *id = (char *) xml_find_attribute (attributes, "id")->value;
   ULONGEST size = * (ULONGEST *)
     xml_find_attribute (attributes, "size")->value;
-  struct tdesc_type *type;
 
   if (size > MAX_FIELD_SIZE)
     {
@@ -284,9 +283,8 @@ tdesc_start_flags (struct gdb_xml_parser *parser,
 		     _("Flags size %s is larger than maximum (%d)"),
 		     pulongest (size), MAX_FIELD_SIZE);
     }
-  type = tdesc_create_flags (data->current_feature, id, size);
 
-  data->current_type = type;
+  data->current_type = tdesc_create_flags (data->current_feature, id, size);
   data->current_type_size = size;
 }
 
@@ -299,7 +297,6 @@ tdesc_start_enum (struct gdb_xml_parser *parser,
   char *id = (char *) xml_find_attribute (attributes, "id")->value;
   int size = * (ULONGEST *)
     xml_find_attribute (attributes, "size")->value;
-  struct tdesc_type *type;
 
   if (size > MAX_FIELD_SIZE)
     {
@@ -307,9 +304,8 @@ tdesc_start_enum (struct gdb_xml_parser *parser,
 		     _("Enum size %s is larger than maximum (%d)"),
 		     pulongest (size), MAX_FIELD_SIZE);
     }
-  type = tdesc_create_enum (data->current_feature, id, size);
 
-  data->current_type = type;
+  data->current_type = tdesc_create_enum (data->current_feature, id, size);
   data->current_type_size = 0;
 }
 
@@ -375,7 +371,7 @@ tdesc_start_field (struct gdb_xml_parser *parser,
 
   if (start != -1)
     {
-      struct tdesc_type *t = data->current_type;
+      tdesc_type_with_fields *t = data->current_type;
 
       /* Older versions of gdb can't handle elided end values.
          Stick with that for now, to help ensure backward compatibility.
-- 
2.15.0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 11/10] Split tdesc_type into multiple classes
  2017-12-03 16:04       ` [PATCH 11/10] Split tdesc_type into multiple classes Simon Marchi
@ 2017-12-05 16:46         ` Yao Qi
  2017-12-05 21:42           ` Simon Marchi
  0 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2017-12-05 16:46 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@polymtl.ca> writes:

> Hi Yao,
>
> I revisited this patch, and implemented as best as I could what you
> suggested.  This patch applies on top of the current series.
>
> This patch makes tdesc_type an abstract base class and creates three
> subclasses:
>
> - tdesc_type_builtin, for builtin types
> - tdesc_type_vector, for vector types
> - tdesc_type_with_fields, for struct, union, flag and enum types
>
> This allows getting rid of the union in tdesc_type and to not allow the
> std::vector separately.  I tried to go further and create separate
> classes for struct, union, flag and enum, but it proved too difficult.
> One problem is that from the point of the of the target description
> code, the types tdesc_type_* are opaque (only forward-declared).
> Therefore, it doesn't know about inheritance relationship between those
> classes.  This makes it impossible to make functions that accept a
> pointer to a base class and pass a pointer to a derived class, for
> example.  I think this patch here is a good compromise, and if somebody
> wants to improve things further, the door is open.
>
> A make_gdb_type virtual pure method is added to tdesc_type, which
> replaces the current tdesc_gdb_type function.  Calling this method on a
> tdesc_type returns the corresponding built gdb type.

Hi Simon,
Thanks for doing this, patch looks good to me.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 11/10] Split tdesc_type into multiple classes
  2017-12-05 16:46         ` Yao Qi
@ 2017-12-05 21:42           ` Simon Marchi
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2017-12-05 21:42 UTC (permalink / raw)
  To: Yao Qi, Simon Marchi; +Cc: gdb-patches

On 2017-12-05 11:46 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
>> Hi Yao,
>>
>> I revisited this patch, and implemented as best as I could what you
>> suggested.  This patch applies on top of the current series.
>>
>> This patch makes tdesc_type an abstract base class and creates three
>> subclasses:
>>
>> - tdesc_type_builtin, for builtin types
>> - tdesc_type_vector, for vector types
>> - tdesc_type_with_fields, for struct, union, flag and enum types
>>
>> This allows getting rid of the union in tdesc_type and to not allow the
>> std::vector separately.  I tried to go further and create separate
>> classes for struct, union, flag and enum, but it proved too difficult.
>> One problem is that from the point of the of the target description
>> code, the types tdesc_type_* are opaque (only forward-declared).
>> Therefore, it doesn't know about inheritance relationship between those
>> classes.  This makes it impossible to make functions that accept a
>> pointer to a base class and pass a pointer to a derived class, for
>> example.  I think this patch here is a good compromise, and if somebody
>> wants to improve things further, the door is open.
>>
>> A make_gdb_type virtual pure method is added to tdesc_type, which
>> replaces the current tdesc_gdb_type function.  Calling this method on a
>> tdesc_type returns the corresponding built gdb type.
> 
> Hi Simon,
> Thanks for doing this, patch looks good to me.
> 

Thanks, I pushed the series and this patch.  I realized too late that I
must have been working on the branch without the fixes after your review
comments... so I added this new patch on top.

Simon


From 858c9d13240e695bc3b750368f5d4e524b12112e Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 5 Dec 2017 16:39:35 -0500
Subject: [PATCH] Address review comments for the previous series

I failed at git and missed adding/lost changes on the wrong branch, the
result being that I didn't incorporate fixes resulting from Yao's review
comments.  This patch fixes that.

There are two places where we should use the unique pointer typedef, and
ChangeLog entries missing.

gdb/ChangeLog:

	* target-descriptions.c (struct tdesc_feature) <registers>: Use
	tdesc_reg_up typedef.
	(struct target_desc) <features>: Use tdesc_feature_up typedef.
---
 gdb/ChangeLog             | 8 ++++++++
 gdb/target-descriptions.c | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1d3a3e8..657f87b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-12-05  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* target-descriptions.c (struct tdesc_feature) <registers>: Use
+	tdesc_reg_up typedef.
+	(struct target_desc) <features>: Use tdesc_feature_up typedef.
+
 2017-12-05  Simon Marchi  <simon.marchi@polymtl.ca>

 	* target-descriptions.c (struct tdesc_type): Use default
@@ -118,6 +124,8 @@
 	(tdesc_register_in_reggroup_p): Adjust.
 	(class print_c_tdesc) <visit>: Adjust.
 	(class print_c_feature) <visit>: Adjust.
+	* features/arc-arcompact.c: Re-generate.
+	* features/arc-v2.c: Re-generate.

 2017-12-05  Simon Marchi  <simon.marchi@ericsson.com>

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 12d72fa..5a6f619 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -529,7 +529,7 @@ struct tdesc_feature : tdesc_element
   std::string name;

   /* The registers associated with this feature.  */
-  std::vector<std::unique_ptr<tdesc_reg>> registers;
+  std::vector<tdesc_reg_up> registers;

   /* The types associated with this feature.  */
   std::vector<tdesc_type_up> types;
@@ -613,7 +613,7 @@ struct target_desc : tdesc_element
   std::vector<property> properties;

   /* The features associated with this target.  */
-  std::vector<std::unique_ptr<tdesc_feature>> features;
+  std::vector<tdesc_feature_up> features;

   void accept (tdesc_element_visitor &v) const override
   {
-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2017-12-05 21:42 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31  1:42 [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Simon Marchi
2017-10-31  1:42 ` [PATCH 08/10] Make tdesc_type::name an std::string Simon Marchi
2017-10-31  1:42 ` [PATCH 09/10] Make tdesc_type::u::u::fields an std::vector Simon Marchi
2017-11-02 10:02   ` Yao Qi
2017-11-02 13:27     ` Simon Marchi
2017-12-03 16:04       ` [PATCH 11/10] Split tdesc_type into multiple classes Simon Marchi
2017-12-05 16:46         ` Yao Qi
2017-12-05 21:42           ` Simon Marchi
2017-10-31  1:42 ` [PATCH 10/10] Make tdesc_arch_data::arch_regs an std::vector Simon Marchi
2017-10-31  1:42 ` [PATCH 04/10] Make tdesc_feature::name an std::string Simon Marchi
2017-10-31  1:42 ` [PATCH 05/10] Make tdesc_feature::registers an std::vector Simon Marchi
2017-11-02  9:32   ` Yao Qi
2017-11-02 13:22     ` Simon Marchi
2017-10-31  1:42 ` [PATCH 07/10] Make tdesc_feature::types " Simon Marchi
2017-10-31  1:42 ` [PATCH 01/10] Make target_desc::properties " Simon Marchi
2017-10-31  1:42 ` [PATCH 06/10] Make tdesc_reg string fields std::string Simon Marchi
2017-11-02  9:43   ` Yao Qi
2017-11-02 13:24     ` Simon Marchi
2017-10-31  1:42 ` [PATCH 03/10] Make target_desc::features an std::vector Simon Marchi
2017-11-02  9:29   ` Yao Qi
2017-11-02 13:20     ` Simon Marchi
2017-10-31  1:43 ` [PATCH 02/10] Make target_desc::compatible " Simon Marchi
2017-11-02 10:16 ` [PATCH 00/10] Use std::vector and std::string throughout target-descriptions.c Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).