From patchwork Mon Oct 16 21:32:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 23627 Received: (qmail 27395 invoked by alias); 16 Oct 2017 21:32:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27384 invoked by uid 89); 16 Oct 2017 21:32:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=fingers, darn X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Oct 2017 21:32:20 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 07F09356F3; Mon, 16 Oct 2017 21:32:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 07F09356F3 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=keiths@redhat.com Received: from valrhona.Home (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id CC6D54C4B8; Mon, 16 Oct 2017 21:32:18 +0000 (UTC) From: Keith Seitz To: simon.marchi@ericsson.com Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Record and output access specifiers for nested typedefs Date: Mon, 16 Oct 2017 14:32:18 -0700 Message-Id: <1508189538-6733-1-git-send-email-keiths@redhat.com> In-Reply-To: References: X-IsSubscribed: yes 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) : 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.