From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27340 invoked by alias); 29 Jan 2010 21:39:36 -0000 Received: (qmail 27331 invoked by uid 22791); 29 Jan 2010 21:39:35 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 29 Jan 2010 21:39:30 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0TLdTZX028797 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 29 Jan 2010 16:39:29 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o0TLdSRP027802; Fri, 29 Jan 2010 16:39:28 -0500 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id o0TLdRGe019776; Fri, 29 Jan 2010 16:39:27 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 0684A379975; Fri, 29 Jan 2010 14:39:26 -0700 (MST) From: Tom Tromey To: gdb-patches@sourceware.org Subject: Re: RFC: fix virtual base class bugs References: Reply-To: Tom Tromey Date: Fri, 29 Jan 2010 21:39:00 -0000 In-Reply-To: (Tom Tromey's message of "Fri, 29 Jan 2010 09:31:07 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-01/txt/msg00654.txt.bz2 >>>>> "Tom" == Tom Tromey writes: Tom> This patch fixes all the virtual base class printing bugs I could find. Tom> I plan to check this in soon unless there are comments or objections. [...] Tom> I looked at this code a little, too. AFAICT none of the DECLARED_TYPE_* Tom> constants are ever written to a C++ structure, and I think both Tom> DECLARED_TYPE_UNION and DECLARED_TYPE_TEMPLATE are useless as well. In Tom> this case I chose to change userdef.exp and then later look into Tom> changing the DWARF reader to respect DW_TAG_class_type. I wrote the follow-on patch today, and I'm pretty happy with it. I intend to commit both of these soon. This second patch removes TYPE_DECLARED_TYPE and a bunch of related constants. These are not currently used in gdb, except by accident: when a cplus_struct_type is allocated, the declared_type field is set to 0 (== TYPE_DECLARED_CLASS). In its place I put a single bit into main_type to indicate whether a structure was declared using "struct" or "class". I could have put this into cplus_struct_type .. but that seems awfully heavy for a single bit of information. Finally, I changed the DWARF reader to set this bit when appropriate. Built and regtested on x86-64 (compile farm). I tested it in isolation, so there is actually a "regression" in userdef.exp, but this is covered by the previous patch. I'm considering also removing TYPE_CODE_CLASS now. Also, I noticed that TYPE_CODE_TEMPLATE is never actually used -- and is probably not actually usable -- so that is another candidate for deletion. Tom 2010-01-29 Tom Tromey * m2-typeprint.c (m2_record_fields): Don't use TYPE_DECLARED_TYPE. * gdbtypes.h (TYPE_DECLARED_CLASS): New macro. (struct main_type) : New field. (struct cplus_struct_type) : Remove. : Move earlier. (DECLARED_TYPE_CLASS, DECLARED_TYPE_UNION, DECLARED_TYPE_STRUCT) (DECLARED_TYPE_TEMPLATE): Remove. (TYPE_DECLARED_TYPE): Remove. * gdbtypes.c (lookup_union): Don't use TYPE_DECLARED_TYPE. * dwarf2read.c (read_structure_type): Set TYPE_DECLARED_CLASS. * c-typeprint.c (c_type_print_base): Use TYPE_DECLARED_CLASS, not TYPE_DECLARED_TYPE. 2010-01-29 Tom Tromey * gdb.dwarf2/member-ptr-forwardref.exp: Update expected result for type-printing change. diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c index d1af481..27746d9 100644 --- a/gdb/c-typeprint.c +++ b/gdb/c-typeprint.c @@ -702,35 +702,10 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show, case TYPE_CODE_STRUCT: c_type_print_modifier (type, stream, 0, 1); - /* Note TYPE_CODE_STRUCT and TYPE_CODE_CLASS have the same value, - * so we use another means for distinguishing them. - */ - if (HAVE_CPLUS_STRUCT (type)) - { - switch (TYPE_DECLARED_TYPE (type)) - { - case DECLARED_TYPE_CLASS: - fprintf_filtered (stream, "class "); - break; - case DECLARED_TYPE_UNION: - fprintf_filtered (stream, "union "); - break; - case DECLARED_TYPE_STRUCT: - fprintf_filtered (stream, "struct "); - break; - default: - /* If there is a CPLUS_STRUCT, assume class if not - * otherwise specified in the declared_type field. - */ - fprintf_filtered (stream, "class "); - break; - } /* switch */ - } + if (TYPE_DECLARED_CLASS (type)) + fprintf_filtered (stream, "class "); else - { - /* If not CPLUS_STRUCT, then assume it's a C struct */ - fprintf_filtered (stream, "struct "); - } + fprintf_filtered (stream, "struct "); goto struct_union; case TYPE_CODE_UNION: @@ -786,8 +761,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show, masquerading as a class, if all members are public, there's no need for a "public:" label. */ - if ((TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_CLASS) - || (TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_TEMPLATE)) + if (TYPE_DECLARED_CLASS (type)) { QUIT; len = TYPE_NFIELDS (type); @@ -815,8 +789,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show, } } } - else if ((TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_STRUCT) - || (TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_UNION)) + else { QUIT; len = TYPE_NFIELDS (type); @@ -863,10 +836,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show, || TYPE_FIELD_ARTIFICIAL (type, i)) continue; - /* If this is a C++ class we can print the various C++ section - labels. */ - - if (HAVE_CPLUS_STRUCT (type) && need_access_label) + if (need_access_label) { if (TYPE_FIELD_PROTECTED (type, i)) { diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 2f671ca..86bfecb 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -5007,11 +5007,12 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) } else { - /* FIXME: TYPE_CODE_CLASS is currently defined to TYPE_CODE_STRUCT - in gdbtypes.h. */ TYPE_CODE (type) = TYPE_CODE_CLASS; } + if (cu->language == language_cplus && die->tag == DW_TAG_class_type) + TYPE_DECLARED_CLASS (type) = 1; + attr = dwarf2_attr (die, DW_AT_byte_size, cu); if (attr) { diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 2ff8647..46846c4 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -1132,13 +1132,6 @@ lookup_union (char *name, struct block *block) if (TYPE_CODE (t) == TYPE_CODE_UNION) return t; - /* C++ unions may come out with TYPE_CODE_CLASS, but we look at - * a further "declared_type" field to discover it is really a union. - */ - if (HAVE_CPLUS_STRUCT (t)) - if (TYPE_DECLARED_TYPE (t) == DECLARED_TYPE_UNION) - return t; - /* If we get here, it's not a union. */ error (_("This context has class, struct or enum %s, not a union."), name); diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 72968a5..643fa03 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -279,6 +279,12 @@ enum type_instance_flag_value #define TYPE_OWNER(t) TYPE_MAIN_TYPE(t)->owner #define TYPE_OBJFILE(t) (TYPE_OBJFILE_OWNED(t)? TYPE_OWNER(t).objfile : NULL) +/* True if this type was declared using the "class" keyword. This is + only valid for C++ structure types, and only used for displaying + the type. If false, the structure was declared as a "struct". */ + +#define TYPE_DECLARED_CLASS(t) (TYPE_MAIN_TYPE (t)->flag_declared_class) + /* Constant type. If this is set, the corresponding type has a * const modifier. */ @@ -386,6 +392,9 @@ struct main_type unsigned int flag_nottext : 1; unsigned int flag_fixed_instance : 1; unsigned int flag_objfile_owned : 1; + /* True if this type was declared with "class" rather than + "struct". */ + unsigned int flag_declared_class : 1; /* A discriminant telling us which field of the type_specific union is being used for this type, if any. */ @@ -676,19 +685,10 @@ struct cplus_struct_type short nfn_fields_total; - /* The "declared_type" field contains a code saying how the - user really declared this type, e.g., "class s", "union s", - "struct s". - The 3 above things come out from the C++ compiler looking like classes, - but we keep track of the real declaration so we can give - the correct information on "ptype". (Note: TEMPLATE may not - belong in this list...) */ + /* Number of template arguments, placed here for better struct + packing. */ -#define DECLARED_TYPE_CLASS 0 -#define DECLARED_TYPE_UNION 1 -#define DECLARED_TYPE_STRUCT 2 -#define DECLARED_TYPE_TEMPLATE 3 - short declared_type; /* One of the above codes */ + short ntemplate_args; /* 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. @@ -813,7 +813,6 @@ struct cplus_struct_type * is a name. "type" will typically just point to a "struct type" with * the placeholder TYPE_CODE_TEMPLATE_ARG type. */ - short ntemplate_args; struct template_arg { char *name; @@ -943,7 +942,6 @@ extern void allocate_gnat_aux_type (struct type *); #define TYPE_NFN_FIELDS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->nfn_fields #define TYPE_NFN_FIELDS_TOTAL(thistype) TYPE_CPLUS_SPECIFIC(thistype)->nfn_fields_total #define TYPE_NTEMPLATE_ARGS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->ntemplate_args -#define TYPE_DECLARED_TYPE(thistype) TYPE_CPLUS_SPECIFIC(thistype)->declared_type #define TYPE_SPECIFIC_FIELD(thistype) \ TYPE_MAIN_TYPE(thistype)->type_specific_field #define TYPE_TYPE_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific diff --git a/gdb/m2-typeprint.c b/gdb/m2-typeprint.c index afa9978..46a35bb 100644 --- a/gdb/m2-typeprint.c +++ b/gdb/m2-typeprint.c @@ -547,9 +547,9 @@ m2_record_fields (struct type *type, struct ui_file *stream, int show, wrap_here (" "); if (show < 0) { - if (TYPE_CODE (type) == DECLARED_TYPE_STRUCT) + if (TYPE_CODE (type) == TYPE_CODE_STRUCT) fprintf_filtered (stream, "RECORD ... END "); - else if (TYPE_DECLARED_TYPE (type) == DECLARED_TYPE_UNION) + else if (TYPE_CODE (type) == TYPE_CODE_UNION) fprintf_filtered (stream, "CASE ... END "); } else if (show > 0) diff --git a/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp b/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp index 0a54dfe..bb947bc 100644 --- a/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp +++ b/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp @@ -45,4 +45,4 @@ gdb_test "show cp-abi" {The currently selected C\+\+ ABI is "gnu-v3".*} gdb_load ${binfile} -gdb_test "ptype c" "type = class C {\[\r\n \t\]*int \\(C::\\*fp\\)\\(C \\*\\);\[\r\n \t\]*}" +gdb_test "ptype c" "type = struct C {\[\r\n \t\]*private:\[\r\n \t\]*int \\(C::\\*fp\\)\\(C \\*\\);\[\r\n \t\]*}"