From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10493 invoked by alias); 25 Jun 2009 20:28:19 -0000 Received: (qmail 10482 invoked by uid 22791); 25 Jun 2009 20:28:18 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Jun 2009 20:28:09 +0000 Received: from wpaz21.hot.corp.google.com (wpaz21.hot.corp.google.com [172.24.198.85]) by smtp-out.google.com with ESMTP id n5PKS5Mb001150 for ; Thu, 25 Jun 2009 21:28:05 +0100 Received: from yxe35 (yxe35.prod.google.com [10.190.2.35]) by wpaz21.hot.corp.google.com with ESMTP id n5PKRcm0014609 for ; Thu, 25 Jun 2009 13:28:03 -0700 Received: by yxe35 with SMTP id 35so1980286yxe.21 for ; Thu, 25 Jun 2009 13:28:02 -0700 (PDT) MIME-Version: 1.0 Received: by 10.90.80.4 with SMTP id d4mr2396900agb.67.1245961682764; Thu, 25 Jun 2009 13:28:02 -0700 (PDT) In-Reply-To: References: <20090620000402.C37E5843F5@localhost> Date: Thu, 25 Jun 2009 20:28:00 -0000 Message-ID: Subject: Re: [RFA] comdat types From: Doug Evans To: tromey@redhat.com Cc: gdb-patches@sourceware.org, ccoutant@google.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes 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/msg00688.txt.bz2 On Thu, Jun 25, 2009 at 12:46 PM, Tom Tromey wrote: > I'm curious about one thing on this page, and I thought I'd take the > opportunity to ask. =A0DW_TAG_type_unit may have a DW_AT_language child. > But, the language is not mentioned in the suggested method for > computing a type's signature. =A0This seems strange to me: either the > language matters (in which case, it seems like it ought to be in the > signature); or the language does not matter, in which case, why > mention it? I'll leave this for Cary. > One concern I have is whether there is any chance that the > specification will change between now and when DWARF-4 is published. Cary? > I do have a few nits. > > Doug> + =A0types_htab =3D htab_create_alloc_ex (41, > Doug> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hash_t= ype_signature, > Doug> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0eq_typ= e_signature, > Doug> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0NULL, > Doug> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&objfi= le->objfile_obstack, > Doug> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hashta= b_obstack_allocate, > Doug> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dummy_= obstack_deallocate); > > This is just a side note -- I've seen a few hash tables allocated on > obstacks. =A0Doesn't resizing the table waste memory? IIRC I use the noresize traversal routine. It's not ideal, no. > Too bad we don't have allocation pools instead of obstacks. I suppose > in this specific case we could use the objfile data machinery to > deallocate hash tables. If that's ok, I'll do that. > Doug> + =A0if (dwarf2_die_debug) > Doug> + =A0 =A0{ > Doug> + =A0 =A0 =A0fprintf_unfiltered (gdb_stdlog, "Signatured types:\n"); > Doug> + =A0 =A0} > > Over-bracing. =A0There's a fair amount of this in the patch. One of the style rules I'm less fond of (and see others are too from scans of gdb, is this a rule or a guideline?). [I'm reminded of Pirates of the Caribbean, "They're more like guidelines anyway." :-)] But ok. > Doug> + =A0if (this_cu->from_debug_types) > Doug> + =A0 =A0{ > Doug> + =A0 =A0 =A0/* ??? How come this is for .debug_types only? =A0*/ > Doug> + =A0 =A0 =A0this_cu->offset =3D cu.header.offset; > Doug> + =A0 =A0 =A0this_cu->length =3D cu.header.length + cu.header.initi= al_length_size; > > Daniel has asked before for "no new FIXMEs". =A0You can't escape this by > spelling it "???" :-) Well, that one was an oversight (these patches drag on and my eyes tend to glaze over ...). While as a general rule I don't disagree, it's kinda odd to see others add new functionality with open issues. > Seriously, I think this is a valid concern and I would like some > definitive resolution to the various "???" comments. > > Doug> + NotFound: > > Should be "not_found". =A0There's a couple of these. fwiw I like StudlyCaps for labels: How many labels should there be in one's code? --> As absolutely few as possible. Same with StudlyCaps. :-) There are existing uses of StudlyCaps for labels (albeit isolated to one file) so I went with that. But ok.