[PATCH 0/7] Remove char-based bitfield macros

[PATCH 0/7] Remove char-based bitfield macros

[Tom Tromey]

This series removes the char-based bitfield macros from gdbtypes.h,
moving the associated data into 'struct field'.  A few other minor
cleanups are done along the way.

Regression tested on x86-64 Fedora 36.

---
Tom Tromey (7):
      Use .def file to stringify type codes
      Print field accessibility inline
      Remove byte vectors from cplus_struct_type
      Add field::is_public
      Remove some QUIT calls from need_access_label_p
      Remove some type field accessor macros
      Remove char-based bitfield macros

 gdb/ada-valprint.c                |   2 +-
 gdb/c-typeprint.c                 |  17 ++--
 gdb/c-varobj.c                    |  16 ++--
 gdb/compile/compile-cplus-types.c |   7 +-
 gdb/cp-valprint.c                 |   4 +-
 gdb/dwarf2/read.c                 |  86 ++++--------------
 gdb/gdbtypes.c                    | 182 ++++++++------------------------------
 gdb/gdbtypes.h                    | 134 ++++++++++++----------------
 gdb/p-typeprint.c                 |   6 +-
 gdb/p-valprint.c                  |   4 +-
 gdb/stabsread.c                   | 123 +++++++++-----------------
 11 files changed, 180 insertions(+), 401 deletions(-)
---
base-commit: 0ad14a8c34c866f64271041cb69b0e8a05913de8
change-id: 20230921-field-bits-9b9f802eb42b

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/7] Use .def file to stringify type codes

[Tom Tromey]

This changes recursive_dump_type to reuse the type-codes.def file when
stringifying type codes.
---
 gdb/gdbtypes.c | 99 +++++++++++-----------------------------------------------
 1 file changed, 18 insertions(+), 81 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 7011fddd695..18aa8e5c29e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5099,6 +5099,23 @@ dump_dynamic_prop (dynamic_prop const& prop)
     }
 }
 
+/* Return a string that represents a type code.  */
+static const char *
+type_code_name (type_code code)
+{
+  switch (code)
+    {
+#define OP(X) case X: return # X;
+#include "type-codes.def"
+#undef OP
+
+    case TYPE_CODE_UNDEF:
+      return "TYPE_CODE_UNDEF";
+    default:
+      return "UNKNOWN TYPE CODE";
+    }
+}
+
 void
 recursive_dump_type (struct type *type, int spaces)
 {
@@ -5136,87 +5153,7 @@ recursive_dump_type (struct type *type, int spaces)
 	      type->name () ? type->name () : "<NULL>",
 	      host_address_to_string (type->name ()));
   gdb_printf ("%*scode 0x%x ", spaces, "", type->code ());
-  switch (type->code ())
-    {
-    case TYPE_CODE_UNDEF:
-      gdb_printf ("(TYPE_CODE_UNDEF)");
-      break;
-    case TYPE_CODE_PTR:
-      gdb_printf ("(TYPE_CODE_PTR)");
-      break;
-    case TYPE_CODE_ARRAY:
-      gdb_printf ("(TYPE_CODE_ARRAY)");
-      break;
-    case TYPE_CODE_STRUCT:
-      gdb_printf ("(TYPE_CODE_STRUCT)");
-      break;
-    case TYPE_CODE_UNION:
-      gdb_printf ("(TYPE_CODE_UNION)");
-      break;
-    case TYPE_CODE_ENUM:
-      gdb_printf ("(TYPE_CODE_ENUM)");
-      break;
-    case TYPE_CODE_FLAGS:
-      gdb_printf ("(TYPE_CODE_FLAGS)");
-      break;
-    case TYPE_CODE_FUNC:
-      gdb_printf ("(TYPE_CODE_FUNC)");
-      break;
-    case TYPE_CODE_INT:
-      gdb_printf ("(TYPE_CODE_INT)");
-      break;
-    case TYPE_CODE_FLT:
-      gdb_printf ("(TYPE_CODE_FLT)");
-      break;
-    case TYPE_CODE_VOID:
-      gdb_printf ("(TYPE_CODE_VOID)");
-      break;
-    case TYPE_CODE_SET:
-      gdb_printf ("(TYPE_CODE_SET)");
-      break;
-    case TYPE_CODE_RANGE:
-      gdb_printf ("(TYPE_CODE_RANGE)");
-      break;
-    case TYPE_CODE_STRING:
-      gdb_printf ("(TYPE_CODE_STRING)");
-      break;
-    case TYPE_CODE_ERROR:
-      gdb_printf ("(TYPE_CODE_ERROR)");
-      break;
-    case TYPE_CODE_MEMBERPTR:
-      gdb_printf ("(TYPE_CODE_MEMBERPTR)");
-      break;
-    case TYPE_CODE_METHODPTR:
-      gdb_printf ("(TYPE_CODE_METHODPTR)");
-      break;
-    case TYPE_CODE_METHOD:
-      gdb_printf ("(TYPE_CODE_METHOD)");
-      break;
-    case TYPE_CODE_REF:
-      gdb_printf ("(TYPE_CODE_REF)");
-      break;
-    case TYPE_CODE_CHAR:
-      gdb_printf ("(TYPE_CODE_CHAR)");
-      break;
-    case TYPE_CODE_BOOL:
-      gdb_printf ("(TYPE_CODE_BOOL)");
-      break;
-    case TYPE_CODE_COMPLEX:
-      gdb_printf ("(TYPE_CODE_COMPLEX)");
-      break;
-    case TYPE_CODE_TYPEDEF:
-      gdb_printf ("(TYPE_CODE_TYPEDEF)");
-      break;
-    case TYPE_CODE_NAMESPACE:
-      gdb_printf ("(TYPE_CODE_NAMESPACE)");
-      break;
-    case TYPE_CODE_FIXED_POINT:
-      gdb_printf ("(TYPE_CODE_FIXED_POINT)");
-      break;
-    default:
-      gdb_printf ("(UNKNOWN TYPE CODE)");
-      break;
-    }
+  gdb_printf ("(%s)", type_code_name (type->code ()));
   gdb_puts ("\n");
   gdb_printf ("%*slength %s\n", spaces, "",
 	      pulongest (type->length ()));

-- 
2.40.1

[PATCH 2/7] Print field accessibility inline

[Tom Tromey]

This changes recursive_dump_type to print field accessibility
information "inline".  This is clearer and preserves the information
when the byte vectors are removed.
---
 gdb/gdbtypes.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 18aa8e5c29e..1aabb38fa5f 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5289,12 +5289,21 @@ recursive_dump_type (struct type *type, int spaces)
 	gdb_printf ("%*s[%d] bitpos %s bitsize %d type ", spaces + 2, "",
 		    idx, plongest (type->field (idx).loc_bitpos ()),
 		    type->field (idx).bitsize ());
-      gdb_printf ("%s name '%s' (%s)\n",
+      gdb_printf ("%s name '%s' (%s)",
 		  host_address_to_string (type->field (idx).type ()),
 		  type->field (idx).name () != NULL
 		  ? type->field (idx).name ()
 		  : "<NULL>",
 		  host_address_to_string (type->field (idx).name ()));
+      if (TYPE_FIELD_VIRTUAL (type, idx))
+	gdb_printf (" virtual");
+      if (TYPE_FIELD_PRIVATE (type, idx))
+	gdb_printf (" private");
+      else if (TYPE_FIELD_PROTECTED (type, idx))
+	gdb_printf (" protected");
+      else if (TYPE_FIELD_IGNORE (type, idx))
+	gdb_printf (" ignored");
+      gdb_printf ("\n");
       if (type->field (idx).type () != NULL)
 	{
 	  recursive_dump_type (type->field (idx).type (), spaces + 4);

-- 
2.40.1

[PATCH 3/7] Remove byte vectors from cplus_struct_type

[Tom Tromey]

This removes some byte vectors from cplus_struct_type, moving the
information into bitfields in holes in struct field.
---
 gdb/dwarf2/read.c |  86 +++++++-------------------------------
 gdb/gdbtypes.c    |  53 -----------------------
 gdb/gdbtypes.h    | 118 ++++++++++++++++++++++++---------------------------
 gdb/stabsread.c   | 123 +++++++++++++++++++-----------------------------------
 4 files changed, 113 insertions(+), 267 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 5bbc8e24cf9..2f7c4c5483f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -670,8 +670,6 @@ struct variant_part_builder
 
 struct nextfield
 {
-  int accessibility = 0;
-  int virtuality = 0;
   /* Variant parts need to find the discriminant, which is a DIE
      reference.  We track the section offset of each field to make
      this link.  */
@@ -694,9 +692,6 @@ struct field_info
   std::vector<struct nextfield> fields;
   std::vector<struct nextfield> baseclasses;
 
-  /* Set if the accessibility of one of the fields is not public.  */
-  bool non_public_fields = false;
-
   /* Member function fieldlist array, contains name of possibly overloaded
      member function, number of overloaded member functions and a pointer
      to the head of the member function field chain.  */
@@ -11644,15 +11639,23 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 
   new_field->offset = die->sect_off;
 
-  new_field->accessibility = dwarf2_access_attribute (die, cu);
-  if (new_field->accessibility != DW_ACCESS_public)
-    fip->non_public_fields = true;
+  switch (dwarf2_access_attribute (die, cu))
+    {
+    case DW_ACCESS_public:
+      break;
+    case DW_ACCESS_private:
+      new_field->field.set_private ();
+      break;
+    case DW_ACCESS_protected:
+      new_field->field.set_protected ();
+      break;
+    default:
+      gdb_assert_not_reached ("invalid accessibility");
+    }
 
   attr = dwarf2_attr (die, DW_AT_virtuality, cu);
-  if (attr != nullptr)
-    new_field->virtuality = attr->as_virtuality ();
-  else
-    new_field->virtuality = DW_VIRTUALITY_none;
+  if (attr != nullptr && attr->as_virtuality ())
+    new_field->field.set_virtual ();
 
   fp = &new_field->field;
 
@@ -11746,8 +11749,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
       if (dwarf2_attr (die, DW_AT_artificial, cu))
 	{
 	  fp->set_is_artificial (true);
-	  new_field->accessibility = DW_ACCESS_private;
-	  fip->non_public_fields = true;
+	  fp->set_private ();
 	}
     }
   else if (die->tag == DW_TAG_member || die->tag == DW_TAG_variable)
@@ -12061,30 +12063,9 @@ dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type,
      and create blank accessibility bitfields if necessary.  */
   type->alloc_fields (nfields);
 
-  if (fip->non_public_fields && cu->lang () != language_ada)
-    {
-      ALLOCATE_CPLUS_STRUCT_TYPE (type);
-
-      TYPE_FIELD_PRIVATE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_PROTECTED_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_IGNORE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-    }
-
-  /* If the type has baseclasses, allocate and clear a bit vector for
-     TYPE_FIELD_VIRTUAL_BITS.  */
   if (!fip->baseclasses.empty () && cu->lang () != language_ada)
     {
-      int num_bytes = B_BYTES (fip->baseclasses.size ());
-      unsigned char *pointer;
-
       ALLOCATE_CPLUS_STRUCT_TYPE (type);
-      pointer = (unsigned char *) TYPE_ZALLOC (type, num_bytes);
-      TYPE_FIELD_VIRTUAL_BITS (type) = pointer;
       TYPE_N_BASECLASSES (type) = fip->baseclasses.size ();
     }
 
@@ -12099,41 +12080,6 @@ dwarf2_attach_fields_to_type (struct field_info *fip, struct type *type,
 	   : fip->fields[i - fip->baseclasses.size ()]);
 
       type->field (i) = field.field;
-      switch (field.accessibility)
-	{
-	case DW_ACCESS_private:
-	  if (cu->lang () != language_ada)
-	    SET_TYPE_FIELD_PRIVATE (type, i);
-	  break;
-
-	case DW_ACCESS_protected:
-	  if (cu->lang () != language_ada)
-	    SET_TYPE_FIELD_PROTECTED (type, i);
-	  break;
-
-	case DW_ACCESS_public:
-	  break;
-
-	default:
-	  /* Unknown accessibility.  Complain and treat it as public.  */
-	  {
-	    complaint (_("unsupported accessibility %d"),
-		       field.accessibility);
-	  }
-	  break;
-	}
-      if (i < fip->baseclasses.size ())
-	{
-	  switch (field.virtuality)
-	    {
-	    case DW_VIRTUALITY_virtual:
-	    case DW_VIRTUALITY_pure_virtual:
-	      if (cu->lang () == language_ada)
-		error (_("unexpected virtuality in component of Ada type"));
-	      SET_TYPE_FIELD_VIRTUAL (type, i);
-	      break;
-	    }
-	}
     }
 }
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 1aabb38fa5f..55d12218b4e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -4883,25 +4883,6 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
 
 /* End of functions for overload resolution.  */
 \f
-/* Routines to pretty-print types.  */
-
-static void
-print_bit_vector (B_TYPE *bits, int nbits)
-{
-  int bitno;
-
-  for (bitno = 0; bitno < nbits; bitno++)
-    {
-      if ((bitno % 8) == 0)
-	{
-	  gdb_puts (" ");
-	}
-      if (B_TST (bits, bitno))
-	gdb_printf (("1"));
-      else
-	gdb_printf (("0"));
-    }
-}
 
 /* Note the first arg should be the "this" pointer, we may not want to
    include it since we may get into a infinitely recursive
@@ -5004,40 +4985,6 @@ print_cplus_stuff (struct type *type, int spaces)
 	      TYPE_N_BASECLASSES (type));
   gdb_printf ("%*snfn_fields %d\n", spaces, "",
 	      TYPE_NFN_FIELDS (type));
-  if (TYPE_N_BASECLASSES (type) > 0)
-    {
-      gdb_printf
-	("%*svirtual_field_bits (%d bits at *%s)",
-	 spaces, "", TYPE_N_BASECLASSES (type),
-	 host_address_to_string (TYPE_FIELD_VIRTUAL_BITS (type)));
-
-      print_bit_vector (TYPE_FIELD_VIRTUAL_BITS (type),
-			TYPE_N_BASECLASSES (type));
-      gdb_puts ("\n");
-    }
-  if (type->num_fields () > 0)
-    {
-      if (TYPE_FIELD_PRIVATE_BITS (type) != NULL)
-	{
-	  gdb_printf
-	    ("%*sprivate_field_bits (%d bits at *%s)",
-	     spaces, "", type->num_fields (),
-	     host_address_to_string (TYPE_FIELD_PRIVATE_BITS (type)));
-	  print_bit_vector (TYPE_FIELD_PRIVATE_BITS (type),
-			    type->num_fields ());
-	  gdb_puts ("\n");
-	}
-      if (TYPE_FIELD_PROTECTED_BITS (type) != NULL)
-	{
-	  gdb_printf
-	    ("%*sprotected_field_bits (%d bits at *%s",
-	     spaces, "", type->num_fields (),
-	     host_address_to_string (TYPE_FIELD_PROTECTED_BITS (type)));
-	  print_bit_vector (TYPE_FIELD_PROTECTED_BITS (type),
-			    type->num_fields ());
-	  gdb_puts ("\n");
-	}
-    }
   if (TYPE_NFN_FIELDS (type) > 0)
     {
       dump_fn_fieldlists (type, spaces);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 505c8ba12b5..c72512b8204 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -668,6 +668,46 @@ struct field
     m_loc.dwarf_block = dwarf_block;
   }
 
+  /* True if this field is 'private'.  */
+  bool is_private () const
+  { return m_private; }
+
+  /* Set the field's "private" flag.  */
+  void set_private ()
+  {
+    /* Can't have both.  */
+    gdb_assert (!m_protected);
+    m_private = true;
+  }
+
+  /* True if this field is 'protected'.  */
+  bool is_protected () const
+  { return m_protected; }
+
+  /* Set the field's "protected" flag.  */
+  void set_protected ()
+  {
+    /* Can't have both.  */
+    gdb_assert (!m_private);
+    m_protected = true;
+  }
+
+  /* True if this field is 'virtual'.  */
+  bool is_virtual () const
+  { return m_virtual; }
+
+  /* Set the field's "virtual" flag.  */
+  void set_virtual ()
+  { m_virtual = true; }
+
+  /* True if this field is 'ignored'.  */
+  bool is_ignored () const
+  { return m_ignored; }
+
+  /* Set the field's "ignored" flag.  */
+  void set_ignored ()
+  { m_ignored = true; }
+
   union field_location m_loc;
 
   /* * For a function or member type, this is 1 if the argument is
@@ -677,6 +717,15 @@ struct field
 
   unsigned int m_artificial : 1;
 
+  /* Whether the field is 'private'.  */
+  bool m_private : 1;
+  /* Whether the field is 'protected'.  */
+  bool m_protected : 1;
+  /* Whether the field is 'virtual'.  */
+  bool m_virtual : 1;
+  /* Whether the field is 'ignored'.  */
+  bool m_ignored : 1;
+
   /* * Discriminant for union field_location.  */
 
   ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
@@ -1672,42 +1721,6 @@ struct cplus_struct_type
 
     struct type *vptr_basetype;
 
-    /* * For derived classes, the number of base classes is given by
-       n_baseclasses and virtual_field_bits is a bit vector containing
-       one bit per base class.  If the base class is virtual, the
-       corresponding bit will be set.
-       I.E, given:
-
-       class A{};
-       class B{};
-       class C : public B, public virtual A {};
-
-       B is a baseclass of C; A is a virtual baseclass for C.
-       This is a C++ 2.0 language feature.  */
-
-    B_TYPE *virtual_field_bits;
-
-    /* * For classes with private fields, the number of fields is
-       given by nfields and private_field_bits is a bit vector
-       containing one bit per field.
-
-       If the field is private, the corresponding bit will be set.  */
-
-    B_TYPE *private_field_bits;
-
-    /* * For classes with protected fields, the number of fields is
-       given by nfields and protected_field_bits is a bit vector
-       containing one bit per field.
-
-       If the field is private, the corresponding bit will be set.  */
-
-    B_TYPE *protected_field_bits;
-
-    /* * For classes with fields to be ignored, either this is
-       optimized out or this field has length 0.  */
-
-    B_TYPE *ignore_field_bits;
-
     /* * For classes, structures, and unions, a description of each
        field, which consists of an overloaded name, followed by the
        types of arguments that the method expects, and then the name
@@ -1952,37 +1965,16 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_CPLUS_DYNAMIC(thistype) TYPE_CPLUS_SPECIFIC (thistype)->is_dynamic
 
 #define BASETYPE_VIA_VIRTUAL(thistype, index) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (index)))
-
-#define TYPE_FIELD_PRIVATE_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits
-#define TYPE_FIELD_PROTECTED_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits
-#define TYPE_FIELD_IGNORE_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits
-#define TYPE_FIELD_VIRTUAL_BITS(thistype) \
-  TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits
-#define SET_TYPE_FIELD_PRIVATE(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits, (n))
-#define SET_TYPE_FIELD_PROTECTED(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits, (n))
-#define SET_TYPE_FIELD_IGNORE(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits, (n))
-#define SET_TYPE_FIELD_VIRTUAL(thistype, n) \
-  B_SET (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n))
+  ((thistype)->field (index).is_virtual ())
+
 #define TYPE_FIELD_PRIVATE(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->private_field_bits, (n)))
+  ((thistype)->field (n).is_private ())
 #define TYPE_FIELD_PROTECTED(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->protected_field_bits, (n)))
+  ((thistype)->field (n).is_protected ())
 #define TYPE_FIELD_IGNORE(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->ignore_field_bits, (n)))
+  ((thistype)->field (n).is_ignored ())
 #define TYPE_FIELD_VIRTUAL(thistype, n) \
-  (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
-    : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
+  ((thistype)->field (n).is_virtual ())
 
 #define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
 #define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 7402a26a401..9a84e5f3f80 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -65,11 +65,6 @@ struct stabs_nextfield
 {
   struct stabs_nextfield *next;
 
-  /* This is the raw visibility from the stab.  It is not checked
-     for being one of the visibilities we recognize, so code which
-     examines this field better be able to deal.  */
-  int visibility;
-
   struct field field;
 };
 
@@ -2789,7 +2784,7 @@ read_cpp_abbrev (struct stab_field_info *fip, const char **pp,
       }
       /* This field is unpacked.  */
       fip->list->field.set_bitsize (0);
-      fip->list->visibility = VISIBILITY_PRIVATE;
+      fip->list->field.set_private ();
     }
   else
     {
@@ -2814,15 +2809,42 @@ read_one_struct_field (struct stab_field_info *fip, const char **pp,
   *pp = p + 1;
 
   /* This means we have a visibility for a field coming.  */
+  int visibility;
   if (**pp == '/')
     {
       (*pp)++;
-      fip->list->visibility = *(*pp)++;
+      visibility = *(*pp)++;
     }
   else
     {
       /* normal dbx-style format, no explicit visibility */
-      fip->list->visibility = VISIBILITY_PUBLIC;
+      visibility = VISIBILITY_PUBLIC;
+    }
+
+  switch (visibility)
+    {
+    case VISIBILITY_PRIVATE:
+      fip->list->field.set_private ();
+      break;
+
+    case VISIBILITY_PROTECTED:
+      fip->list->field.set_protected ();
+      break;
+
+    case VISIBILITY_IGNORE:
+      fip->list->field.set_ignored ();
+      break;
+
+    case VISIBILITY_PUBLIC:
+      break;
+
+    default:
+      /* Unknown visibility.  Complain and treat it as public.  */
+      {
+	complaint (_("Unknown visibility `%c' for field"),
+		   visibility);
+      }
+      break;
     }
 
   fip->list->field.set_type (read_type (pp, objfile));
@@ -2892,7 +2914,7 @@ read_one_struct_field (struct stab_field_info *fip, const char **pp,
 	 for dbx compatibility.  */
 
       /* Ignore this field.  */
-      fip->list->visibility = VISIBILITY_IGNORE;
+      fip->list->field.set_ignored ();
     }
   else
     {
@@ -3066,21 +3088,6 @@ read_baseclasses (struct stab_field_info *fip, const char **pp,
       return 0;
   }
 
-#if 0
-  /* Some stupid compilers have trouble with the following, so break
-     it up into simpler expressions.  */
-  TYPE_FIELD_VIRTUAL_BITS (type) = (B_TYPE *)
-    TYPE_ZALLOC (type, B_BYTES (TYPE_N_BASECLASSES (type)));
-#else
-  {
-    int num_bytes = B_BYTES (TYPE_N_BASECLASSES (type));
-    char *pointer;
-
-    pointer = (char *) TYPE_ZALLOC (type, num_bytes);
-    TYPE_FIELD_VIRTUAL_BITS (type) = (B_TYPE *) pointer;
-  }
-#endif /* 0 */
-
   for (i = 0; i < TYPE_N_BASECLASSES (type); i++)
     {
       newobj = OBSTACK_ZALLOC (&fip->obstack, struct stabs_nextfield);
@@ -3097,7 +3104,7 @@ read_baseclasses (struct stab_field_info *fip, const char **pp,
 	  /* Nothing to do.  */
 	  break;
 	case '1':
-	  SET_TYPE_FIELD_VIRTUAL (type, i);
+	  newobj->field.set_virtual ();
 	  break;
 	default:
 	  /* Unknown character.  Complain and treat it as non-virtual.  */
@@ -3108,11 +3115,15 @@ read_baseclasses (struct stab_field_info *fip, const char **pp,
 	}
       ++(*pp);
 
-      newobj->visibility = *(*pp)++;
-      switch (newobj->visibility)
+      int visibility = *(*pp)++;
+      switch (visibility)
 	{
 	case VISIBILITY_PRIVATE:
+	  newobj->field.set_private ();
+	  break;
 	case VISIBILITY_PROTECTED:
+	  newobj->field.set_private ();
+	  break;
 	case VISIBILITY_PUBLIC:
 	  break;
 	default:
@@ -3120,8 +3131,7 @@ read_baseclasses (struct stab_field_info *fip, const char **pp,
 	     public.  */
 	  {
 	    complaint (_("Unknown visibility `%c' for baseclass"),
-		       newobj->visibility);
-	    newobj->visibility = VISIBILITY_PUBLIC;
+		       visibility);
 	  }
 	}
 
@@ -3268,43 +3278,19 @@ attach_fields_to_type (struct stab_field_info *fip, struct type *type,
 		       struct objfile *objfile)
 {
   int nfields = 0;
-  int non_public_fields = 0;
   struct stabs_nextfield *scan;
 
-  /* Count up the number of fields that we have, as well as taking note of
-     whether or not there are any non-public fields, which requires us to
-     allocate and build the private_field_bits and protected_field_bits
-     bitfields.  */
+  /* Count up the number of fields that we have.  */
 
   for (scan = fip->list; scan != NULL; scan = scan->next)
-    {
-      nfields++;
-      if (scan->visibility != VISIBILITY_PUBLIC)
-	{
-	  non_public_fields++;
-	}
-    }
+    nfields++;
 
   /* Now we know how many fields there are, and whether or not there are any
      non-public fields.  Record the field count, allocate space for the
-     array of fields, and create blank visibility bitfields if necessary.  */
+     array of fields.  */
 
   type->alloc_fields (nfields);
 
-  if (non_public_fields)
-    {
-      ALLOCATE_CPLUS_STRUCT_TYPE (type);
-
-      TYPE_FIELD_PRIVATE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_PROTECTED_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-
-      TYPE_FIELD_IGNORE_BITS (type) =
-	(B_TYPE *) TYPE_ZALLOC (type, B_BYTES (nfields));
-    }
-
   /* Copy the saved-up fields into the field vector.  Start from the
      head of the list, adding to the tail of the field array, so that
      they end up in the same order in the array in which they were
@@ -3313,31 +3299,6 @@ attach_fields_to_type (struct stab_field_info *fip, struct type *type,
   while (nfields-- > 0)
     {
       type->field (nfields) = fip->list->field;
-      switch (fip->list->visibility)
-	{
-	case VISIBILITY_PRIVATE:
-	  SET_TYPE_FIELD_PRIVATE (type, nfields);
-	  break;
-
-	case VISIBILITY_PROTECTED:
-	  SET_TYPE_FIELD_PROTECTED (type, nfields);
-	  break;
-
-	case VISIBILITY_IGNORE:
-	  SET_TYPE_FIELD_IGNORE (type, nfields);
-	  break;
-
-	case VISIBILITY_PUBLIC:
-	  break;
-
-	default:
-	  /* Unknown visibility.  Complain and treat it as public.  */
-	  {
-	    complaint (_("Unknown visibility `%c' for field"),
-		       fip->list->visibility);
-	  }
-	  break;
-	}
       fip->list = fip->list->next;
     }
   return 1;

-- 
2.40.1

[PATCH 4/7] Add field::is_public

[Tom Tromey]

This adds a field::is_public convenience method, and updates one spot
to use it.
---
 gdb/gdbtypes.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c72512b8204..53a8f7e803c 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -668,6 +668,12 @@ struct field
     m_loc.dwarf_block = dwarf_block;
   }
 
+  /* True if this field is 'public'.  */
+  bool is_public () const
+  {
+    return !m_private && !m_protected;
+  }
+
   /* True if this field is 'private'.  */
   bool is_private () const
   { return m_private; }
@@ -1961,7 +1967,7 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_BASECLASS_NAME(thistype,index) (thistype->field (index).name ())
 #define TYPE_BASECLASS_BITPOS(thistype,index) (thistype->field (index).loc_bitpos ())
 #define BASETYPE_VIA_PUBLIC(thistype, index) \
-  ((!TYPE_FIELD_PRIVATE(thistype, index)) && (!TYPE_FIELD_PROTECTED(thistype, index)))
+  ((thistype)->field (index).is_public ())
 #define TYPE_CPLUS_DYNAMIC(thistype) TYPE_CPLUS_SPECIFIC (thistype)->is_dynamic
 
 #define BASETYPE_VIA_VIRTUAL(thistype, index) \

-- 
2.40.1

[PATCH 5/7] Remove some QUIT calls from need_access_label_p

[Tom Tromey]

I think these invocations of QUIT in need_access_label_p are overkill.
QUIT is already called from its caller.  This just removes them.
---
 gdb/c-typeprint.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index e45098268c0..241fbca49b7 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -911,31 +911,25 @@ need_access_label_p (struct type *type)
 {
   if (type->is_declared_class ())
     {
-      QUIT;
       for (int i = TYPE_N_BASECLASSES (type); i < type->num_fields (); i++)
 	if (!TYPE_FIELD_PRIVATE (type, i))
 	  return true;
-      QUIT;
       for (int j = 0; j < TYPE_NFN_FIELDS (type); j++)
 	for (int i = 0; i < TYPE_FN_FIELDLIST_LENGTH (type, j); i++)
 	  if (!TYPE_FN_FIELD_PRIVATE (TYPE_FN_FIELDLIST1 (type,
 							  j), i))
 	    return true;
-      QUIT;
       for (int i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
 	if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
 	  return true;
     }
   else
     {
-      QUIT;
       for (int i = TYPE_N_BASECLASSES (type); i < type->num_fields (); i++)
 	if (TYPE_FIELD_PRIVATE (type, i) || TYPE_FIELD_PROTECTED (type, i))
 	  return true;
-      QUIT;
       for (int j = 0; j < TYPE_NFN_FIELDS (type); j++)
 	{
-	  QUIT;
 	  for (int i = 0; i < TYPE_FN_FIELDLIST_LENGTH (type, j); i++)
 	    if (TYPE_FN_FIELD_PROTECTED (TYPE_FN_FIELDLIST1 (type,
 							     j), i)
@@ -944,7 +938,6 @@ need_access_label_p (struct type *type)
 					  i))
 	      return true;
 	}
-      QUIT;
       for (int i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
 	if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i)
 	    || TYPE_TYPEDEF_FIELD_PRIVATE (type, i))

-- 
2.40.1

[PATCH 6/7] Remove some type field accessor macros

[Tom Tromey]

This removes TYPE_FIELD_PRIVATE, TYPE_FIELD_PROTECTED,
TYPE_FIELD_IGNORE, and TYPE_FIELD_VIRTUAL.
---
 gdb/ada-valprint.c                |  2 +-
 gdb/c-typeprint.c                 | 10 +++++-----
 gdb/c-varobj.c                    | 16 +++++++++-------
 gdb/compile/compile-cplus-types.c |  7 ++++---
 gdb/cp-valprint.c                 |  4 ++--
 gdb/gdbtypes.c                    | 27 ++++++++++++++-------------
 gdb/gdbtypes.h                    |  9 ---------
 gdb/p-typeprint.c                 |  6 ++++--
 gdb/p-valprint.c                  |  4 ++--
 9 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index eaeca0f6516..686655b18c9 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -619,7 +619,7 @@ print_field_values (struct value *value, struct value *outer_value,
 	{
 	  /* Bitfields require special handling, especially due to byte
 	     order problems.  */
-	  if (HAVE_CPLUS_STRUCT (type) && TYPE_FIELD_IGNORE (type, i))
+	  if (type->field (i).is_ignored ())
 	    {
 	      fputs_styled (_("<optimized out or zero length>"),
 			    metadata_style.style (), stream);
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 241fbca49b7..4a0c6950b0c 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -238,7 +238,7 @@ cp_type_print_derivation_info (struct ui_file *stream,
       gdb_puts (i == 0 ? ": " : ", ", stream);
       gdb_printf (stream, "%s%s ",
 		  BASETYPE_VIA_PUBLIC (type, i)
-		  ? "public" : (TYPE_FIELD_PROTECTED (type, i)
+		  ? "public" : (type->field (i).is_protected ()
 				? "protected" : "private"),
 		  BASETYPE_VIA_VIRTUAL (type, i) ? " virtual" : "");
       name = TYPE_BASECLASS (type, i)->name ();
@@ -912,7 +912,7 @@ need_access_label_p (struct type *type)
   if (type->is_declared_class ())
     {
       for (int i = TYPE_N_BASECLASSES (type); i < type->num_fields (); i++)
-	if (!TYPE_FIELD_PRIVATE (type, i))
+	if (!type->field (i).is_private ())
 	  return true;
       for (int j = 0; j < TYPE_NFN_FIELDS (type); j++)
 	for (int i = 0; i < TYPE_FN_FIELDLIST_LENGTH (type, j); i++)
@@ -926,7 +926,7 @@ need_access_label_p (struct type *type)
   else
     {
       for (int i = TYPE_N_BASECLASSES (type); i < type->num_fields (); i++)
-	if (TYPE_FIELD_PRIVATE (type, i) || TYPE_FIELD_PROTECTED (type, i))
+	if (!type->field (i).is_public ())
 	  return true;
       for (int j = 0; j < TYPE_NFN_FIELDS (type); j++)
 	{
@@ -1102,8 +1102,8 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 	    {
 	      section_type = output_access_specifier
 		(stream, section_type, level,
-		 TYPE_FIELD_PROTECTED (type, i),
-		 TYPE_FIELD_PRIVATE (type, i), flags);
+		 type->field (i).is_protected (),
+		 type->field (i).is_private (), flags);
 	    }
 
 	  bool is_static = type->field (i).is_static ();
diff --git a/gdb/c-varobj.c b/gdb/c-varobj.c
index b00a2345e2c..10edc37c2ea 100644
--- a/gdb/c-varobj.c
+++ b/gdb/c-varobj.c
@@ -647,16 +647,18 @@ cplus_class_num_children (struct type *type, int children[3])
   vptr_fieldno = get_vptr_fieldno (type, &basetype);
   for (i = TYPE_N_BASECLASSES (type); i < type->num_fields (); i++)
     {
+      field &fld = type->field (i);
+
       /* If we have a virtual table pointer, omit it.  Even if virtual
 	 table pointers are not specifically marked in the debug info,
 	 they should be artificial.  */
       if ((type == basetype && i == vptr_fieldno)
-	  || type->field (i).is_artificial ())
+	  || fld.is_artificial ())
 	continue;
 
-      if (TYPE_FIELD_PROTECTED (type, i))
+      if (fld.is_protected ())
 	children[v_protected]++;
-      else if (TYPE_FIELD_PRIVATE (type, i))
+      else if (fld.is_private ())
 	children[v_private]++;
       else
 	children[v_public]++;
@@ -677,12 +679,12 @@ enum accessibility { private_field, protected_field, public_field };
 static int 
 match_accessibility (struct type *type, int index, enum accessibility acc)
 {
-  if (acc == private_field && TYPE_FIELD_PRIVATE (type, index))
+  field &fld = type->field (index);
+  if (acc == private_field && fld.is_private ())
     return 1;
-  else if (acc == protected_field && TYPE_FIELD_PROTECTED (type, index))
+  else if (acc == protected_field && fld.is_protected ())
     return 1;
-  else if (acc == public_field && !TYPE_FIELD_PRIVATE (type, index)
-	   && !TYPE_FIELD_PROTECTED (type, index))
+  else if (acc == public_field && fld.is_public ())
     return 1;
   else
     return 0;
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index ac27e83618b..a59d77b76a9 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -73,9 +73,10 @@ compile_cplus_instance::decl_name (const char *natural)
 static enum gcc_cp_symbol_kind
 get_field_access_flag (const struct type *type, int num)
 {
-  if (TYPE_FIELD_PROTECTED (type, num))
+  field &fld = type->field (num);
+  if (fld.is_protected ())
     return GCC_CP_ACCESS_PROTECTED;
-  else if (TYPE_FIELD_PRIVATE (type, num))
+  else if (fld.is_private ())
     return GCC_CP_ACCESS_PRIVATE;
 
   /* GDB assumes everything else is public.  */
@@ -583,7 +584,7 @@ compile_cplus_convert_struct_or_union_members
     {
       const char *field_name = type->field (i).name ();
 
-      if (TYPE_FIELD_IGNORE (type, i)
+      if (type->field (i).is_ignored ()
 	  || type->field (i).is_artificial ())
 	continue;
 
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 820a761054a..a803c786d2b 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -265,7 +265,7 @@ cp_print_value_fields (struct value *val, struct ui_file *stream,
 
 	      /* Bitfields require special handling, especially due to
 		 byte order problems.  */
-	      if (TYPE_FIELD_IGNORE (type, i))
+	      if (type->field (i).is_ignored ())
 		{
 		  fputs_styled ("<optimized out or zero length>",
 				metadata_style.style (), stream);
@@ -290,7 +290,7 @@ cp_print_value_fields (struct value *val, struct ui_file *stream,
 	    }
 	  else
 	    {
-	      if (TYPE_FIELD_IGNORE (type, i))
+	      if (type->field (i).is_ignored ())
 		{
 		  fputs_styled ("<optimized out or zero length>",
 				metadata_style.style (), stream);
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 55d12218b4e..e9c84004026 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5229,31 +5229,32 @@ recursive_dump_type (struct type *type, int spaces)
   gdb_printf ("%s\n", host_address_to_string (type->fields ()));
   for (idx = 0; idx < type->num_fields (); idx++)
     {
+      field &fld = type->field (idx);
       if (type->code () == TYPE_CODE_ENUM)
 	gdb_printf ("%*s[%d] enumval %s type ", spaces + 2, "",
-		    idx, plongest (type->field (idx).loc_enumval ()));
+		    idx, plongest (fld.loc_enumval ()));
       else
 	gdb_printf ("%*s[%d] bitpos %s bitsize %d type ", spaces + 2, "",
-		    idx, plongest (type->field (idx).loc_bitpos ()),
-		    type->field (idx).bitsize ());
+		    idx, plongest (fld.loc_bitpos ()),
+		    fld.bitsize ());
       gdb_printf ("%s name '%s' (%s)",
-		  host_address_to_string (type->field (idx).type ()),
-		  type->field (idx).name () != NULL
-		  ? type->field (idx).name ()
+		  host_address_to_string (fld.type ()),
+		  fld.name () != NULL
+		  ? fld.name ()
 		  : "<NULL>",
-		  host_address_to_string (type->field (idx).name ()));
-      if (TYPE_FIELD_VIRTUAL (type, idx))
+		  host_address_to_string (fld.name ()));
+      if (fld.is_virtual ())
 	gdb_printf (" virtual");
-      if (TYPE_FIELD_PRIVATE (type, idx))
+      if (fld.is_private ())
 	gdb_printf (" private");
-      else if (TYPE_FIELD_PROTECTED (type, idx))
+      else if (fld.is_protected ())
 	gdb_printf (" protected");
-      else if (TYPE_FIELD_IGNORE (type, idx))
+      else if (fld.is_ignored ())
 	gdb_printf (" ignored");
       gdb_printf ("\n");
-      if (type->field (idx).type () != NULL)
+      if (fld.type () != NULL)
 	{
-	  recursive_dump_type (type->field (idx).type (), spaces + 4);
+	  recursive_dump_type (fld.type (), spaces + 4);
 	}
     }
   if (type->code () == TYPE_CODE_RANGE)
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 53a8f7e803c..31745f28156 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1973,15 +1973,6 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define BASETYPE_VIA_VIRTUAL(thistype, index) \
   ((thistype)->field (index).is_virtual ())
 
-#define TYPE_FIELD_PRIVATE(thistype, n) \
-  ((thistype)->field (n).is_private ())
-#define TYPE_FIELD_PROTECTED(thistype, n) \
-  ((thistype)->field (n).is_protected ())
-#define TYPE_FIELD_IGNORE(thistype, n) \
-  ((thistype)->field (n).is_ignored ())
-#define TYPE_FIELD_VIRTUAL(thistype, n) \
-  ((thistype)->field (n).is_virtual ())
-
 #define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
 #define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
 #define TYPE_FN_FIELDLIST1(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n].fn_fields
diff --git a/gdb/p-typeprint.c b/gdb/p-typeprint.c
index 41058a8b59e..b5d66269d6e 100644
--- a/gdb/p-typeprint.c
+++ b/gdb/p-typeprint.c
@@ -486,7 +486,9 @@ pascal_language::type_print_base (struct type *type, struct ui_file *stream, int
 
 	      if (HAVE_CPLUS_STRUCT (type))
 		{
-		  if (TYPE_FIELD_PROTECTED (type, i))
+		  field &fld = type->field (i);
+
+		  if (fld.is_protected ())
 		    {
 		      if (section_type != s_protected)
 			{
@@ -495,7 +497,7 @@ pascal_language::type_print_base (struct type *type, struct ui_file *stream, int
 				      level + 2, "");
 			}
 		    }
-		  else if (TYPE_FIELD_PRIVATE (type, i))
+		  else if (fld.is_private ())
 		    {
 		      if (section_type != s_private)
 			{
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index fb9386293a6..05601622560 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -604,7 +604,7 @@ pascal_object_print_value_fields (struct value *val, struct ui_file *stream,
 
 	      /* Bitfields require special handling, especially due to byte
 		 order problems.  */
-	      if (TYPE_FIELD_IGNORE (type, i))
+	      if (type->field (i).is_ignored ())
 		{
 		  fputs_styled ("<optimized out or zero length>",
 				metadata_style.style (), stream);
@@ -629,7 +629,7 @@ pascal_object_print_value_fields (struct value *val, struct ui_file *stream,
 	    }
 	  else
 	    {
-	      if (TYPE_FIELD_IGNORE (type, i))
+	      if (type->field (i).is_ignored ())
 		{
 		  fputs_styled ("<optimized out or zero length>",
 				metadata_style.style (), stream);

-- 
2.40.1

[PATCH 7/7] Remove char-based bitfield macros

[Tom Tromey]

This removes the char-based bitfield macros from gdbtypes.h, as they
are no longer used.
---
 gdb/gdbtypes.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 31745f28156..e797455dde0 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -66,15 +66,6 @@ struct dwarf2_per_cu_data;
 struct dwarf2_per_objfile;
 struct dwarf2_property_baton;
 
-/* Some macros for char-based bitfields.  */
-
-#define B_SET(a,x)	((a)[(x)>>3] |= (1 << ((x)&7)))
-#define B_CLR(a,x)	((a)[(x)>>3] &= ~(1 << ((x)&7)))
-#define B_TST(a,x)	((a)[(x)>>3] & (1 << ((x)&7)))
-#define B_TYPE		unsigned char
-#define	B_BYTES(x)	( 1 + ((x)>>3) )
-#define	B_CLRALL(a,x)	memset ((a), 0, B_BYTES(x))
-
 /* * Different kinds of data types are distinguished by the `code'
    field.  */
 

-- 
2.40.1

Re: [PATCH 1/7] Use .def file to stringify type codes

[Lancelot SIX]

Hi Tom,

On Thu, Sep 21, 2023 at 12:01:28PM -0600, Tom Tromey via Gdb-patches wrote:
> This changes recursive_dump_type to reuse the type-codes.def file when
> stringifying type codes.
> ---
>  gdb/gdbtypes.c | 99 +++++++++++-----------------------------------------------
>  1 file changed, 18 insertions(+), 81 deletions(-)
> 
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 7011fddd695..18aa8e5c29e 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5099,6 +5099,23 @@ dump_dynamic_prop (dynamic_prop const& prop)
>      }
>  }
>  
> +/* Return a string that represents a type code.  */
> +static const char *
> +type_code_name (type_code code)
> +{
> +  switch (code)
> +    {
> +#define OP(X) case X: return # X;
> +#include "type-codes.def"
> +#undef OP
> +
> +    case TYPE_CODE_UNDEF:
> +      return "TYPE_CODE_UNDEF";
> +    default:
> +      return "UNKNOWN TYPE CODE";

Is this default "just" to make the compiler happy?  I don't expect it
should ever executed.  If so, I usually prefer the following construct:

    static const char *
    foo (some_enume e)
    {
      switch (e)
        {
        case A:
          return "A";
        case B:
          return "B";
        [...]
        /* No "default:".  */
        }
      return "unknown";
    }

The return after the switch block avoids the "control reaches end of
non-void function" error, but since there is no "default:" the compiler
can warn if some enumerator is not covered by the switch.

That being said, given how type_code is constructed, the
"#include type-codes.def" make it really unlikely  that type_code_name
can evolve without having this function evolve as well.

Best,
Lancelot.

> +    }
> +}
> +
>  void
>  recursive_dump_type (struct type *type, int spaces)
>  {
> @@ -5136,87 +5153,7 @@ recursive_dump_type (struct type *type, int spaces)
>  	      type->name () ? type->name () : "<NULL>",
>  	      host_address_to_string (type->name ()));
>    gdb_printf ("%*scode 0x%x ", spaces, "", type->code ());
> -  switch (type->code ())
> -    {
> -    case TYPE_CODE_UNDEF:
> -      gdb_printf ("(TYPE_CODE_UNDEF)");
> -      break;
> -    case TYPE_CODE_PTR:
> -      gdb_printf ("(TYPE_CODE_PTR)");
> -      break;
> -    case TYPE_CODE_ARRAY:
> -      gdb_printf ("(TYPE_CODE_ARRAY)");
> -      break;
> -    case TYPE_CODE_STRUCT:
> -      gdb_printf ("(TYPE_CODE_STRUCT)");
> -      break;
> -    case TYPE_CODE_UNION:
> -      gdb_printf ("(TYPE_CODE_UNION)");
> -      break;
> -    case TYPE_CODE_ENUM:
> -      gdb_printf ("(TYPE_CODE_ENUM)");
> -      break;
> -    case TYPE_CODE_FLAGS:
> -      gdb_printf ("(TYPE_CODE_FLAGS)");
> -      break;
> -    case TYPE_CODE_FUNC:
> -      gdb_printf ("(TYPE_CODE_FUNC)");
> -      break;
> -    case TYPE_CODE_INT:
> -      gdb_printf ("(TYPE_CODE_INT)");
> -      break;
> -    case TYPE_CODE_FLT:
> -      gdb_printf ("(TYPE_CODE_FLT)");
> -      break;
> -    case TYPE_CODE_VOID:
> -      gdb_printf ("(TYPE_CODE_VOID)");
> -      break;
> -    case TYPE_CODE_SET:
> -      gdb_printf ("(TYPE_CODE_SET)");
> -      break;
> -    case TYPE_CODE_RANGE:
> -      gdb_printf ("(TYPE_CODE_RANGE)");
> -      break;
> -    case TYPE_CODE_STRING:
> -      gdb_printf ("(TYPE_CODE_STRING)");
> -      break;
> -    case TYPE_CODE_ERROR:
> -      gdb_printf ("(TYPE_CODE_ERROR)");
> -      break;
> -    case TYPE_CODE_MEMBERPTR:
> -      gdb_printf ("(TYPE_CODE_MEMBERPTR)");
> -      break;
> -    case TYPE_CODE_METHODPTR:
> -      gdb_printf ("(TYPE_CODE_METHODPTR)");
> -      break;
> -    case TYPE_CODE_METHOD:
> -      gdb_printf ("(TYPE_CODE_METHOD)");
> -      break;
> -    case TYPE_CODE_REF:
> -      gdb_printf ("(TYPE_CODE_REF)");
> -      break;
> -    case TYPE_CODE_CHAR:
> -      gdb_printf ("(TYPE_CODE_CHAR)");
> -      break;
> -    case TYPE_CODE_BOOL:
> -      gdb_printf ("(TYPE_CODE_BOOL)");
> -      break;
> -    case TYPE_CODE_COMPLEX:
> -      gdb_printf ("(TYPE_CODE_COMPLEX)");
> -      break;
> -    case TYPE_CODE_TYPEDEF:
> -      gdb_printf ("(TYPE_CODE_TYPEDEF)");
> -      break;
> -    case TYPE_CODE_NAMESPACE:
> -      gdb_printf ("(TYPE_CODE_NAMESPACE)");
> -      break;
> -    case TYPE_CODE_FIXED_POINT:
> -      gdb_printf ("(TYPE_CODE_FIXED_POINT)");
> -      break;
> -    default:
> -      gdb_printf ("(UNKNOWN TYPE CODE)");
> -      break;
> -    }
> +  gdb_printf ("(%s)", type_code_name (type->code ()));
>    gdb_puts ("\n");
>    gdb_printf ("%*slength %s\n", spaces, "",
>  	      pulongest (type->length ()));
> 
> -- 
> 2.40.1
> 

Re: [PATCH 3/7] Remove byte vectors from cplus_struct_type

[Lancelot SIX]

Hi Tom

On Thu, Sep 21, 2023 at 12:01:30PM -0600, Tom Tromey via Gdb-patches wrote:
> This removes some byte vectors from cplus_struct_type, moving the
> information into bitfields in holes in struct field.
> ---
>  gdb/dwarf2/read.c |  86 +++++++-------------------------------
>  gdb/gdbtypes.c    |  53 -----------------------
>  gdb/gdbtypes.h    | 118 ++++++++++++++++++++++++---------------------------
>  gdb/stabsread.c   | 123 +++++++++++++++++++-----------------------------------
>  4 files changed, 113 insertions(+), 267 deletions(-)
> 
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 505c8ba12b5..c72512b8204 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -668,6 +668,46 @@ struct field
>      m_loc.dwarf_block = dwarf_block;
>    }
>  
> +  /* True if this field is 'private'.  */
> +  bool is_private () const
> +  { return m_private; }
> +
> +  /* Set the field's "private" flag.  */
> +  void set_private ()
> +  {
> +    /* Can't have both.  */
> +    gdb_assert (!m_protected);
> +    m_private = true;
> +  }
> +
> +  /* True if this field is 'protected'.  */
> +  bool is_protected () const
> +  { return m_protected; }
> +
> +  /* Set the field's "protected" flag.  */
> +  void set_protected ()
> +  {
> +    /* Can't have both.  */
> +    gdb_assert (!m_private);
> +    m_protected = true;
> +  }
> +
> +  /* True if this field is 'virtual'.  */
> +  bool is_virtual () const
> +  { return m_virtual; }
> +
> +  /* Set the field's "virtual" flag.  */
> +  void set_virtual ()
> +  { m_virtual = true; }
> +
> +  /* True if this field is 'ignored'.  */
> +  bool is_ignored () const
> +  { return m_ignored; }
> +
> +  /* Set the field's "ignored" flag.  */
> +  void set_ignored ()
> +  { m_ignored = true; }
> +
>    union field_location m_loc;
>  
>    /* * For a function or member type, this is 1 if the argument is
> @@ -677,6 +717,15 @@ struct field
>  
>    unsigned int m_artificial : 1;
>  
> +  /* Whether the field is 'private'.  */
> +  bool m_private : 1;
> +  /* Whether the field is 'protected'.  */
> +  bool m_protected : 1;

Is there a reason I miss to not use an enum to describe the visibility?
This would avoid having to verify that m_private and m_protected are
mutually exclusive.

Is seems to me that something similar to

    debug_visibility m_visibility : 2;

(or a custom enum) should work.

> +  /* Whether the field is 'virtual'.  */
> +  bool m_virtual : 1;
> +  /* Whether the field is 'ignored'.  */
> +  bool m_ignored : 1;
> +
>    /* * Discriminant for union field_location.  */
>  
>    ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c
> index 7402a26a401..9a84e5f3f80 100644
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
> @@ -3108,11 +3115,15 @@ read_baseclasses (struct stab_field_info *fip, const char **pp,
>  	}
>        ++(*pp);
>  
> -      newobj->visibility = *(*pp)++;
> -      switch (newobj->visibility)
> +      int visibility = *(*pp)++;
> +      switch (visibility)
>  	{
>  	case VISIBILITY_PRIVATE:
> +	  newobj->field.set_private ();
> +	  break;
>  	case VISIBILITY_PROTECTED:
> +	  newobj->field.set_private ();

I think this should be:

  newobj->field.set_protected ();

> +	  break;
>  	case VISIBILITY_PUBLIC:
>  	  break;
>  	default:

Best,
Lancelot.

Re: [PATCH 1/7] Use .def file to stringify type codes

[Tom Tromey]

>>>>> "Lancelot" == Lancelot SIX <lsix@lancelotsix.com> writes:

>> +    default:
>> +      return "UNKNOWN TYPE CODE";

Lancelot> Is this default "just" to make the compiler happy?  I don't expect it
Lancelot> should ever executed.  If so, I usually prefer the following construct:

It's just to avoid any semantic change.
I doubt it can happen -- it certainly isn't supposed to happen.

Lancelot> The return after the switch block avoids the "control reaches end of
Lancelot> non-void function" error, but since there is no "default:" the compiler
Lancelot> can warn if some enumerator is not covered by the switch.

I'll do it.

Tom

Re: [PATCH 3/7] Remove byte vectors from cplus_struct_type

[Tom Tromey]

>>>>> "Lancelot" == Lancelot SIX <lsix@lancelotsix.com> writes:

>> +  /* Whether the field is 'private'.  */
>> +  bool m_private : 1;
>> +  /* Whether the field is 'protected'.  */
>> +  bool m_protected : 1;

Lancelot> Is there a reason I miss to not use an enum to describe the visibility?
Lancelot> This would avoid having to verify that m_private and m_protected are
Lancelot> mutually exclusive.

I changed this in v2.

>> case VISIBILITY_PROTECTED:
>> +	  newobj->field.set_private ();

Lancelot> I think this should be:

newobj-> field.set_protected ();

I fixed this too.

Tom