* [PATCH v2 0/9] Remove char-based bitfield macros
@ 2023-10-27 17:36 Tom Tromey
2023-10-27 17:36 ` [PATCH v2 1/9] Use .def file to stringify type codes Tom Tromey
` (9 more replies)
0 siblings, 10 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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 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 | 181 +++++++------------------------
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, 228 insertions(+), 549 deletions(-)
---
base-commit: 2c1e03b4520e908aa1eb36ecda279047b17bab23
change-id: 20230921-field-bits-9b9f802eb42b
Best regards,
--
Tom Tromey <tromey@adacore.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/9] Use .def file to stringify type codes
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-11-03 4:07 ` Simon Marchi
2023-10-27 17:36 ` [PATCH v2 2/9] Print field accessibility inline Tom Tromey
` (8 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
This changes recursive_dump_type to reuse the type-codes.def file when
stringifying type codes.
---
gdb/gdbtypes.c | 98 ++++++++++------------------------------------------------
1 file changed, 17 insertions(+), 81 deletions(-)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 7011fddd695..1de90eff7ee 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5099,6 +5099,22 @@ 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";
+ }
+ return "UNKNOWN TYPE CODE";
+}
+
void
recursive_dump_type (struct type *type, int spaces)
{
@@ -5136,87 +5152,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] 18+ messages in thread
* [PATCH v2 2/9] Print field accessibility inline
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
2023-10-27 17:36 ` [PATCH v2 1/9] Use .def file to stringify type codes Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-11-03 4:10 ` Simon Marchi
2023-10-27 17:36 ` [PATCH v2 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
` (7 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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 1de90eff7ee..203728a61d8 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5288,12 +5288,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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/9] Remove byte vectors from cplus_struct_type
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
2023-10-27 17:36 ` [PATCH v2 1/9] Use .def file to stringify type codes Tom Tromey
2023-10-27 17:36 ` [PATCH v2 2/9] Print field accessibility inline Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-11-03 4:22 ` Simon Marchi
2023-10-27 17:36 ` [PATCH v2 4/9] Add field::is_public Tom Tromey
` (6 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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.
---
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 ea0b2328a3e..22aaec30ff1 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. */
@@ -11655,15 +11650,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;
@@ -11757,8 +11760,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)
@@ -12072,30 +12074,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 ();
}
@@ -12110,41 +12091,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 203728a61d8..68a210faca4 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..0a6bd425a5f 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. */
+ accessibility get_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] 18+ messages in thread
* [PATCH v2 4/9] Add field::is_public
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (2 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 5/9] Remove some QUIT calls from need_access_label_p Tom Tromey
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
This adds a field::is_public convenience method, and updates one spot
to use it.
---
gdb/gdbtypes.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 0a6bd425a5f..a4d2f3fa3e6 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -686,6 +686,10 @@ struct field
accessibility get_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] 18+ messages in thread
* [PATCH v2 5/9] Remove some QUIT calls from need_access_label_p
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (3 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 4/9] Add field::is_public Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 6/9] Remove some type field accessor macros Tom Tromey
` (4 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 6/9] Remove some type field accessor macros
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (4 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 5/9] Remove some QUIT calls from need_access_label_p Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 7/9] Remove char-based bitfield macros Tom Tromey
` (3 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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.
---
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..f3bc0731c99 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).get_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 68a210faca4..10fea9f74fa 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5228,31 +5228,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 a4d2f3fa3e6..ec27ca3b47a 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] 18+ messages in thread
* [PATCH v2 7/9] Remove char-based bitfield macros
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (5 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 6/9] Remove some type field accessor macros Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 8/9] Use enum accessibility in types and member functions Tom Tromey
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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 ec27ca3b47a..cfa378f77a1 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] 18+ messages in thread
* [PATCH v2 8/9] Use enum accessibility in types and member functions
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (6 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 7/9] Remove char-based bitfield macros Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 9/9] Simplify C++ type-printing Tom Tromey
2023-11-02 16:54 ` [PATCH v2 0/9] Remove char-based bitfield macros Keith Seitz
9 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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.
---
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 22aaec30ff1..15acec68788 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -11855,10 +11855,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;
}
@@ -12217,10 +12217,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 cfa378f77a1..3c7c420f501 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] 18+ messages in thread
* [PATCH v2 9/9] Simplify C++ type-printing
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (7 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 8/9] Use enum accessibility in types and member functions Tom Tromey
@ 2023-10-27 17:36 ` Tom Tromey
2023-11-02 16:54 ` [PATCH v2 0/9] Remove char-based bitfield macros Keith Seitz
9 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-27 17:36 UTC (permalink / raw)
To: gdb-patches
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.
---
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..40f69481f07 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).get_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] 18+ messages in thread
* Re: [PATCH v2 0/9] Remove char-based bitfield macros
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
` (8 preceding siblings ...)
2023-10-27 17:36 ` [PATCH v2 9/9] Simplify C++ type-printing Tom Tromey
@ 2023-11-02 16:54 ` Keith Seitz
2023-11-03 4:23 ` Simon Marchi
9 siblings, 1 reply; 18+ messages in thread
From: Keith Seitz @ 2023-11-02 16:54 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 10/27/23 10:36, Tom Tromey wrote:
> 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.
This is a really nice change which demonstrates the wins we can see
with C++. I find the code eminently more readable and understandable.
I know Lancelot reviewed your original patches, but I've looked over
this revision, and I am at a loss to find anything requiring additional
attention.
Reviewed-by: Keith Seitz <keiths@redhat.com>
Keith
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/9] Use .def file to stringify type codes
2023-10-27 17:36 ` [PATCH v2 1/9] Use .def file to stringify type codes Tom Tromey
@ 2023-11-03 4:07 ` Simon Marchi
2023-11-03 15:52 ` Tom Tromey
0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2023-11-03 4:07 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2023-10-27 13:36, Tom Tromey wrote:
> This changes recursive_dump_type to reuse the type-codes.def file when
> stringifying type codes.
> ---
> gdb/gdbtypes.c | 98 ++++++++++------------------------------------------------
> 1 file changed, 17 insertions(+), 81 deletions(-)
>
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 7011fddd695..1de90eff7ee 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5099,6 +5099,22 @@ 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";
> + }
> + return "UNKNOWN TYPE CODE";
I think this could be a gdb_assert_not_reached / internal_error.
Especially with the .def approach, where it's not possible to forget to
handle a type code. If we reached this point, it's really because
something in GDB is broken.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/9] Print field accessibility inline
2023-10-27 17:36 ` [PATCH v2 2/9] Print field accessibility inline Tom Tromey
@ 2023-11-03 4:10 ` Simon Marchi
2023-11-03 15:53 ` Tom Tromey
0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2023-11-03 4:10 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2023-10-27 13:36, Tom Tromey wrote:
> 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 1de90eff7ee..203728a61d8 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5288,12 +5288,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");
I know this is probably personal preference and you write things with
fewer empty lines than I doo, but I find it very difficult to read when
it's so packed, it's hard to discern that this is two ifs, not just one
chain if if/else if/else. Can you add some new lines to separate them
like this?
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");
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/9] Remove byte vectors from cplus_struct_type
2023-10-27 17:36 ` [PATCH v2 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
@ 2023-11-03 4:22 ` Simon Marchi
2023-11-03 16:07 ` Tom Tromey
0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2023-11-03 4:22 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2023-10-27 13:36, Tom Tromey wrote:
> @@ -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. */
> + accessibility get_accessibility () const
> + { return m_accessibility; }
I'd suggest removing the `get_` prefix. Our getters generally don't
have the prefix (at least those that I add).
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/9] Remove char-based bitfield macros
2023-11-02 16:54 ` [PATCH v2 0/9] Remove char-based bitfield macros Keith Seitz
@ 2023-11-03 4:23 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2023-11-03 4:23 UTC (permalink / raw)
To: Keith Seitz, Tom Tromey, gdb-patches
On 2023-11-02 12:54, Keith Seitz wrote:
> On 10/27/23 10:36, Tom Tromey wrote:
>> 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.
>
> This is a really nice change which demonstrates the wins we can see
> with C++. I find the code eminently more readable and understandable.
Agreed, I'm especially glad that you got rid of some more TYPE_
macros.
I only looked at the changes in diagonal, but I like where this is
going, so:
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/9] Use .def file to stringify type codes
2023-11-03 4:07 ` Simon Marchi
@ 2023-11-03 15:52 ` Tom Tromey
0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-11-03 15:52 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>> + return "UNKNOWN TYPE CODE";
Simon> I think this could be a gdb_assert_not_reached / internal_error.
Simon> Especially with the .def approach, where it's not possible to forget to
Simon> handle a type code. If we reached this point, it's really because
Simon> something in GDB is broken.
I did this.
I didn't do it originally to preserve the previous semantics.
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/9] Print field accessibility inline
2023-11-03 4:10 ` Simon Marchi
@ 2023-11-03 15:53 ` Tom Tromey
0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-11-03 15:53 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> I know this is probably personal preference and you write things with
Simon> fewer empty lines than I doo, but I find it very difficult to read when
Simon> it's so packed, it's hard to discern that this is two ifs, not just one
Simon> chain if if/else if/else. Can you add some new lines to separate them
Simon> like this?
I made this change.
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/9] Remove byte vectors from cplus_struct_type
2023-11-03 4:22 ` Simon Marchi
@ 2023-11-03 16:07 ` Tom Tromey
0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-11-03 16:07 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>> + /* Fetch the field's accessibility. */
>> + accessibility get_accessibility () const
>> + { return m_accessibility; }
Simon> I'd suggest removing the `get_` prefix. Our getters generally don't
Simon> have the prefix (at least those that I add).
I did this.
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-11-03 16:07 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-27 17:36 [PATCH v2 0/9] Remove char-based bitfield macros Tom Tromey
2023-10-27 17:36 ` [PATCH v2 1/9] Use .def file to stringify type codes Tom Tromey
2023-11-03 4:07 ` Simon Marchi
2023-11-03 15:52 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 2/9] Print field accessibility inline Tom Tromey
2023-11-03 4:10 ` Simon Marchi
2023-11-03 15:53 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 3/9] Remove byte vectors from cplus_struct_type Tom Tromey
2023-11-03 4:22 ` Simon Marchi
2023-11-03 16:07 ` Tom Tromey
2023-10-27 17:36 ` [PATCH v2 4/9] Add field::is_public Tom Tromey
2023-10-27 17:36 ` [PATCH v2 5/9] Remove some QUIT calls from need_access_label_p Tom Tromey
2023-10-27 17:36 ` [PATCH v2 6/9] Remove some type field accessor macros Tom Tromey
2023-10-27 17:36 ` [PATCH v2 7/9] Remove char-based bitfield macros Tom Tromey
2023-10-27 17:36 ` [PATCH v2 8/9] Use enum accessibility in types and member functions Tom Tromey
2023-10-27 17:36 ` [PATCH v2 9/9] Simplify C++ type-printing Tom Tromey
2023-11-02 16:54 ` [PATCH v2 0/9] Remove char-based bitfield macros Keith Seitz
2023-11-03 4:23 ` Simon Marchi
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).