public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/9] Remove char-based bitfield macros
@ 2023-11-03 16:09 Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 1/9] Use .def file to stringify type codes Tom Tromey
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

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.

---
Changes in v3:
- Updated per review comments
- Link to v2: https://inbox.sourceware.org/gdb-patches/20231027-field-bits-v2-0-cbec64f2136a@adacore.com

Changes in v2:
- Introduced accessibility enum per review
- Added two new simplification patches
- Link to v1: https://inbox.sourceware.org/gdb-patches/20230921-field-bits-v1-0-201285360900@adacore.com

---
Tom Tromey (9):
      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
      Use enum accessibility in types and member functions
      Simplify C++ type-printing

 gdb/ada-valprint.c                       |   2 +-
 gdb/c-typeprint.c                        | 145 ++++++------------------
 gdb/c-varobj.c                           |  39 ++-----
 gdb/compile/compile-cplus-types.c        |   7 +-
 gdb/cp-valprint.c                        |   4 +-
 gdb/dwarf2/read.c                        |  94 ++++------------
 gdb/gdbtypes.c                           | 184 +++++++------------------------
 gdb/gdbtypes.h                           | 162 ++++++++++++---------------
 gdb/p-typeprint.c                        |   6 +-
 gdb/p-valprint.c                         |   4 +-
 gdb/stabsread.c                          | 127 ++++++++-------------
 gdb/testsuite/gdb.base/ptype-offsets.exp |   6 -
 12 files changed, 231 insertions(+), 549 deletions(-)
---
base-commit: 88bfe6ac8bcbaf1eb0c1e4be02c21a5c048b7335
change-id: 20230921-field-bits-9b9f802eb42b

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


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

* [PATCH v3 1/9] Use .def file to stringify type codes
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 2/9] Print field accessibility inline Tom Tromey
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This changes recursive_dump_type to reuse the type-codes.def file when
stringifying type codes.

This version of the patch changes the "unknown" case to an assert.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/gdbtypes.c | 99 +++++++++++-----------------------------------------------
 1 file changed, 18 insertions(+), 81 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 7011fddd695..a585674ebe4 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";
+    }
+
+  gdb_assert_not_reached ("unhandled 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


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

* [PATCH v3 2/9] Print field accessibility inline
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 1/9] Use .def file to stringify type codes Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This changes recursive_dump_type to print field accessibility
information "inline".  This is clearer and preserves the information
when the byte vectors are removed.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/gdbtypes.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index a585674ebe4..2cd74a2fb29 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5289,12 +5289,23 @@ 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


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

* [PATCH v3 3/9] Remove byte vectors from cplus_struct_type
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 1/9] Use .def file to stringify type codes Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 2/9] Print field accessibility inline Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 4/9] Add field::is_public Tom Tromey
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This removes some byte vectors from cplus_struct_type, moving the
information into bitfields in holes in struct field.

A new 'enum accessibility' is added to hold some of this information.
A similar enum is removed from c-varobj.c.

Note that the stabs reader treats "ignored" as an accessibility.
However, the stabs texinfo documents this as a public field that is
optimized out -- unfortunately nobody has updated the stabs reader to
use the better value-based optimized-out machinery.  I looked and
apparently gcc never emitted this visibility value, so whatever
compiler generated this stab is unknown.  I left a comment in
gdbtypes.h to this effect.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/c-varobj.c    |  14 +++----
 gdb/dwarf2/read.c |  86 +++++++-------------------------------
 gdb/gdbtypes.c    |  53 -----------------------
 gdb/gdbtypes.h    | 121 +++++++++++++++++++++++++----------------------------
 gdb/stabsread.c   | 123 +++++++++++++++++++-----------------------------------
 5 files changed, 122 insertions(+), 275 deletions(-)

diff --git a/gdb/c-varobj.c b/gdb/c-varobj.c
index b00a2345e2c..543cd585eb2 100644
--- a/gdb/c-varobj.c
+++ b/gdb/c-varobj.c
@@ -669,19 +669,17 @@ cplus_name_of_variable (const struct varobj *parent)
   return c_name_of_variable (parent);
 }
 
-enum accessibility { private_field, protected_field, public_field };
-
 /* Check if field INDEX of TYPE has the specified accessibility.
    Return 0 if so and 1 otherwise.  */
 
 static int 
 match_accessibility (struct type *type, int index, enum accessibility acc)
 {
-  if (acc == private_field && TYPE_FIELD_PRIVATE (type, index))
+  if (acc == accessibility::PRIVATE && TYPE_FIELD_PRIVATE (type, index))
     return 1;
-  else if (acc == protected_field && TYPE_FIELD_PROTECTED (type, index))
+  else if (acc == accessibility::PROTECTED && TYPE_FIELD_PROTECTED (type, index))
     return 1;
-  else if (acc == public_field && !TYPE_FIELD_PRIVATE (type, index)
+  else if (acc == accessibility::PUBLIC && !TYPE_FIELD_PRIVATE (type, index)
 	   && !TYPE_FIELD_PROTECTED (type, index))
     return 1;
   else
@@ -737,16 +735,16 @@ cplus_describe_child (const struct varobj *parent, int index,
 	     have the access control we are looking for to properly
 	     find the indexed field.  */
 	  int type_index = TYPE_N_BASECLASSES (type);
-	  enum accessibility acc = public_field;
+	  enum accessibility acc = accessibility::PUBLIC;
 	  int vptr_fieldno;
 	  struct type *basetype = NULL;
 	  const char *field_name;
 
 	  vptr_fieldno = get_vptr_fieldno (type, &basetype);
 	  if (parent->name == "private")
-	    acc = private_field;
+	    acc = accessibility::PRIVATE;
 	  else if (parent->name == "protected")
-	    acc = protected_field;
+	    acc = accessibility::PROTECTED;
 
 	  while (index >= 0)
 	    {
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c4948164fb1..e52ca5b5b83 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -673,8 +673,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.  */
@@ -697,9 +695,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.  */
@@ -11514,15 +11509,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_accessibility (accessibility::PRIVATE);
+      break;
+    case DW_ACCESS_protected:
+      new_field->field.set_accessibility (accessibility::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;
 
@@ -11616,8 +11619,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_accessibility (accessibility::PRIVATE);
 	}
     }
   else if (die->tag == DW_TAG_member || die->tag == DW_TAG_variable)
@@ -11931,30 +11933,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 ();
     }
 
@@ -11969,41 +11950,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 2cd74a2fb29..e3a12a45afd 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..f5dd2b8076d 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -542,6 +542,16 @@ union field_location
   struct dwarf2_locexpr_baton *dwarf_block;
 };
 
+/* Accessibility of a member.  */
+enum class accessibility : unsigned
+{
+  /* It's important that this be 0 so that fields default to
+     public.  */
+  PUBLIC = 0,
+  PROTECTED = 1,
+  PRIVATE = 2,
+};
+
 struct field
 {
   struct type *type () const
@@ -668,6 +678,41 @@ struct field
     m_loc.dwarf_block = dwarf_block;
   }
 
+  /* Set the field's accessibility.  */
+  void set_accessibility (accessibility acc)
+  { m_accessibility = acc; }
+
+  /* Fetch the field's accessibility.  */
+  enum accessibility accessibility () const
+  { return m_accessibility; }
+
+  /* True if this field is 'private'.  */
+  bool is_private () const
+  { return m_accessibility == accessibility::PRIVATE; }
+
+  /* True if this field is 'protected'.  */
+  bool is_protected () const
+  { return m_accessibility == accessibility::PROTECTED; }
+
+  /* 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.  Note that the 'ignored' bit is
+     deprecated.  It was used by some unknown stabs generator, and has
+     been replaced by the optimized-out approach -- however, it
+     remains because the stabs reader was never updated.  */
+  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 +722,13 @@ struct field
 
   unsigned int m_artificial : 1;
 
+  /* Accessibility of the field.  */
+  ENUM_BITFIELD (accessibility) m_accessibility : 2;
+  /* 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 +1724,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 +1968,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..334371c2e46 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_accessibility (accessibility::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_accessibility (accessibility::PRIVATE);
+      break;
+
+    case VISIBILITY_PROTECTED:
+      fip->list->field.set_accessibility (accessibility::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_accessibility (accessibility::PRIVATE);
+	  break;
 	case VISIBILITY_PROTECTED:
+	  newobj->field.set_accessibility (accessibility::PROTECTED);
+	  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


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

* [PATCH v3 4/9] Add field::is_public
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (2 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 5/9] Remove some QUIT calls from need_access_label_p Tom Tromey
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This adds a field::is_public convenience method, and updates one spot
to use it.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/gdbtypes.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index f5dd2b8076d..b6a693f1316 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -686,6 +686,10 @@ struct field
   enum accessibility accessibility () const
   { return m_accessibility; }
 
+  /* True if this field is 'public'.  */
+  bool is_public () const
+  { return m_accessibility == accessibility::PUBLIC; }
+
   /* True if this field is 'private'.  */
   bool is_private () const
   { return m_accessibility == accessibility::PRIVATE; }
@@ -1964,7 +1968,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


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

* [PATCH v3 5/9] Remove some QUIT calls from need_access_label_p
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (3 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 4/9] Add field::is_public Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 6/9] Remove some type field accessor macros Tom Tromey
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

I think these invocations of QUIT in need_access_label_p are overkill.
QUIT is already called from its caller.  This just removes them.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 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


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

* [PATCH v3 6/9] Remove some type field accessor macros
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (4 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 5/9] Remove some QUIT calls from need_access_label_p Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 7/9] Remove char-based bitfield macros Tom Tromey
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This removes TYPE_FIELD_PRIVATE, TYPE_FIELD_PROTECTED,
TYPE_FIELD_IGNORE, and TYPE_FIELD_VIRTUAL.

In c-varobj.c, match_accessibility can be removed entirely now.  Note
that the comment before this function was incorrect.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/ada-valprint.c                |  2 +-
 gdb/c-typeprint.c                 | 10 +++++-----
 gdb/c-varobj.c                    | 31 ++++++++-----------------------
 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, 40 insertions(+), 60 deletions(-)

diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index b32f1e506d1..b981be89b18 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -627,7 +627,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 543cd585eb2..05b5bd7fa23 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]++;
@@ -669,23 +671,6 @@ cplus_name_of_variable (const struct varobj *parent)
   return c_name_of_variable (parent);
 }
 
-/* Check if field INDEX of TYPE has the specified accessibility.
-   Return 0 if so and 1 otherwise.  */
-
-static int 
-match_accessibility (struct type *type, int index, enum accessibility acc)
-{
-  if (acc == accessibility::PRIVATE && TYPE_FIELD_PRIVATE (type, index))
-    return 1;
-  else if (acc == accessibility::PROTECTED && TYPE_FIELD_PROTECTED (type, index))
-    return 1;
-  else if (acc == accessibility::PUBLIC && !TYPE_FIELD_PRIVATE (type, index)
-	   && !TYPE_FIELD_PROTECTED (type, index))
-    return 1;
-  else
-    return 0;
-}
-
 static void
 cplus_describe_child (const struct varobj *parent, int index,
 		      std::string *cname, struct value **cvalue, struct type **ctype,
@@ -751,9 +736,9 @@ cplus_describe_child (const struct varobj *parent, int index,
 	      if ((type == basetype && type_index == vptr_fieldno)
 		  || type->field (type_index).is_artificial ())
 		; /* ignore vptr */
-	      else if (match_accessibility (type, type_index, acc))
-		    --index;
-		  ++type_index;
+	      else if (type->field (type_index).accessibility () == acc)
+		--index;
+	      ++type_index;
 	    }
 	  --type_index;
 
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 e3a12a45afd..d40de91001f 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5229,33 +5229,34 @@ 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 b6a693f1316..6c011a56dd8 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1974,15 +1974,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


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

* [PATCH v3 7/9] Remove char-based bitfield macros
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (5 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 6/9] Remove some type field accessor macros Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-03 16:09 ` [PATCH v3 8/9] Use enum accessibility in types and member functions Tom Tromey
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This removes the char-based bitfield macros from gdbtypes.h, as they
are no longer used.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/gdbtypes.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 6c011a56dd8..bec69bdc3d1 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


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

* [PATCH v3 8/9] Use enum accessibility in types and member functions
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (6 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 7/9] Remove char-based bitfield macros Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-22  9:46   ` Tom de Vries
  2023-11-03 16:09 ` [PATCH v3 9/9] Simplify C++ type-printing Tom Tromey
  2023-11-21 21:52 ` [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
  9 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

This changes nested types and member functions to use the new
'accessibility' enum, rather than separate private/protected flags.
This is done for consistency, but it also lets us simplify some other
code in the next patch.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/dwarf2/read.c |  8 ++++----
 gdb/gdbtypes.h    | 27 ++++++++++++++-------------
 gdb/stabsread.c   |  4 ++--
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e52ca5b5b83..0d8c24ad5b1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -11714,10 +11714,10 @@ dwarf2_add_type_defn (struct field_info *fip, struct die_info *die,
       /* The assumed value if neither private nor protected.  */
       break;
     case DW_ACCESS_private:
-      fp.is_private = 1;
+      fp.accessibility = accessibility::PRIVATE;
       break;
     case DW_ACCESS_protected:
-      fp.is_protected = 1;
+      fp.accessibility = accessibility::PROTECTED;
       break;
     }
 
@@ -12076,10 +12076,10 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
   switch (accessibility)
     {
     case DW_ACCESS_private:
-      fnp->is_private = 1;
+      fnp->accessibility = accessibility::PRIVATE;
       break;
     case DW_ACCESS_protected:
-      fnp->is_protected = 1;
+      fnp->accessibility = accessibility::PROTECTED;
       break;
     }
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index bec69bdc3d1..d3c7a206b34 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1611,8 +1611,8 @@ struct fn_field
 
   unsigned int is_const:1;
   unsigned int is_volatile:1;
-  unsigned int is_private:1;
-  unsigned int is_protected:1;
+  /* Accessibility of the field.  */
+  ENUM_BITFIELD (accessibility) accessibility : 2;
   unsigned int is_artificial:1;
 
   /* * A stub method only has some fields valid (but they are enough
@@ -1657,11 +1657,8 @@ struct decl_field
 
   struct type *type;
 
-  /* * True if this field was declared protected, false otherwise.  */
-  unsigned int is_protected : 1;
-
-  /* * True if this field was declared private, false otherwise.  */
-  unsigned int is_private : 1;
+  /* Accessibility of the field.  */
+  ENUM_BITFIELD (accessibility) accessibility : 2;
 };
 
 /* * C++ language-specific information for TYPE_CODE_STRUCT and
@@ -1984,8 +1981,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_FN_FIELD_ARGS(thisfn, n) (((thisfn)[n].type)->fields ())
 #define TYPE_FN_FIELD_CONST(thisfn, n) ((thisfn)[n].is_const)
 #define TYPE_FN_FIELD_VOLATILE(thisfn, n) ((thisfn)[n].is_volatile)
-#define TYPE_FN_FIELD_PRIVATE(thisfn, n) ((thisfn)[n].is_private)
-#define TYPE_FN_FIELD_PROTECTED(thisfn, n) ((thisfn)[n].is_protected)
+#define TYPE_FN_FIELD_PRIVATE(thisfn, n) \
+  ((thisfn)[n].accessibility == accessibility::PRIVATE)
+#define TYPE_FN_FIELD_PROTECTED(thisfn, n) \
+  ((thisfn)[n].accessibility == accessibility::PROTECTED)
 #define TYPE_FN_FIELD_ARTIFICIAL(thisfn, n) ((thisfn)[n].is_artificial)
 #define TYPE_FN_FIELD_STUB(thisfn, n) ((thisfn)[n].is_stub)
 #define TYPE_FN_FIELD_CONSTRUCTOR(thisfn, n) ((thisfn)[n].is_constructor)
@@ -2008,9 +2007,9 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_TYPEDEF_FIELD_COUNT(thistype) \
   TYPE_CPLUS_SPECIFIC (thistype)->typedef_field_count
 #define TYPE_TYPEDEF_FIELD_PROTECTED(thistype, n) \
-  TYPE_TYPEDEF_FIELD (thistype, n).is_protected
+  (TYPE_TYPEDEF_FIELD (thistype, n).accessibility == accessibility::PROTECTED)
 #define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n)        \
-  TYPE_TYPEDEF_FIELD (thistype, n).is_private
+  (TYPE_TYPEDEF_FIELD (thistype, n).accessibility == accessibility::PRIVATE)
 
 #define TYPE_NESTED_TYPES_ARRAY(thistype)	\
   TYPE_CPLUS_SPECIFIC (thistype)->nested_types
@@ -2023,9 +2022,11 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
 #define TYPE_NESTED_TYPES_COUNT(thistype) \
   TYPE_CPLUS_SPECIFIC (thistype)->nested_types_count
 #define TYPE_NESTED_TYPES_FIELD_PROTECTED(thistype, n) \
-  TYPE_NESTED_TYPES_FIELD (thistype, n).is_protected
+  (TYPE_NESTED_TYPES_FIELD (thistype, n).accessibility \
+   == accessibility::PROTECTED)
 #define TYPE_NESTED_TYPES_FIELD_PRIVATE(thistype, n)	\
-  TYPE_NESTED_TYPES_FIELD (thistype, n).is_private
+  (TYPE_NESTED_TYPES_FIELD (thistype, n).accessibility \
+   == accessibility::PRIVATE)
 
 #define TYPE_IS_OPAQUE(thistype) \
   ((((thistype)->code () == TYPE_CODE_STRUCT) \
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 334371c2e46..fbf11701337 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -2372,10 +2372,10 @@ read_member_functions (struct stab_field_info *fip, const char **pp,
 	  switch (*(*pp)++)
 	    {
 	    case VISIBILITY_PRIVATE:
-	      new_sublist->fn_field.is_private = 1;
+	      new_sublist->fn_field.accessibility = accessibility::PRIVATE;
 	      break;
 	    case VISIBILITY_PROTECTED:
-	      new_sublist->fn_field.is_protected = 1;
+	      new_sublist->fn_field.accessibility = accessibility::PROTECTED;
 	      break;
 	    }
 

-- 
2.40.1


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

* [PATCH v3 9/9] Simplify C++ type-printing
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (7 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 8/9] Use enum accessibility in types and member functions Tom Tromey
@ 2023-11-03 16:09 ` Tom Tromey
  2023-11-21 21:52 ` [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-03 16:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Keith Seitz

The C++ type-printing code had its own variant of the accessibility
enum.  This patch removes this and changes the code to use the new one
from gdbtypes.h.

This patch also changes the C++ code to recognize the default
accessibility of a class.  This makes ptype a bit more C++-like, and
lets us remove a chunk of questionable code.

Acked-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-by: Keith Seitz <keiths@redhat.com>
---
 gdb/c-typeprint.c                        | 136 +++++++------------------------
 gdb/testsuite/gdb.base/ptype-offsets.exp |   6 --
 2 files changed, 30 insertions(+), 112 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 4a0c6950b0c..3402c783cce 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -33,16 +33,6 @@
 #include "cp-abi.h"
 #include "cp-support.h"
 
-/* A list of access specifiers used for printing.  */
-
-enum access_specifier
-{
-  s_none,
-  s_public,
-  s_private,
-  s_protected
-};
-
 static void c_type_print_varspec_suffix (struct type *, struct ui_file *, int,
 					 int, int,
 					 enum language,
@@ -866,85 +856,32 @@ print_spaces_filtered_with_print_options
 /* Output an access specifier to STREAM, if needed.  LAST_ACCESS is the
    last access specifier output (typically returned by this function).  */
 
-static enum access_specifier
+static accessibility
 output_access_specifier (struct ui_file *stream,
-			 enum access_specifier last_access,
-			 int level, bool is_protected, bool is_private,
+			 accessibility last_access,
+			 int level, accessibility new_access,
 			 const struct type_print_options *flags)
 {
-  if (is_protected)
+  if (last_access == new_access)
+    return new_access;
+
+  if (new_access == accessibility::PROTECTED)
     {
-      if (last_access != s_protected)
-	{
-	  last_access = s_protected;
-	  print_spaces_filtered_with_print_options (level + 2, stream, flags);
-	  gdb_printf (stream, "protected:\n");
-	}
+      print_spaces_filtered_with_print_options (level + 2, stream, flags);
+      gdb_printf (stream, "protected:\n");
     }
-  else if (is_private)
+  else if (new_access == accessibility::PRIVATE)
     {
-      if (last_access != s_private)
-	{
-	  last_access = s_private;
-	  print_spaces_filtered_with_print_options (level + 2, stream, flags);
-	  gdb_printf (stream, "private:\n");
-	}
+      print_spaces_filtered_with_print_options (level + 2, stream, flags);
+      gdb_printf (stream, "private:\n");
     }
   else
     {
-      if (last_access != s_public)
-	{
-	  last_access = s_public;
-	  print_spaces_filtered_with_print_options (level + 2, stream, flags);
-	  gdb_printf (stream, "public:\n");
-	}
+      print_spaces_filtered_with_print_options (level + 2, stream, flags);
+      gdb_printf (stream, "public:\n");
     }
 
-  return last_access;
-}
-
-/* Return true if an access label (i.e., "public:", "private:",
-   "protected:") needs to be printed for TYPE.  */
-
-static bool
-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 (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++)
-	  if (!TYPE_FN_FIELD_PRIVATE (TYPE_FN_FIELDLIST1 (type,
-							  j), i))
-	    return true;
-      for (int i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
-	if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
-	  return true;
-    }
-  else
-    {
-      for (int i = TYPE_N_BASECLASSES (type); i < type->num_fields (); i++)
-	if (!type->field (i).is_public ())
-	  return true;
-      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_PROTECTED (TYPE_FN_FIELDLIST1 (type,
-							     j), i)
-		|| TYPE_FN_FIELD_PRIVATE (TYPE_FN_FIELDLIST1 (type,
-							      j),
-					  i))
-	      return true;
-	}
-      for (int i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
-	if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i)
-	    || TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
-	  return true;
-    }
-
-  return false;
+  return new_access;
 }
 
 /* Helper function that temporarily disables FLAGS->PRINT_OFFSETS,
@@ -1067,17 +1004,9 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 			metadata_style.style ().ptr (), nullptr);
 	}
 
-      /* Start off with no specific section type, so we can print
-	 one for the first field we find, and use that section type
-	 thereafter until we find another type.  */
-
-      enum access_specifier section_type = s_none;
-
-      /* For a class, if all members are private, there's no need
-	 for a "private:" label; similarly, for a struct or union
-	 masquerading as a class, if all members are public, there's
-	 no need for a "public:" label.  */
-      bool need_access_label = need_access_label_p (type);
+      accessibility section_type = (type->is_declared_class ()
+				    ? accessibility::PRIVATE
+				    : accessibility::PUBLIC);
 
       /* If there is a base class for this type,
 	 do not print the field that it occupies.  */
@@ -1098,13 +1027,10 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 	      || type->field (i).is_artificial ())
 	    continue;
 
-	  if (need_access_label)
-	    {
-	      section_type = output_access_specifier
-		(stream, section_type, level,
-		 type->field (i).is_protected (),
-		 type->field (i).is_private (), flags);
-	    }
+	  section_type
+	    = output_access_specifier (stream, section_type, level,
+				       type->field (i).accessibility (),
+				       flags);
 
 	  bool is_static = type->field (i).is_static ();
 
@@ -1176,7 +1102,7 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 	    if (!TYPE_FN_FIELD_ARTIFICIAL (f, j))
 	      real_len++;
 	}
-      if (real_len > 0 && section_type != s_none)
+      if (real_len > 0)
 	gdb_printf (stream, "\n");
 
       /* C++: print out the methods.  */
@@ -1207,8 +1133,8 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 	      QUIT;
 	      section_type = output_access_specifier
 		(stream, section_type, level,
-		 TYPE_FN_FIELD_PROTECTED (f, j),
-		 TYPE_FN_FIELD_PRIVATE (f, j), flags);
+		 TYPE_FN_FIELD (f, j).accessibility,
+		 flags);
 
 	      print_spaces_filtered_with_print_options (level + 4, stream,
 							flags);
@@ -1334,13 +1260,11 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
 	      gdb_assert (target->code () == TYPE_CODE_TYPEDEF);
 	      target = target->target_type ();
 
-	      if (need_access_label)
-		{
-		  section_type = output_access_specifier
-		    (stream, section_type, level,
-		     TYPE_TYPEDEF_FIELD_PROTECTED (type, i),
-		     TYPE_TYPEDEF_FIELD_PRIVATE (type, i), flags);
-		}
+	      section_type = (output_access_specifier
+			      (stream, section_type, level,
+			       TYPE_TYPEDEF_FIELD (type, i).accessibility,
+			       flags));
+
 	      print_spaces_filtered_with_print_options (level + 4, stream,
 							flags);
 	      gdb_printf (stream, "typedef ");
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index 2cfc70f1686..f36bd2fdedf 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -36,7 +36,6 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
 gdb_test "ptype /o struct abc" \
     [string_to_regexp [multi_line \
 "/* offset      |    size */  type = struct abc \{" \
-"                             public:" \
 "/*      8      |       8 */    void *field1;" \
 "/*     16: 0   |       4 */    unsigned int field2 : 1;" \
 "/* XXX  7-bit hole       */" \
@@ -61,7 +60,6 @@ gdb_test "ptype /o struct abc" \
 gdb_test "ptype /ox struct abc" \
     [string_to_regexp [multi_line \
 "/* offset      |    size */  type = struct abc {" \
-"                             public:" \
 "/* 0x0008      |  0x0008 */    void *field1;" \
 "/* 0x0010: 0x0 |  0x0004 */    unsigned int field2 : 1;" \
 "/* XXX  7-bit hole       */" \
@@ -86,7 +84,6 @@ gdb_test "ptype /ox struct abc" \
 gdb_test "ptype /oTM struct abc" \
     [string_to_regexp [multi_line \
 "/* offset      |    size */  type = struct abc \{" \
-"                             public:" \
 "/*      8      |       8 */    void *field1;" \
 "/*     16: 0   |       4 */    unsigned int field2 : 1;" \
 "/* XXX  7-bit hole       */" \
@@ -116,7 +113,6 @@ gdb_test "ptype /oTM struct abc" \
 gdb_test "ptype /TMo struct abc" \
     [string_to_regexp [multi_line \
 "/* offset      |    size */  type = struct abc \{" \
-"                             public:" \
 "/*      8      |       8 */    void *field1;" \
 "/*     16: 0   |       4 */    unsigned int field2 : 1;" \
 "/* XXX  7-bit hole       */" \
@@ -421,7 +417,6 @@ with_test_prefix "with_hex_default" {
   gdb_test "ptype /o struct abc" \
       [string_to_regexp [multi_line \
   "/* offset      |    size */  type = struct abc \{" \
-  "                             public:" \
   "/* 0x0008      |  0x0008 */    void *field1;" \
   "/* 0x0010: 0x0 |  0x0004 */    unsigned int field2 : 1;" \
   "/* XXX  7-bit hole       */" \
@@ -445,7 +440,6 @@ with_test_prefix "with_hex_default" {
   gdb_test "ptype /od struct abc" \
       [string_to_regexp [multi_line \
   "/* offset      |    size */  type = struct abc \{" \
-  "                             public:" \
   "/*      8      |       8 */    void *field1;" \
   "/*     16: 0   |       4 */    unsigned int field2 : 1;" \
   "/* XXX  7-bit hole       */" \

-- 
2.40.1


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

* Re: [PATCH v3 0/9] Remove char-based bitfield macros
  2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
                   ` (8 preceding siblings ...)
  2023-11-03 16:09 ` [PATCH v3 9/9] Simplify C++ type-printing Tom Tromey
@ 2023-11-21 21:52 ` Tom Tromey
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-21 21:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Simon Marchi, Keith Seitz

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

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

Tom> Regression tested on x86-64 Fedora 36.

I'm checking these in.

Tom

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

* Re: [PATCH v3 8/9] Use enum accessibility in types and member functions
  2023-11-03 16:09 ` [PATCH v3 8/9] Use enum accessibility in types and member functions Tom Tromey
@ 2023-11-22  9:46   ` Tom de Vries
  2023-11-22 13:54     ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Tom de Vries @ 2023-11-22  9:46 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches; +Cc: Simon Marchi, Keith Seitz

On 11/3/23 17:09, Tom Tromey wrote:
> This changes nested types and member functions to use the new
> 'accessibility' enum, rather than separate private/protected flags.
> This is done for consistency, but it also lets us simplify some other
> code in the next patch.
> 

FWIW, this breaks the build for me with system gcc 7.5.0:
...
/data/vries/gdb/src/gdb/gdbtypes.h:721:51: error: 
‘field::m_accessibility’ is too small to hold all values of ‘enum class 
accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) m_accessibility : 2;
                                                    ^
/data/vries/gdb/src/gdb/gdbtypes.h:1615:49: error: 
‘fn_field::accessibility’ is too small to hold all values of ‘enum class 
accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) accessibility : 2;
                                                  ^
/data/vries/gdb/src/gdb/gdbtypes.h:1661:49: error: 
‘decl_field::accessibility’ is too small to hold all values of ‘enum 
class accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) accessibility : 2;
                                                  ^
...

I can fix this by using --disable-werror.

Also builds fine when I switch to gcc 9.3.1 with --enable-werror.

Thanks,
- Tom

> Acked-By: Simon Marchi <simon.marchi@efficios.com>
> Reviewed-by: Keith Seitz <keiths@redhat.com>
> ---
>   gdb/dwarf2/read.c |  8 ++++----
>   gdb/gdbtypes.h    | 27 ++++++++++++++-------------
>   gdb/stabsread.c   |  4 ++--
>   3 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index e52ca5b5b83..0d8c24ad5b1 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -11714,10 +11714,10 @@ dwarf2_add_type_defn (struct field_info *fip, struct die_info *die,
>         /* The assumed value if neither private nor protected.  */
>         break;
>       case DW_ACCESS_private:
> -      fp.is_private = 1;
> +      fp.accessibility = accessibility::PRIVATE;
>         break;
>       case DW_ACCESS_protected:
> -      fp.is_protected = 1;
> +      fp.accessibility = accessibility::PROTECTED;
>         break;
>       }
>   
> @@ -12076,10 +12076,10 @@ dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
>     switch (accessibility)
>       {
>       case DW_ACCESS_private:
> -      fnp->is_private = 1;
> +      fnp->accessibility = accessibility::PRIVATE;
>         break;
>       case DW_ACCESS_protected:
> -      fnp->is_protected = 1;
> +      fnp->accessibility = accessibility::PROTECTED;
>         break;
>       }
>   
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index bec69bdc3d1..d3c7a206b34 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1611,8 +1611,8 @@ struct fn_field
>   
>     unsigned int is_const:1;
>     unsigned int is_volatile:1;
> -  unsigned int is_private:1;
> -  unsigned int is_protected:1;
> +  /* Accessibility of the field.  */
> +  ENUM_BITFIELD (accessibility) accessibility : 2;
>     unsigned int is_artificial:1;
>   
>     /* * A stub method only has some fields valid (but they are enough
> @@ -1657,11 +1657,8 @@ struct decl_field
>   
>     struct type *type;
>   
> -  /* * True if this field was declared protected, false otherwise.  */
> -  unsigned int is_protected : 1;
> -
> -  /* * True if this field was declared private, false otherwise.  */
> -  unsigned int is_private : 1;
> +  /* Accessibility of the field.  */
> +  ENUM_BITFIELD (accessibility) accessibility : 2;
>   };
>   
>   /* * C++ language-specific information for TYPE_CODE_STRUCT and
> @@ -1984,8 +1981,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
>   #define TYPE_FN_FIELD_ARGS(thisfn, n) (((thisfn)[n].type)->fields ())
>   #define TYPE_FN_FIELD_CONST(thisfn, n) ((thisfn)[n].is_const)
>   #define TYPE_FN_FIELD_VOLATILE(thisfn, n) ((thisfn)[n].is_volatile)
> -#define TYPE_FN_FIELD_PRIVATE(thisfn, n) ((thisfn)[n].is_private)
> -#define TYPE_FN_FIELD_PROTECTED(thisfn, n) ((thisfn)[n].is_protected)
> +#define TYPE_FN_FIELD_PRIVATE(thisfn, n) \
> +  ((thisfn)[n].accessibility == accessibility::PRIVATE)
> +#define TYPE_FN_FIELD_PROTECTED(thisfn, n) \
> +  ((thisfn)[n].accessibility == accessibility::PROTECTED)
>   #define TYPE_FN_FIELD_ARTIFICIAL(thisfn, n) ((thisfn)[n].is_artificial)
>   #define TYPE_FN_FIELD_STUB(thisfn, n) ((thisfn)[n].is_stub)
>   #define TYPE_FN_FIELD_CONSTRUCTOR(thisfn, n) ((thisfn)[n].is_constructor)
> @@ -2008,9 +2007,9 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
>   #define TYPE_TYPEDEF_FIELD_COUNT(thistype) \
>     TYPE_CPLUS_SPECIFIC (thistype)->typedef_field_count
>   #define TYPE_TYPEDEF_FIELD_PROTECTED(thistype, n) \
> -  TYPE_TYPEDEF_FIELD (thistype, n).is_protected
> +  (TYPE_TYPEDEF_FIELD (thistype, n).accessibility == accessibility::PROTECTED)
>   #define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n)        \
> -  TYPE_TYPEDEF_FIELD (thistype, n).is_private
> +  (TYPE_TYPEDEF_FIELD (thistype, n).accessibility == accessibility::PRIVATE)
>   
>   #define TYPE_NESTED_TYPES_ARRAY(thistype)	\
>     TYPE_CPLUS_SPECIFIC (thistype)->nested_types
> @@ -2023,9 +2022,11 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
>   #define TYPE_NESTED_TYPES_COUNT(thistype) \
>     TYPE_CPLUS_SPECIFIC (thistype)->nested_types_count
>   #define TYPE_NESTED_TYPES_FIELD_PROTECTED(thistype, n) \
> -  TYPE_NESTED_TYPES_FIELD (thistype, n).is_protected
> +  (TYPE_NESTED_TYPES_FIELD (thistype, n).accessibility \
> +   == accessibility::PROTECTED)
>   #define TYPE_NESTED_TYPES_FIELD_PRIVATE(thistype, n)	\
> -  TYPE_NESTED_TYPES_FIELD (thistype, n).is_private
> +  (TYPE_NESTED_TYPES_FIELD (thistype, n).accessibility \
> +   == accessibility::PRIVATE)
>   
>   #define TYPE_IS_OPAQUE(thistype) \
>     ((((thistype)->code () == TYPE_CODE_STRUCT) \
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c
> index 334371c2e46..fbf11701337 100644
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
> @@ -2372,10 +2372,10 @@ read_member_functions (struct stab_field_info *fip, const char **pp,
>   	  switch (*(*pp)++)
>   	    {
>   	    case VISIBILITY_PRIVATE:
> -	      new_sublist->fn_field.is_private = 1;
> +	      new_sublist->fn_field.accessibility = accessibility::PRIVATE;
>   	      break;
>   	    case VISIBILITY_PROTECTED:
> -	      new_sublist->fn_field.is_protected = 1;
> +	      new_sublist->fn_field.accessibility = accessibility::PROTECTED;
>   	      break;
>   	    }
>   
> 


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

* Re: [PATCH v3 8/9] Use enum accessibility in types and member functions
  2023-11-22  9:46   ` Tom de Vries
@ 2023-11-22 13:54     ` Tom Tromey
  2023-11-22 14:03       ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2023-11-22 13:54 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Tom Tromey, gdb-patches, Simon Marchi, Keith Seitz

Tom> FWIW, this breaks the build for me with system gcc 7.5.0:

Mark mentioned something like this yesterday, but didn't say which
patch...  it's a GCC bug.

However, I will push a gdb fix momentarily.

Tom

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

* Re: [PATCH v3 8/9] Use enum accessibility in types and member functions
  2023-11-22 13:54     ` Tom Tromey
@ 2023-11-22 14:03       ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-11-22 14:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom de Vries, gdb-patches, Simon Marchi, Keith Seitz

Tom> However, I will push a gdb fix momentarily.

Actually, since I can't test it, I'll just send the patch.  I'll CC you.
Please try it out.

Tom

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

end of thread, other threads:[~2023-11-22 14:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-03 16:09 [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey
2023-11-03 16:09 ` [PATCH v3 1/9] Use .def file to stringify type codes Tom Tromey
2023-11-03 16:09 ` [PATCH v3 2/9] Print field accessibility inline Tom Tromey
2023-11-03 16:09 ` [PATCH v3 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
2023-11-03 16:09 ` [PATCH v3 4/9] Add field::is_public Tom Tromey
2023-11-03 16:09 ` [PATCH v3 5/9] Remove some QUIT calls from need_access_label_p Tom Tromey
2023-11-03 16:09 ` [PATCH v3 6/9] Remove some type field accessor macros Tom Tromey
2023-11-03 16:09 ` [PATCH v3 7/9] Remove char-based bitfield macros Tom Tromey
2023-11-03 16:09 ` [PATCH v3 8/9] Use enum accessibility in types and member functions Tom Tromey
2023-11-22  9:46   ` Tom de Vries
2023-11-22 13:54     ` Tom Tromey
2023-11-22 14:03       ` Tom Tromey
2023-11-03 16:09 ` [PATCH v3 9/9] Simplify C++ type-printing Tom Tromey
2023-11-21 21:52 ` [PATCH v3 0/9] Remove char-based bitfield macros Tom Tromey

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