From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26155 invoked by alias); 15 Apr 2004 07:31:29 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 26143 invoked from network); 15 Apr 2004 07:31:28 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 15 Apr 2004 07:31:28 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i3F7VSu6005158 for ; Thu, 15 Apr 2004 03:31:28 -0400 Received: from zenia.home.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i3F7VQj05161; Thu, 15 Apr 2004 03:31:26 -0400 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com, ezannoni@redhat.com Subject: Re: [RFA/dwarf] Partial DIE support for specifications References: <20040402204733.GA16725@nevyn.them.org> From: Jim Blandy Date: Thu, 15 Apr 2004 07:31:00 -0000 In-Reply-To: <20040402204733.GA16725@nevyn.them.org> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2004-04/txt/msg00298.txt.bz2 Overall, I think it makes the partial symbol processing much more legible. It's much easier to follow the flow of things. It's a shame about the speed hit, but I think we just have to take that for now. So once the points below are addressed or answered, I think it can go in. - In fixup_partial_die, in the last 'if', you check for DW_TAG_structure_type twice. And shouldn't that same 'if' make sure part_die->name is NULL before calling guess_structure_name? And some trivial comments: - Could the 'make_cleanup (free_stack_comp_unit, &cu)' call be moved to directly after the call to 'obstack_init (&cu.comp_unit_obstack)'? - The comment atop 'scan_partial_symbols' says "Read in all interesting dies". But it doesn't do that any more. - Why not write the body of scan_partial_symbols like this? It seems clearer to me, but if you don't agree, then don't bother: /* Namespaces have interesting children, anonymous or not. */ if (pdi->tag == DW_TAG_namespace) add_partial_namespace (pdi, lowpc, highpc, cu); /* Enums have interesting children, anonymous or not. */ else if (pdi->tag == DW_TAG_enumeration_type) { if (!pdi->is_declaration) add_partial_enumeration (pdi, cu); } /* Everything else is only interesting if it has a name. */ else if (pdi->name != NULL) { switch (pdi->tag) { case DW_TAG_subprogram: if (pdi->has_pc_info) { if (pdi->lowpc < *lowpc) { *lowpc = pdi->lowpc; } if (pdi->highpc > *highpc) { *highpc = pdi->highpc; } if (!pdi->is_declaration) { add_partial_symbol (pdi, cu); } } break; case DW_TAG_variable: case DW_TAG_typedef: case DW_TAG_union_type: if (!pdi->is_declaration) { add_partial_symbol (pdi, cu); } break; case DW_TAG_class_type: case DW_TAG_structure_type: if (!pdi->is_declaration) { add_partial_symbol (pdi, cu); } break; case DW_TAG_base_type: case DW_TAG_subrange_type: /* File scope base type definitions are added to the partial symbol table. */ add_partial_symbol (pdi, cu); break; default: break; } }