* [PATCH] Record and output access specifiers for nested typedefs @ 2017-10-13 19:18 Keith Seitz 2017-10-16 19:57 ` Simon Marchi 0 siblings, 1 reply; 8+ messages in thread From: Keith Seitz @ 2017-10-13 19:18 UTC (permalink / raw) To: gdb-patches We currently do not record access information for typedefs defined inside classes. Consider: struct foo { typedef int PUBLIC; private: typedef int PRIVATE; PRIVATE b; }; (gdb) ptype foo type = struct foo { private: PRIVATE b; typedef int PRIVATE; typedef int PUBLIC; } This patch fixes this: (gdb) ptype foo type = struct foo { private: PRIVATE b; typedef int PRIVATE; public: typedef int PUBLIC; } gdb/ChangeLog: * c-typeprint.c (enum access_specifier): Moved here from c_type_print_base. (output_access_specifier): New function. (c_type_print_base): Consider typedefs when assessing whether access labels are needed. Use output_access_specifier as needed. Output access specifier for typedefs, if needed. * dwarf2read.c (dwarf2_add_typedef): Record DW_AT_accessibility. * gdbtypes.h (struct typedef_field) <is_protected, is_private>: New fields. (TYPE_TYPEDEF_FIELD_PROTECTED, TYPE_TYEPDEF_FIELD_PRIVATE): New accessor macros. gdb/testsuite/ChangeLog: * gdb.cp/classes.cc (class_with_typedefs, class_with_public_typedef) (class_with_protected_typedef, class_with_private_typedef) (struct_with_public_typedef, struct_with_protected_typedef) (struct_with_private_typedef): New classes/structs. * gdb.cp/classes.exp (test_ptype_class_objects): Add tests for typedefs and access specifiers. --- gdb/c-typeprint.c | 145 +++++++++++++++++++++++---------------- gdb/dwarf2read.c | 22 ++++++ gdb/gdbtypes.h | 11 +++ gdb/testsuite/gdb.cp/classes.cc | 83 ++++++++++++++++++++++ gdb/testsuite/gdb.cp/classes.exp | 58 ++++++++++++++++ 5 files changed, 259 insertions(+), 60 deletions(-) diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c index 978fcc4..3dc74f9 100644 --- a/gdb/c-typeprint.c +++ b/gdb/c-typeprint.c @@ -32,6 +32,16 @@ #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_prefix (struct type *, struct ui_file *, int, int, int, @@ -826,7 +836,42 @@ c_type_print_template_args (const struct type_print_options *flags, fputs_filtered (_("] "), stream); } -/* Print the name of the type (or the ultimate pointer target, +/* Output an access specifier to STREAM, if needed. */ + +static void +output_access_specifier (struct ui_file *stream, enum access_specifier &access, + int level, bool is_protected, bool is_private) +{ + if (is_protected) + { + if (access != s_protected) + { + access = s_protected; + fprintfi_filtered (level + 2, stream, + "protected:\n"); + } + } + else if (is_private) + { + if (access != s_private) + { + access = s_private; + fprintfi_filtered (level + 2, stream, + "private:\n"); + } + } + else + { + if (access != s_public) + { + access = s_public; + fprintfi_filtered (level + 2, stream, + "public:\n"); + } + } +} + + /* Print the name of the type (or the ultimate pointer target, function value or array element), or the description of a structure or union. @@ -850,11 +895,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, { int i; int len, real_len; - enum - { - s_none, s_public, s_private, s_protected - } - section_type; + enum access_specifier section_type; int need_access_label = 0; int j, len2; @@ -1034,6 +1075,18 @@ c_type_print_base (struct type *type, struct ui_file *stream, break; } } + QUIT; + if (!need_access_label) + { + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i) + { + if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i)) + { + need_access_label = 1; + break; + } + } + } } else { @@ -1068,6 +1121,19 @@ c_type_print_base (struct type *type, struct ui_file *stream, break; } } + QUIT; + if (!need_access_label) + { + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i) + { + if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i) + || TYPE_TYPEDEF_FIELD_PRIVATE (type, i)) + { + need_access_label = 1; + break; + } + } + } } /* If there is a base class for this type, @@ -1088,33 +1154,9 @@ c_type_print_base (struct type *type, struct ui_file *stream, if (need_access_label) { - if (TYPE_FIELD_PROTECTED (type, i)) - { - if (section_type != s_protected) - { - section_type = s_protected; - fprintfi_filtered (level + 2, stream, - "protected:\n"); - } - } - else if (TYPE_FIELD_PRIVATE (type, i)) - { - if (section_type != s_private) - { - section_type = s_private; - fprintfi_filtered (level + 2, stream, - "private:\n"); - } - } - else - { - if (section_type != s_public) - { - section_type = s_public; - fprintfi_filtered (level + 2, stream, - "public:\n"); - } - } + output_access_specifier (stream, section_type, level, + TYPE_FIELD_PROTECTED (type, i), + TYPE_FIELD_PRIVATE (type, i)); } print_spaces_filtered (level + 4, stream); @@ -1188,33 +1230,9 @@ c_type_print_base (struct type *type, struct ui_file *stream, inner_cleanup = make_cleanup (null_cleanup, NULL); QUIT; - if (TYPE_FN_FIELD_PROTECTED (f, j)) - { - if (section_type != s_protected) - { - section_type = s_protected; - fprintfi_filtered (level + 2, stream, - "protected:\n"); - } - } - else if (TYPE_FN_FIELD_PRIVATE (f, j)) - { - if (section_type != s_private) - { - section_type = s_private; - fprintfi_filtered (level + 2, stream, - "private:\n"); - } - } - else - { - if (section_type != s_public) - { - section_type = s_public; - fprintfi_filtered (level + 2, stream, - "public:\n"); - } - } + output_access_specifier (stream, section_type, level, + TYPE_FN_FIELD_PROTECTED (f, j), + TYPE_FN_FIELD_PRIVATE (f, j)); print_spaces_filtered (level + 4, stream); if (TYPE_FN_FIELD_VIRTUAL_P (f, j)) @@ -1326,6 +1344,13 @@ c_type_print_base (struct type *type, struct ui_file *stream, gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF); target = TYPE_TARGET_TYPE (target); + if (need_access_label) + { + output_access_specifier + (stream, section_type, level, + TYPE_TYPEDEF_FIELD_PROTECTED (type, i), + TYPE_TYPEDEF_FIELD_PRIVATE (type, i)); + } print_spaces_filtered (level + 4, stream); fprintf_filtered (stream, "typedef "); diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 9bdb75d..c0200f9 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -13274,6 +13274,28 @@ dwarf2_add_typedef (struct field_info *fip, struct die_info *die, fp->type = read_type_die (die, cu); + /* Save accessibility. */ + enum dwarf_access_attribute accessibility; + struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu); + if (attr != NULL) + accessibility = (enum dwarf_access_attribute) DW_UNSND (attr); + else + accessibility = dwarf2_default_access_attribute (die, cu); + switch (accessibility) + { + case DW_ACCESS_public: + /* The assumed value if neither private nor protected. */ + break; + case DW_ACCESS_private: + fp->is_private = 1; + break; + case DW_ACCESS_protected: + fp->is_protected = 1; + break; + default: + gdb_assert_not_reached ("unexpected accessibility attribute"); + } + new_field->next = fip->typedef_field_list; fip->typedef_field_list = new_field; fip->typedef_field_list_count++; diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 0e0aead..48af6f1 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -896,6 +896,12 @@ struct typedef_field /* * Type this typedef named NAME represents. */ 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; }; /* * C++ language-specific information for TYPE_CODE_STRUCT and @@ -1427,6 +1433,7 @@ extern void set_type_vptr_basetype (struct type *, struct type *); #define TYPE_FN_FIELD_VIRTUAL_P(thisfn, n) ((thisfn)[n].voffset > 1) #define TYPE_FN_FIELD_STATIC_P(thisfn, n) ((thisfn)[n].voffset == VOFFSET_STATIC) +/* Accessors for typedefs defined by a class. */ #define TYPE_TYPEDEF_FIELD_ARRAY(thistype) \ TYPE_CPLUS_SPECIFIC (thistype)->typedef_field #define TYPE_TYPEDEF_FIELD(thistype, n) \ @@ -1437,6 +1444,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *); TYPE_TYPEDEF_FIELD (thistype, n).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 +#define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n) \ + TYPE_TYPEDEF_FIELD (thistype, n).is_private #define TYPE_IS_OPAQUE(thistype) \ (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \ diff --git a/gdb/testsuite/gdb.cp/classes.cc b/gdb/testsuite/gdb.cp/classes.cc index 2a81473..50f0740 100644 --- a/gdb/testsuite/gdb.cp/classes.cc +++ b/gdb/testsuite/gdb.cp/classes.cc @@ -545,6 +545,82 @@ small::method () return x + 5; } +class class_with_typedefs +{ +public: + typedef int public_int; +protected: + typedef int protected_int; +private: + typedef int private_int; + +public: + class_with_typedefs () + : public_int_ (1), protected_int_ (2), private_int_ (3) {} + public_int add_public (public_int a) { return a + public_int_; } + public_int add_all (int a) + { return add_public (a) + add_protected (a) + add_private (a); } + +protected: + protected_int add_protected (protected_int a) { return a + protected_int_; } + +private: + private_int add_private (private_int a) { return a + private_int_; } + +protected: + public_int public_int_; + protected_int protected_int_; + private_int private_int_; +}; + +class class_with_public_typedef +{ + int a; +public: + typedef int INT; + INT b; +}; + +class class_with_protected_typedef +{ + int a; +protected: + typedef int INT; + INT b; +}; + +class class_with_private_typedef +{ + int a; +private: + typedef int INT; + INT b; +}; + +struct struct_with_public_typedef +{ + int a; +public: + typedef int INT; + INT b; +}; + +struct struct_with_protected_typedef +{ + int a; +protected: + typedef int INT; + INT b; +}; + +struct struct_with_private_typedef +{ + int a; +private: + typedef int INT; + INT b; +}; + void marker_reg1 () {} int @@ -624,3 +700,10 @@ protected_class protected_c; default_private_class default_private_c; explicit_private_class explicit_private_c; mixed_protection_class mixed_protection_c; +class_with_typedefs class_with_typedefs_c; +class_with_public_typedef class_with_public_typedef_c; +class_with_protected_typedef class_with_protected_typedef_c; +class_with_private_typedef class_with_private_typedef_c; +struct_with_public_typedef struct_with_public_typedef_s; +struct_with_protected_typedef struct_with_protected_typedef_s; +struct_with_private_typedef struct_with_private_typedef_s; diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp index 256fa68..1f6d377 100644 --- a/gdb/testsuite/gdb.cp/classes.exp +++ b/gdb/testsuite/gdb.cp/classes.exp @@ -316,6 +316,64 @@ proc test_ptype_class_objects {} { { field public "int y;" } { method public "DynamicBar(int, int);" } } + + # Classes with tyepdefs of different access. + + cp_test_ptype_class \ + "class class_with_typedefs" "" "class" "class_with_typedefs" \ + { + { field protected \ + "class_with_typedefs::public_int public_int_;" } + { field protected \ + "class_with_typedefs::protected_int protected_int_;" } + { field protected \ + "class_with_typedefs::private_int private_int_;" } + { method public "class_with_typedefs(void);" } + { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" } + { method public \ + "class_with_typedefs::public_int add_all(int);" } + { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" } + { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" } + {typedef public "typedef int public_int;" } + {typedef protected "typedef int protected_int;" } + {typedef private "typedef int private_int;" } + } + + cp_test_ptype_class \ + "class class_with_public_typedef" "" "class" \ + "class_with_public_typedef" { + { field private "int a;" } + { field public "class_with_public_typedef::INT b;" } + { typedef public "typedef int INT;" } + } + cp_test_ptype_class \ + "class class_with_protected_typedef" "" "class" \ + "class_with_protected_typedef" { + { field private "int a;" } + { field protected "class_with_protected_typedef::INT b;" } + { typedef protected "typedef int INT;" } + } + cp_test_ptype_class \ + "struct struct_with_protected_typedef" "" "struct" \ + "struct_with_protected_typedef" { + { field public "int a;" } + { field protected "struct_with_protected_typedef::INT b;" } + { typedef protected "typedef int INT;" } + } + cp_test_ptype_class \ + "struct struct_with_private_typedef" "" "struct" \ + "struct_with_private_typedef" { + { field public "int a;" } + { field private "struct_with_private_typedef::INT b;" } + { typedef private "typedef int INT;" } + } + + # For the following two cases, we cannot use cp_test_ptype_class. + # We need to explicitly check whether the access label was suppressed. + set ws {[\ \t\r\n]*} + foreach {tag lbl} {"class" "private" "struct" "public"} { + gdb_test "ptype/r ${tag}_with_${lbl}_typedef" "type = $tag ${tag}_with_${lbl}_typedef \{${ws}int a;${ws}${tag}_with_${lbl}_typedef::INT b;${ws}typedef int INT;${ws}\}" + } } # Test simple access to class members. -- 2.1.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Record and output access specifiers for nested typedefs 2017-10-13 19:18 [PATCH] Record and output access specifiers for nested typedefs Keith Seitz @ 2017-10-16 19:57 ` Simon Marchi 2017-10-16 21:32 ` Keith Seitz 0 siblings, 1 reply; 8+ messages in thread From: Simon Marchi @ 2017-10-16 19:57 UTC (permalink / raw) To: Keith Seitz, gdb-patches On 2017-10-13 03:18 PM, Keith Seitz wrote: > We currently do not record access information for typedefs defined inside > classes. Consider: > > struct foo > { > typedef int PUBLIC; > private: > typedef int PRIVATE; > PRIVATE b; > }; > > (gdb) ptype foo > type = struct foo { > private: > PRIVATE b; > > typedef int PRIVATE; > typedef int PUBLIC; > } > > This patch fixes this: > > (gdb) ptype foo > type = struct foo { > private: > PRIVATE b; > > typedef int PRIVATE; > public: > typedef int PUBLIC; > } Hi Keith, I just have a few minor comments. > gdb/ChangeLog: > > * c-typeprint.c (enum access_specifier): Moved here from > c_type_print_base. > (output_access_specifier): New function. > (c_type_print_base): Consider typedefs when assessing > whether access labels are needed. > Use output_access_specifier as needed. > Output access specifier for typedefs, if needed. > * dwarf2read.c (dwarf2_add_typedef): Record DW_AT_accessibility. > * gdbtypes.h (struct typedef_field) <is_protected, is_private>: New > fields. > (TYPE_TYPEDEF_FIELD_PROTECTED, TYPE_TYEPDEF_FIELD_PRIVATE): New "TYPE_TYEPDEF_FIELD_PRIVATE" > accessor macros. > > gdb/testsuite/ChangeLog: > > * gdb.cp/classes.cc (class_with_typedefs, class_with_public_typedef) > (class_with_protected_typedef, class_with_private_typedef) > (struct_with_public_typedef, struct_with_protected_typedef) > (struct_with_private_typedef): New classes/structs. > * gdb.cp/classes.exp (test_ptype_class_objects): Add tests for > typedefs and access specifiers. > --- > gdb/c-typeprint.c | 145 +++++++++++++++++++++++---------------- > gdb/dwarf2read.c | 22 ++++++ > gdb/gdbtypes.h | 11 +++ > gdb/testsuite/gdb.cp/classes.cc | 83 ++++++++++++++++++++++ > gdb/testsuite/gdb.cp/classes.exp | 58 ++++++++++++++++ > 5 files changed, 259 insertions(+), 60 deletions(-) > > diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c > index 978fcc4..3dc74f9 100644 > --- a/gdb/c-typeprint.c > +++ b/gdb/c-typeprint.c > @@ -32,6 +32,16 @@ > #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_prefix (struct type *, > struct ui_file *, > int, int, int, > @@ -826,7 +836,42 @@ c_type_print_template_args (const struct type_print_options *flags, > fputs_filtered (_("] "), stream); > } > > -/* Print the name of the type (or the ultimate pointer target, > +/* Output an access specifier to STREAM, if needed. */ > + > +static void > +output_access_specifier (struct ui_file *stream, enum access_specifier &access, > + int level, bool is_protected, bool is_private) I would suggest renaming access it to last_access or previous_access. Could you also document it in the function comment? It might also be clearer to have output_access_specifier return the access_specifier, rather than modifying the passed value directly. Otherwise, it's not really obvious what's happening in the caller. > +{ > + if (is_protected) > + { > + if (access != s_protected) > + { > + access = s_protected; > + fprintfi_filtered (level + 2, stream, > + "protected:\n"); > + } > + } > + else if (is_private) > + { > + if (access != s_private) > + { > + access = s_private; > + fprintfi_filtered (level + 2, stream, > + "private:\n"); > + } > + } > + else > + { > + if (access != s_public) > + { > + access = s_public; > + fprintfi_filtered (level + 2, stream, > + "public:\n"); > + } > + } > +} > + > + /* Print the name of the type (or the ultimate pointer target, Spurious indent change on that last line. > function value or array element), or the description of a structure > or union. > > @@ -850,11 +895,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, > { > int i; > int len, real_len; > - enum > - { > - s_none, s_public, s_private, s_protected > - } > - section_type; > + enum access_specifier section_type; > int need_access_label = 0; > int j, len2; > > @@ -1034,6 +1075,18 @@ c_type_print_base (struct type *type, struct ui_file *stream, > break; > } > } > + QUIT; > + if (!need_access_label) > + { > + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i) > + { > + if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i)) > + { > + need_access_label = 1; > + break; > + } > + } > + } > } > else > { > @@ -1068,6 +1121,19 @@ c_type_print_base (struct type *type, struct ui_file *stream, > break; > } > } > + QUIT; > + if (!need_access_label) > + { > + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i) > + { > + if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i) > + || TYPE_TYPEDEF_FIELD_PRIVATE (type, i)) > + { > + need_access_label = 1; > + break; > + } > + } > + } > } > > /* If there is a base class for this type, > @@ -1088,33 +1154,9 @@ c_type_print_base (struct type *type, struct ui_file *stream, > > if (need_access_label) > { > - if (TYPE_FIELD_PROTECTED (type, i)) > - { > - if (section_type != s_protected) > - { > - section_type = s_protected; > - fprintfi_filtered (level + 2, stream, > - "protected:\n"); > - } > - } > - else if (TYPE_FIELD_PRIVATE (type, i)) > - { > - if (section_type != s_private) > - { > - section_type = s_private; > - fprintfi_filtered (level + 2, stream, > - "private:\n"); > - } > - } > - else > - { > - if (section_type != s_public) > - { > - section_type = s_public; > - fprintfi_filtered (level + 2, stream, > - "public:\n"); > - } > - } > + output_access_specifier (stream, section_type, level, > + TYPE_FIELD_PROTECTED (type, i), > + TYPE_FIELD_PRIVATE (type, i)); > } > > print_spaces_filtered (level + 4, stream); > @@ -1188,33 +1230,9 @@ c_type_print_base (struct type *type, struct ui_file *stream, > inner_cleanup = make_cleanup (null_cleanup, NULL); > > QUIT; > - if (TYPE_FN_FIELD_PROTECTED (f, j)) > - { > - if (section_type != s_protected) > - { > - section_type = s_protected; > - fprintfi_filtered (level + 2, stream, > - "protected:\n"); > - } > - } > - else if (TYPE_FN_FIELD_PRIVATE (f, j)) > - { > - if (section_type != s_private) > - { > - section_type = s_private; > - fprintfi_filtered (level + 2, stream, > - "private:\n"); > - } > - } > - else > - { > - if (section_type != s_public) > - { > - section_type = s_public; > - fprintfi_filtered (level + 2, stream, > - "public:\n"); > - } > - } > + output_access_specifier (stream, section_type, level, > + TYPE_FN_FIELD_PROTECTED (f, j), > + TYPE_FN_FIELD_PRIVATE (f, j)); > > print_spaces_filtered (level + 4, stream); > if (TYPE_FN_FIELD_VIRTUAL_P (f, j)) > @@ -1326,6 +1344,13 @@ c_type_print_base (struct type *type, struct ui_file *stream, > gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF); > target = TYPE_TARGET_TYPE (target); > > + if (need_access_label) > + { > + output_access_specifier > + (stream, section_type, level, > + TYPE_TYPEDEF_FIELD_PROTECTED (type, i), > + TYPE_TYPEDEF_FIELD_PRIVATE (type, i)); > + } > print_spaces_filtered (level + 4, stream); > fprintf_filtered (stream, "typedef "); > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 9bdb75d..c0200f9 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -13274,6 +13274,28 @@ dwarf2_add_typedef (struct field_info *fip, struct die_info *die, > > fp->type = read_type_die (die, cu); > > + /* Save accessibility. */ > + enum dwarf_access_attribute accessibility; > + struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu); > + if (attr != NULL) > + accessibility = (enum dwarf_access_attribute) DW_UNSND (attr); > + else > + accessibility = dwarf2_default_access_attribute (die, cu); > + switch (accessibility) > + { > + case DW_ACCESS_public: > + /* The assumed value if neither private nor protected. */ > + break; > + case DW_ACCESS_private: > + fp->is_private = 1; > + break; > + case DW_ACCESS_protected: > + fp->is_protected = 1; > + break; > + default: > + gdb_assert_not_reached ("unexpected accessibility attribute"); > + } > + > new_field->next = fip->typedef_field_list; > fip->typedef_field_list = new_field; > fip->typedef_field_list_count++; > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h > index 0e0aead..48af6f1 100644 > --- a/gdb/gdbtypes.h > +++ b/gdb/gdbtypes.h > @@ -896,6 +896,12 @@ struct typedef_field > /* * Type this typedef named NAME represents. */ > > 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; > }; > > /* * C++ language-specific information for TYPE_CODE_STRUCT and > @@ -1427,6 +1433,7 @@ extern void set_type_vptr_basetype (struct type *, struct type *); > #define TYPE_FN_FIELD_VIRTUAL_P(thisfn, n) ((thisfn)[n].voffset > 1) > #define TYPE_FN_FIELD_STATIC_P(thisfn, n) ((thisfn)[n].voffset == VOFFSET_STATIC) > > +/* Accessors for typedefs defined by a class. */ > #define TYPE_TYPEDEF_FIELD_ARRAY(thistype) \ > TYPE_CPLUS_SPECIFIC (thistype)->typedef_field > #define TYPE_TYPEDEF_FIELD(thistype, n) \ > @@ -1437,6 +1444,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *); > TYPE_TYPEDEF_FIELD (thistype, n).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 > +#define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n) \ > + TYPE_TYPEDEF_FIELD (thistype, n).is_private > > #define TYPE_IS_OPAQUE(thistype) \ > (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \ > diff --git a/gdb/testsuite/gdb.cp/classes.cc b/gdb/testsuite/gdb.cp/classes.cc > index 2a81473..50f0740 100644 > --- a/gdb/testsuite/gdb.cp/classes.cc > +++ b/gdb/testsuite/gdb.cp/classes.cc > @@ -545,6 +545,82 @@ small::method () > return x + 5; > } > > +class class_with_typedefs > +{ > +public: > + typedef int public_int; > +protected: > + typedef int protected_int; > +private: > + typedef int private_int; > + > +public: > + class_with_typedefs () > + : public_int_ (1), protected_int_ (2), private_int_ (3) {} > + public_int add_public (public_int a) { return a + public_int_; } > + public_int add_all (int a) > + { return add_public (a) + add_protected (a) + add_private (a); } > + > +protected: > + protected_int add_protected (protected_int a) { return a + protected_int_; } > + > +private: > + private_int add_private (private_int a) { return a + private_int_; } > + > +protected: > + public_int public_int_; > + protected_int protected_int_; > + private_int private_int_; > +}; > + > +class class_with_public_typedef > +{ > + int a; > +public: > + typedef int INT; > + INT b; > +}; > + > +class class_with_protected_typedef > +{ > + int a; > +protected: > + typedef int INT; > + INT b; > +}; > + > +class class_with_private_typedef > +{ > + int a; > +private: > + typedef int INT; > + INT b; > +}; > + > +struct struct_with_public_typedef > +{ > + int a; > +public: > + typedef int INT; > + INT b; > +}; > + > +struct struct_with_protected_typedef > +{ > + int a; > +protected: > + typedef int INT; > + INT b; > +}; > + > +struct struct_with_private_typedef > +{ > + int a; > +private: > + typedef int INT; > + INT b; > +}; > + > void marker_reg1 () {} > > int > @@ -624,3 +700,10 @@ protected_class protected_c; > default_private_class default_private_c; > explicit_private_class explicit_private_c; > mixed_protection_class mixed_protection_c; > +class_with_typedefs class_with_typedefs_c; > +class_with_public_typedef class_with_public_typedef_c; > +class_with_protected_typedef class_with_protected_typedef_c; > +class_with_private_typedef class_with_private_typedef_c; > +struct_with_public_typedef struct_with_public_typedef_s; > +struct_with_protected_typedef struct_with_protected_typedef_s; > +struct_with_private_typedef struct_with_private_typedef_s; > diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp > index 256fa68..1f6d377 100644 > --- a/gdb/testsuite/gdb.cp/classes.exp > +++ b/gdb/testsuite/gdb.cp/classes.exp > @@ -316,6 +316,64 @@ proc test_ptype_class_objects {} { > { field public "int y;" } > { method public "DynamicBar(int, int);" } > } > + > + # Classes with tyepdefs of different access. "tyepdefs". > + > + cp_test_ptype_class \ > + "class class_with_typedefs" "" "class" "class_with_typedefs" \ > + { > + { field protected \ > + "class_with_typedefs::public_int public_int_;" } > + { field protected \ > + "class_with_typedefs::protected_int protected_int_;" } > + { field protected \ > + "class_with_typedefs::private_int private_int_;" } > + { method public "class_with_typedefs(void);" } > + { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" } > + { method public \ > + "class_with_typedefs::public_int add_all(int);" } > + { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" } > + { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" } > + {typedef public "typedef int public_int;" } > + {typedef protected "typedef int protected_int;" } > + {typedef private "typedef int private_int;" } Add spaces after {, for consistency. > + } > + > + cp_test_ptype_class \ > + "class class_with_public_typedef" "" "class" \ > + "class_with_public_typedef" { > + { field private "int a;" } > + { field public "class_with_public_typedef::INT b;" } > + { typedef public "typedef int INT;" } > + } > + cp_test_ptype_class \ > + "class class_with_protected_typedef" "" "class" \ > + "class_with_protected_typedef" { > + { field private "int a;" } > + { field protected "class_with_protected_typedef::INT b;" } > + { typedef protected "typedef int INT;" } > + } > + cp_test_ptype_class \ > + "struct struct_with_protected_typedef" "" "struct" \ > + "struct_with_protected_typedef" { > + { field public "int a;" } > + { field protected "struct_with_protected_typedef::INT b;" } > + { typedef protected "typedef int INT;" } > + } > + cp_test_ptype_class \ > + "struct struct_with_private_typedef" "" "struct" \ > + "struct_with_private_typedef" { > + { field public "int a;" } > + { field private "struct_with_private_typedef::INT b;" } > + { typedef private "typedef int INT;" } > + } > + > + # For the following two cases, we cannot use cp_test_ptype_class. > + # We need to explicitly check whether the access label was suppressed. > + set ws {[\ \t\r\n]*} I don't think it's necessary to escape the space. > + foreach {tag lbl} {"class" "private" "struct" "public"} { > + gdb_test "ptype/r ${tag}_with_${lbl}_typedef" "type = $tag ${tag}_with_${lbl}_typedef \{${ws}int a;${ws}${tag}_with_${lbl}_typedef::INT b;${ws}typedef int INT;${ws}\}" > + } > } > > # Test simple access to class members. > Thanks! Simon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Record and output access specifiers for nested typedefs 2017-10-16 19:57 ` Simon Marchi @ 2017-10-16 21:32 ` Keith Seitz 2017-10-16 21:40 ` Simon Marchi 2017-10-17 1:05 ` Pedro Alves 0 siblings, 2 replies; 8+ messages in thread From: Keith Seitz @ 2017-10-16 21:32 UTC (permalink / raw) To: simon.marchi; +Cc: gdb-patches On 10/16/2017 12:56 PM, Simon Marchi wrote: > On 2017-10-13 03:18 PM, Keith Seitz wrote: >> gdb/ChangeLog: >> >> * c-typeprint.c (enum access_specifier): Moved here from >> c_type_print_base. >> (output_access_specifier): New function. >> (c_type_print_base): Consider typedefs when assessing >> whether access labels are needed. >> Use output_access_specifier as needed. >> Output access specifier for typedefs, if needed. >> * dwarf2read.c (dwarf2_add_typedef): Record DW_AT_accessibility. >> * gdbtypes.h (struct typedef_field) <is_protected, is_private>: New >> fields. >> (TYPE_TYPEDEF_FIELD_PROTECTED, TYPE_TYEPDEF_FIELD_PRIVATE): New > "TYPE_TYEPDEF_FIELD_PRIVATE" > Fixed. >> accessor macros. >> >> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c >> index 978fcc4..3dc74f9 100644 >> --- a/gdb/c-typeprint.c >> +++ b/gdb/c-typeprint.c >> @@ -826,7 +836,42 @@ c_type_print_template_args (const struct type_print_options *flags, >> fputs_filtered (_("] "), stream); >> } >> >> -/* Print the name of the type (or the ultimate pointer target, >> +/* Output an access specifier to STREAM, if needed. */ >> + >> +static void >> +output_access_specifier (struct ui_file *stream, enum access_specifier &access, >> + int level, bool is_protected, bool is_private) > I would suggest renaming access it to last_access or previous_access. Could you also document > it in the function comment? Done. > It might also be clearer to have output_access_specifier return the access_specifier, > rather than modifying the passed value directly. Otherwise, it's not really obvious > what's happening in the caller. Also done. >> + /* Print the name of the type (or the ultimate pointer target, > Spurious indent change on that last line. Removed. >> diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp >> index 256fa68..1f6d377 100644 >> --- a/gdb/testsuite/gdb.cp/classes.exp >> +++ b/gdb/testsuite/gdb.cp/classes.exp >> @@ -316,6 +316,64 @@ proc test_ptype_class_objects {} { >> { field public "int y;" } >> { method public "DynamicBar(int, int);" } >> } >> + >> + # Classes with tyepdefs of different access. > "tyepdefs". Darn fingers. Fixed. > >> + >> + cp_test_ptype_class \ >> + "class class_with_typedefs" "" "class" "class_with_typedefs" \ >> + { >> + { field protected \ >> + "class_with_typedefs::public_int public_int_;" } >> + { field protected \ >> + "class_with_typedefs::protected_int protected_int_;" } >> + { field protected \ >> + "class_with_typedefs::private_int private_int_;" } >> + { method public "class_with_typedefs(void);" } >> + { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" } >> + { method public \ >> + "class_with_typedefs::public_int add_all(int);" } >> + { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" } >> + { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" } >> + {typedef public "typedef int public_int;" } >> + {typedef protected "typedef int protected_int;" } >> + {typedef private "typedef int private_int;" } > Add spaces after {, for consistency. Also fixed. [How'd I not see that?!] >> + # For the following two cases, we cannot use cp_test_ptype_class. >> + # We need to explicitly check whether the access label was suppressed. >> + set ws {[\ \t\r\n]*} > I don't think it's necessary to escape the space. > Indeed not. Removed. Thank you for the reivew. Keith diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c index 978fcc4..970b6a0 100644 --- a/gdb/c-typeprint.c +++ b/gdb/c-typeprint.c @@ -32,6 +32,16 @@ #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_prefix (struct type *, struct ui_file *, int, int, int, @@ -826,6 +836,45 @@ c_type_print_template_args (const struct type_print_options *flags, fputs_filtered (_("] "), stream); } +/* 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 +output_access_specifier (struct ui_file *stream, + enum access_specifier last_access, + int level, bool is_protected, bool is_private) +{ + if (is_protected) + { + if (last_access != s_protected) + { + last_access = s_protected; + fprintfi_filtered (level + 2, stream, + "protected:\n"); + } + } + else if (is_private) + { + if (last_access != s_private) + { + last_access = s_private; + fprintfi_filtered (level + 2, stream, + "private:\n"); + } + } + else + { + if (last_access != s_public) + { + last_access = s_public; + fprintfi_filtered (level + 2, stream, + "public:\n"); + } + } + + return last_access; +} + /* Print the name of the type (or the ultimate pointer target, function value or array element), or the description of a structure or union. @@ -850,11 +899,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, { int i; int len, real_len; - enum - { - s_none, s_public, s_private, s_protected - } - section_type; + enum access_specifier section_type; int need_access_label = 0; int j, len2; @@ -1034,6 +1079,18 @@ c_type_print_base (struct type *type, struct ui_file *stream, break; } } + QUIT; + if (!need_access_label) + { + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i) + { + if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i)) + { + need_access_label = 1; + break; + } + } + } } else { @@ -1068,6 +1125,19 @@ c_type_print_base (struct type *type, struct ui_file *stream, break; } } + QUIT; + if (!need_access_label) + { + for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i) + { + if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i) + || TYPE_TYPEDEF_FIELD_PRIVATE (type, i)) + { + need_access_label = 1; + break; + } + } + } } /* If there is a base class for this type, @@ -1088,33 +1158,10 @@ c_type_print_base (struct type *type, struct ui_file *stream, if (need_access_label) { - if (TYPE_FIELD_PROTECTED (type, i)) - { - if (section_type != s_protected) - { - section_type = s_protected; - fprintfi_filtered (level + 2, stream, - "protected:\n"); - } - } - else if (TYPE_FIELD_PRIVATE (type, i)) - { - if (section_type != s_private) - { - section_type = s_private; - fprintfi_filtered (level + 2, stream, - "private:\n"); - } - } - else - { - if (section_type != s_public) - { - section_type = s_public; - fprintfi_filtered (level + 2, stream, - "public:\n"); - } - } + section_type = output_access_specifier + (stream, section_type, level, + TYPE_FIELD_PROTECTED (type, i), + TYPE_FIELD_PRIVATE (type, i)); } print_spaces_filtered (level + 4, stream); @@ -1188,33 +1235,10 @@ c_type_print_base (struct type *type, struct ui_file *stream, inner_cleanup = make_cleanup (null_cleanup, NULL); QUIT; - if (TYPE_FN_FIELD_PROTECTED (f, j)) - { - if (section_type != s_protected) - { - section_type = s_protected; - fprintfi_filtered (level + 2, stream, - "protected:\n"); - } - } - else if (TYPE_FN_FIELD_PRIVATE (f, j)) - { - if (section_type != s_private) - { - section_type = s_private; - fprintfi_filtered (level + 2, stream, - "private:\n"); - } - } - else - { - if (section_type != s_public) - { - section_type = s_public; - fprintfi_filtered (level + 2, stream, - "public:\n"); - } - } + section_type = output_access_specifier + (stream, section_type, level, + TYPE_FN_FIELD_PROTECTED (f, j), + TYPE_FN_FIELD_PRIVATE (f, j)); print_spaces_filtered (level + 4, stream); if (TYPE_FN_FIELD_VIRTUAL_P (f, j)) @@ -1326,6 +1350,13 @@ c_type_print_base (struct type *type, struct ui_file *stream, gdb_assert (TYPE_CODE (target) == TYPE_CODE_TYPEDEF); target = TYPE_TARGET_TYPE (target); + 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)); + } print_spaces_filtered (level + 4, stream); fprintf_filtered (stream, "typedef "); diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 9bdb75d..c0200f9 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -13274,6 +13274,28 @@ dwarf2_add_typedef (struct field_info *fip, struct die_info *die, fp->type = read_type_die (die, cu); + /* Save accessibility. */ + enum dwarf_access_attribute accessibility; + struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu); + if (attr != NULL) + accessibility = (enum dwarf_access_attribute) DW_UNSND (attr); + else + accessibility = dwarf2_default_access_attribute (die, cu); + switch (accessibility) + { + case DW_ACCESS_public: + /* The assumed value if neither private nor protected. */ + break; + case DW_ACCESS_private: + fp->is_private = 1; + break; + case DW_ACCESS_protected: + fp->is_protected = 1; + break; + default: + gdb_assert_not_reached ("unexpected accessibility attribute"); + } + new_field->next = fip->typedef_field_list; fip->typedef_field_list = new_field; fip->typedef_field_list_count++; diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 0e0aead..48af6f1 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -896,6 +896,12 @@ struct typedef_field /* * Type this typedef named NAME represents. */ 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; }; /* * C++ language-specific information for TYPE_CODE_STRUCT and @@ -1427,6 +1433,7 @@ extern void set_type_vptr_basetype (struct type *, struct type *); #define TYPE_FN_FIELD_VIRTUAL_P(thisfn, n) ((thisfn)[n].voffset > 1) #define TYPE_FN_FIELD_STATIC_P(thisfn, n) ((thisfn)[n].voffset == VOFFSET_STATIC) +/* Accessors for typedefs defined by a class. */ #define TYPE_TYPEDEF_FIELD_ARRAY(thistype) \ TYPE_CPLUS_SPECIFIC (thistype)->typedef_field #define TYPE_TYPEDEF_FIELD(thistype, n) \ @@ -1437,6 +1444,10 @@ extern void set_type_vptr_basetype (struct type *, struct type *); TYPE_TYPEDEF_FIELD (thistype, n).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 +#define TYPE_TYPEDEF_FIELD_PRIVATE(thistype, n) \ + TYPE_TYPEDEF_FIELD (thistype, n).is_private #define TYPE_IS_OPAQUE(thistype) \ (((TYPE_CODE (thistype) == TYPE_CODE_STRUCT) \ diff --git a/gdb/testsuite/gdb.cp/classes.cc b/gdb/testsuite/gdb.cp/classes.cc index 2a81473..50f0740 100644 --- a/gdb/testsuite/gdb.cp/classes.cc +++ b/gdb/testsuite/gdb.cp/classes.cc @@ -545,6 +545,82 @@ small::method () return x + 5; } +class class_with_typedefs +{ +public: + typedef int public_int; +protected: + typedef int protected_int; +private: + typedef int private_int; + +public: + class_with_typedefs () + : public_int_ (1), protected_int_ (2), private_int_ (3) {} + public_int add_public (public_int a) { return a + public_int_; } + public_int add_all (int a) + { return add_public (a) + add_protected (a) + add_private (a); } + +protected: + protected_int add_protected (protected_int a) { return a + protected_int_; } + +private: + private_int add_private (private_int a) { return a + private_int_; } + +protected: + public_int public_int_; + protected_int protected_int_; + private_int private_int_; +}; + +class class_with_public_typedef +{ + int a; +public: + typedef int INT; + INT b; +}; + +class class_with_protected_typedef +{ + int a; +protected: + typedef int INT; + INT b; +}; + +class class_with_private_typedef +{ + int a; +private: + typedef int INT; + INT b; +}; + +struct struct_with_public_typedef +{ + int a; +public: + typedef int INT; + INT b; +}; + +struct struct_with_protected_typedef +{ + int a; +protected: + typedef int INT; + INT b; +}; + +struct struct_with_private_typedef +{ + int a; +private: + typedef int INT; + INT b; +}; + void marker_reg1 () {} int @@ -624,3 +700,10 @@ protected_class protected_c; default_private_class default_private_c; explicit_private_class explicit_private_c; mixed_protection_class mixed_protection_c; +class_with_typedefs class_with_typedefs_c; +class_with_public_typedef class_with_public_typedef_c; +class_with_protected_typedef class_with_protected_typedef_c; +class_with_private_typedef class_with_private_typedef_c; +struct_with_public_typedef struct_with_public_typedef_s; +struct_with_protected_typedef struct_with_protected_typedef_s; +struct_with_private_typedef struct_with_private_typedef_s; diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp index 256fa68..9e2630a 100644 --- a/gdb/testsuite/gdb.cp/classes.exp +++ b/gdb/testsuite/gdb.cp/classes.exp @@ -316,6 +316,64 @@ proc test_ptype_class_objects {} { { field public "int y;" } { method public "DynamicBar(int, int);" } } + + # Classes with typedefs of different access. + + cp_test_ptype_class \ + "class class_with_typedefs" "" "class" "class_with_typedefs" \ + { + { field protected \ + "class_with_typedefs::public_int public_int_;" } + { field protected \ + "class_with_typedefs::protected_int protected_int_;" } + { field protected \ + "class_with_typedefs::private_int private_int_;" } + { method public "class_with_typedefs(void);" } + { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" } + { method public \ + "class_with_typedefs::public_int add_all(int);" } + { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" } + { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" } + { typedef public "typedef int public_int;" } + { typedef protected "typedef int protected_int;" } + { typedef private "typedef int private_int;" } + } + + cp_test_ptype_class \ + "class class_with_public_typedef" "" "class" \ + "class_with_public_typedef" { + { field private "int a;" } + { field public "class_with_public_typedef::INT b;" } + { typedef public "typedef int INT;" } + } + cp_test_ptype_class \ + "class class_with_protected_typedef" "" "class" \ + "class_with_protected_typedef" { + { field private "int a;" } + { field protected "class_with_protected_typedef::INT b;" } + { typedef protected "typedef int INT;" } + } + cp_test_ptype_class \ + "struct struct_with_protected_typedef" "" "struct" \ + "struct_with_protected_typedef" { + { field public "int a;" } + { field protected "struct_with_protected_typedef::INT b;" } + { typedef protected "typedef int INT;" } + } + cp_test_ptype_class \ + "struct struct_with_private_typedef" "" "struct" \ + "struct_with_private_typedef" { + { field public "int a;" } + { field private "struct_with_private_typedef::INT b;" } + { typedef private "typedef int INT;" } + } + + # For the following two cases, we cannot use cp_test_ptype_class. + # We need to explicitly check whether the access label was suppressed. + set ws {[ \t\r\n]*} + foreach {tag lbl} {"class" "private" "struct" "public"} { + gdb_test "ptype/r ${tag}_with_${lbl}_typedef" "type = $tag ${tag}_with_${lbl}_typedef \{${ws}int a;${ws}${tag}_with_${lbl}_typedef::INT b;${ws}typedef int INT;${ws}\}" + } } # Test simple access to class members. -- 2.1.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Record and output access specifiers for nested typedefs 2017-10-16 21:32 ` Keith Seitz @ 2017-10-16 21:40 ` Simon Marchi 2017-10-17 0:56 ` Keith Seitz 2017-10-17 1:05 ` Pedro Alves 1 sibling, 1 reply; 8+ messages in thread From: Simon Marchi @ 2017-10-16 21:40 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches On 2017-10-16 05:32 PM, Keith Seitz wrote: > On 10/16/2017 12:56 PM, Simon Marchi wrote: >> On 2017-10-13 03:18 PM, Keith Seitz wrote: >>> gdb/ChangeLog: >>> >>> * c-typeprint.c (enum access_specifier): Moved here from >>> c_type_print_base. >>> (output_access_specifier): New function. >>> (c_type_print_base): Consider typedefs when assessing >>> whether access labels are needed. >>> Use output_access_specifier as needed. >>> Output access specifier for typedefs, if needed. >>> * dwarf2read.c (dwarf2_add_typedef): Record DW_AT_accessibility. >>> * gdbtypes.h (struct typedef_field) <is_protected, is_private>: New >>> fields. >>> (TYPE_TYPEDEF_FIELD_PROTECTED, TYPE_TYEPDEF_FIELD_PRIVATE): New >> "TYPE_TYEPDEF_FIELD_PRIVATE" >> > Fixed. > >>> accessor macros. >>> >>> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c >>> index 978fcc4..3dc74f9 100644 >>> --- a/gdb/c-typeprint.c >>> +++ b/gdb/c-typeprint.c >>> @@ -826,7 +836,42 @@ c_type_print_template_args (const struct type_print_options *flags, >>> fputs_filtered (_("] "), stream); >>> } >>> >>> -/* Print the name of the type (or the ultimate pointer target, >>> +/* Output an access specifier to STREAM, if needed. */ >>> + >>> +static void >>> +output_access_specifier (struct ui_file *stream, enum access_specifier &access, >>> + int level, bool is_protected, bool is_private) >> I would suggest renaming access it to last_access or previous_access. Could you also document >> it in the function comment? > Done. > >> It might also be clearer to have output_access_specifier return the access_specifier, >> rather than modifying the passed value directly. Otherwise, it's not really obvious >> what's happening in the caller. > Also done. > >>> + /* Print the name of the type (or the ultimate pointer target, >> Spurious indent change on that last line. > Removed. > >>> diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp >>> index 256fa68..1f6d377 100644 >>> --- a/gdb/testsuite/gdb.cp/classes.exp >>> +++ b/gdb/testsuite/gdb.cp/classes.exp >>> @@ -316,6 +316,64 @@ proc test_ptype_class_objects {} { >>> { field public "int y;" } >>> { method public "DynamicBar(int, int);" } >>> } >>> + >>> + # Classes with tyepdefs of different access. >> "tyepdefs". > Darn fingers. Fixed. > >> >>> + >>> + cp_test_ptype_class \ >>> + "class class_with_typedefs" "" "class" "class_with_typedefs" \ >>> + { >>> + { field protected \ >>> + "class_with_typedefs::public_int public_int_;" } >>> + { field protected \ >>> + "class_with_typedefs::protected_int protected_int_;" } >>> + { field protected \ >>> + "class_with_typedefs::private_int private_int_;" } >>> + { method public "class_with_typedefs(void);" } >>> + { method public "class_with_typedefs::public_int add_public(class_with_typedefs::public_int);" } >>> + { method public \ >>> + "class_with_typedefs::public_int add_all(int);" } >>> + { method protected "class_with_typedefs::protected_int add_protected(class_with_typedefs::protected_int);" } >>> + { method private "class_with_typedefs::private_int add_private(class_with_typedefs::private_int);" } >>> + {typedef public "typedef int public_int;" } >>> + {typedef protected "typedef int protected_int;" } >>> + {typedef private "typedef int private_int;" } >> Add spaces after {, for consistency. > Also fixed. [How'd I not see that?!] > >>> + # For the following two cases, we cannot use cp_test_ptype_class. >>> + # We need to explicitly check whether the access label was suppressed. >>> + set ws {[\ \t\r\n]*} >> I don't think it's necessary to escape the space. >> > Indeed not. Removed. > > Thank you for the reivew. > > Keith Great, LGTM. Simon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Record and output access specifiers for nested typedefs 2017-10-16 21:40 ` Simon Marchi @ 2017-10-17 0:56 ` Keith Seitz 0 siblings, 0 replies; 8+ messages in thread From: Keith Seitz @ 2017-10-17 0:56 UTC (permalink / raw) To: gdb-patches On 10/16/2017 02:40 PM, Simon Marchi wrote: > > Great, LGTM. > Pushed. Thank you again for the review. Keith ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Record and output access specifiers for nested typedefs 2017-10-16 21:32 ` Keith Seitz 2017-10-16 21:40 ` Simon Marchi @ 2017-10-17 1:05 ` Pedro Alves 2017-10-17 21:38 ` [PATCH] Issue complaint instead of assert for invalid/unhandled DW_AT_accessibility Keith Seitz 1 sibling, 1 reply; 8+ messages in thread From: Pedro Alves @ 2017-10-17 1:05 UTC (permalink / raw) To: Keith Seitz, simon.marchi; +Cc: gdb-patches On 10/16/2017 10:32 PM, Keith Seitz wrote: > @@ -13274,6 +13274,28 @@ dwarf2_add_typedef (struct field_info *fip, struct die_info *die, > > fp->type = read_type_die (die, cu); > > + /* Save accessibility. */ > + enum dwarf_access_attribute accessibility; > + struct attribute *attr = dwarf2_attr (die, DW_AT_accessibility, cu); > + if (attr != NULL) > + accessibility = (enum dwarf_access_attribute) DW_UNSND (attr); > + else > + accessibility = dwarf2_default_access_attribute (die, cu); > + switch (accessibility) > + { > + case DW_ACCESS_public: > + /* The assumed value if neither private nor protected. */ > + break; > + case DW_ACCESS_private: > + fp->is_private = 1; > + break; > + case DW_ACCESS_protected: > + fp->is_protected = 1; > + break; > + default: > + gdb_assert_not_reached ("unexpected accessibility attribute"); > + } > + Please don't gdb_assert on invalid input. Bogus input caused by a buggy compiler shouldn't bring down gdb. (I can't claim that I've really read the patch; I've barely skimmed it, there may be other instances. Please double check.) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] Issue complaint instead of assert for invalid/unhandled DW_AT_accessibility 2017-10-17 1:05 ` Pedro Alves @ 2017-10-17 21:38 ` Keith Seitz 2017-10-18 10:21 ` Pedro Alves 0 siblings, 1 reply; 8+ messages in thread From: Keith Seitz @ 2017-10-17 21:38 UTC (permalink / raw) To: gdb-patches A previous patch called gdb_assert_not_reached whenever reading the accessibility of a nested typedef definition. Wisely, Pedro has asked me not do this. This patch changes the previous one so that it issues a complaint instead. gdb/ChangeLog: * dwarf2read.c (dwarf2_add_typedef): Issue a complaint on unhandled DW_AT_accessibility. --- gdb/ChangeLog | 5 +++++ gdb/dwarf2read.c | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index e16b048..4afd897 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2017-10-17 Keith Seitz <keiths@redhat.com> + + * dwarf2read.c (dwarf2_add_typedef): Issue a complaint on unhandled + DW_AT_accessibility. + 2017-10-17 Tom Tromey <tom@tromey.com> * disasm.c (do_mixed_source_and_assembly_deprecated): Use diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index f27d9b9..686fa3f 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -13131,7 +13131,8 @@ dwarf2_add_typedef (struct field_info *fip, struct die_info *die, fp->is_protected = 1; break; default: - gdb_assert_not_reached ("unexpected accessibility attribute"); + complaint (&symfile_complaints, + _("Unhandled DW_AT_accessibility value (%x)"), accessibility); } new_field->next = fip->typedef_field_list; -- 2.1.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Issue complaint instead of assert for invalid/unhandled DW_AT_accessibility 2017-10-17 21:38 ` [PATCH] Issue complaint instead of assert for invalid/unhandled DW_AT_accessibility Keith Seitz @ 2017-10-18 10:21 ` Pedro Alves 0 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2017-10-18 10:21 UTC (permalink / raw) To: Keith Seitz, gdb-patches On 10/17/2017 10:38 PM, Keith Seitz wrote: > A previous patch called gdb_assert_not_reached whenever reading > the accessibility of a nested typedef definition. Wisely, Pedro has asked me > not do this. > > This patch changes the previous one so that it issues a complaint instead. > > gdb/ChangeLog: > > * dwarf2read.c (dwarf2_add_typedef): Issue a complaint on unhandled > DW_AT_accessibility. OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-18 10:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-13 19:18 [PATCH] Record and output access specifiers for nested typedefs Keith Seitz 2017-10-16 19:57 ` Simon Marchi 2017-10-16 21:32 ` Keith Seitz 2017-10-16 21:40 ` Simon Marchi 2017-10-17 0:56 ` Keith Seitz 2017-10-17 1:05 ` Pedro Alves 2017-10-17 21:38 ` [PATCH] Issue complaint instead of assert for invalid/unhandled DW_AT_accessibility Keith Seitz 2017-10-18 10:21 ` Pedro Alves
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).