Re: [RFA] comdat types

[Tom Tromey <tromey@redhat.com>]

>>>>> "Doug" == Doug Evans <dje@google.com> 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.  There'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 = cu.header.offset;
+      this_cu->length = 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