From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24238 invoked by alias); 25 Jun 2009 20:49:22 -0000 Received: (qmail 24226 invoked by uid 22791); 25 Jun 2009 20:49:18 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Jun 2009 20:49:13 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n5PKn8AL028898; Thu, 25 Jun 2009 16:49:08 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n5PKn7AX026656; Thu, 25 Jun 2009 16:49:07 -0400 Received: from opsy.redhat.com (vpn-13-18.rdu.redhat.com [10.11.13.18]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n5PKn6KT015132; Thu, 25 Jun 2009 16:49:06 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 0E4363785DB; Thu, 25 Jun 2009 14:48:58 -0600 (MDT) To: Doug Evans Cc: gdb-patches@sourceware.org, ccoutant@google.com Subject: Re: [RFA] comdat types References: <20090620000402.C37E5843F5@localhost> From: Tom Tromey Reply-To: tromey@redhat.com Date: Thu, 25 Jun 2009 20:49:00 -0000 In-Reply-To: (Doug Evans's message of "Thu\, 25 Jun 2009 13\:28\:02 -0700") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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: 2009-06/txt/msg00689.txt.bz2 >>>>> "Doug" =3D=3D Doug Evans writes: Tom> Too bad we don't have allocation pools instead of obstacks. I suppose Tom> in this specific case we could use the objfile data machinery to Tom> deallocate hash tables. Doug> If that's ok, I'll do that. Sorry, I wasn't clear. This particular thing was just a drive-by complaint on my part. I don't think you need to make any change to your patch. Tom> Over-bracing. =C2=A0There's a fair amount of this in the patch. Doug> One of the style rules I'm less fond of (and see others are too from Doug> scans of gdb, is this a rule or a guideline?). Doug> [I'm reminded of Pirates of the Caribbean, "They're more like Doug> guidelines anyway." :-)] Doug> But ok. Yeah, I tend to treat this one as a rule. Doug> Well, that one was an oversight (these patches drag on and my eyes Doug> tend to glaze over ...). Doug> While as a general rule I don't disagree, it's kinda odd to see others Doug> add new functionality with open issues. I'm sorry. I try to be reasonably consistent when reviewing, but I'm probably not. It is fair to call me on that when it happens. Perhaps it was wrong of me to drag in the general rule (which, as we were discussing on irc, has various corner cases and ambiguities) when something specific would serve better. The comments in question: + if (this_cu->from_debug_types) + { + /* ??? How come this is for .debug_types only? */ + this_cu->offset =3D cu.header.offset; + this_cu->length =3D cu.header.length + cu.header.initial_length_size; + } /* Possibly set the default values of LOWPC and HIGHPC from - `DW_AT_ranges'. */ + `DW_AT_ranges'. + ??? Is this valid for .debug_types? */ if (cu.has_ranges_offset) { + if (this_cu->from_debug_types) + { + /* ??? It's not clear we want to do anything with stmt lists here. + Waiting to see what gcc ultimately does. */ + } It seems to me that the first one is (IIUC) a question about the implementation of dwarf2read.c itself. So, can it answered and rephrased as a note? The second seems like a question about the spec. The third one, I'm less sure. Perhaps just removing the "???" part is appropriate. Doug> fwiw I like StudlyCaps for labels: How many labels should there be in Doug> one's code? Doug> --> As absolutely few as possible. Doug> Same with StudlyCaps. :-) :-) StudlyCaps just look weird to me in GNU code. And the many gotos in gdb largely follow the GNUish naming convention, though I see some counterexamples in Ada: ada-typeprint.c: goto Huh; ada-exp.y: goto BadEncoding; ada-exp.y: goto TryAfterRenaming; Tom