Re: Forgot to note

[Eli Zaretskii <eliz@delorie.com>]

> From: Daniel Berlin <dberlin@redhat.com>
> Date: 10 Oct 2000 20:38:50 -0400
> 
> When the C++ abi moves to the new-abi, I can't fix stabs support
> without either adding a whole bunch of cruft, or making it not support
> the old ABI.
> 
> This is because things like gdb_mangle_name have to be changed to
> handle the new mangling scheme.
> So I have to either detect whether we have old-abi or new-abi
> somewhere, and then add a whole bunch of "if gnu-new-abi" type
> statements, or stop supporting the old abi for stabs/C++.
[snip]
> What should we do?
> Not support new-abi/stabs?
> Not support old-abi/stabs?
> Support both? (this is a not insignificant amount of work).

Sorry, I'm not privy enough to the C++ ABI issues, so I'm not sure I
understand the implications of the ABI change for the platforms I'm
interested in.  Could you say a few more words about this?

For instance, it is not clear to me whether the ABI change will affect
all platforms that use G++, or is this dependent on the executable
format which GCC/Binutils are congfigured to support.  Specifically,
what about non-dwarf platforms such as COFF and stabs?

Assuming that this affects all platforms, discontinuing the support
for the old ABI looks at first sight as a rather drastic and
user-unfriendly move.
From eliz@delorie.com Tue Oct 10 23:56:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: dberlin@redhat.com
Cc: meissner@cygnus.com, gdb-patches@sources.redhat.com
Subject: Re: Forgot to note
Date: Tue, 10 Oct 2000 23:56:00 -0000
Message-id: <200010110656.CAA06320@indy.delorie.com>
References: <m33di46wkl.fsf@dan2.cygnus.com> <20001010211904.49719@cse.cygnus.com> <m3bswsds9s.fsf@dan2.cygnus.com>
X-SW-Source: 2000-10/msg00062.html
Content-length: 659

> From: Daniel Berlin <dberlin@redhat.com>
> Date: 10 Oct 2000 22:29:51 -0400
> 
> It's arguably a *lot* simpler to change those platforms to dwarf2,
> then to support new-abi/stabs *and* old-abi/stabs at the same time.
> 
> One is a case of changing a define in a gcc config file (correct me if
> i'm incorrect here, I'm under the impression it should only really
> require changing the DEFAULT_DEBUGGING_FORMAT, and possibly redefining
> a few macros.)

Doesn't it require to configure Binutils differently as well?

I don't think the DJGPP port ever used dwarf2 code in Binutils, so
Someone (tm) will have to go through it carefully and see that it
works.
From eliz@delorie.com Tue Oct 10 23:56:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: kevinb@cygnus.com
Cc: gdb-patches@sources.redhat.com, gdb@sources.redhat.com
Subject: Re: Forgot to note
Date: Tue, 10 Oct 2000 23:56:00 -0000
Message-id: <200010110655.CAA06317@indy.delorie.com>
References: <m33di46wkl.fsf@dan2.cygnus.com> <1001011011645.ZM2970@ocotillo.lan>
X-SW-Source: 2000-10/msg00061.html
Content-length: 366

> Date: Tue, 10 Oct 2000 18:16:45 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
> 
> It'd be useful if someone could enumerate the compilers which still
> produce stabs as the debug info.

The DJGPP port of GCC still uses stabs.  (It also supports COFF, but
that's almost useless for C++ debugging.)  I'm not aware of any work
being done to port dwarf2 to DJGPP.
From kevinb@cygnus.com Wed Oct 11 00:20:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Daniel Berlin <dberlin@redhat.com>
Cc: Elena Zannoni <ezannoni@cygnus.com>, cgf@cygnus.com, gdb-patches@sources.redhat.com
Subject: Re: [PATCH]: C++ mangling patch that is about to be committed
Date: Wed, 11 Oct 2000 00:20:00 -0000
Message-id: <1001011072037.ZM3267@ocotillo.lan>
References: <m38zrw4w2p.fsf@dan2.cygnus.com> <14819.13803.331153.108644@kwikemart.cygnus.com> <m3k8bgadxp.fsf@dan2.cygnus.com> <39E35F1D.6070@redhat.com> <m3zokc8p3c.fsf@dan2.cygnus.com> <14819.41553.706248.391933@kwikemart.cygnus.com> <m3bsws6wsv.fsf@dan2.cygnus.com> <dberlin@redhat.com>
X-SW-Source: 2000-10/msg00063.html
Content-length: 3875

On Oct 10,  8:33pm, Daniel Berlin wrote:

> gdb_mangle_name is a perfect example.
> It has nothing to do with symbols.
> All it does is take a type structure, a method id, and a signature,
> and build a new mangled name for it.
> It involves magical knowledge of how g++ mangles names.
> It doesn't even have a struct symbol in it, or anywhere near it.
> It isn't called except from a few C++ specific routines (cp-valprint
> or typeprint calls it, and one other place that escapes the mind).
> Of course, it's in symtab.c.
> 
> Are you starting to see my point?
> The people i have to get approval for are really not qualified to
> examine the patches.
> No offense, but taking you as an example, it took you a while to
> figure out what my changes were doing, and they aren't obsfuscated
> code.

Hmm... "really not qualified"...?

Daniel, I don't think that this choice of words is the way to win
friends and influence people.

If I read your implications correctly, then I am in complete
disagreement with you regarding Elena's qualifications.  Specifically,
when I've asked her for help in this area (gdb symbol tables), she's
always provided good and useful answers.

Prior to that though (in your message), I was starting to see your
point.  It's like this...  there are times when someone relatively
unfamiliar with a piece of code can go into it to fix a bug, or work
on an optimization, or add a new feature, or whatever.  At the end of
that time, whether it be a matter of hours or a number of days, I
think it is frequently the case that that person who was heretofore
unfamiliar with the code in question will have learned enough to rival
the maintainer's knowledge of the code, particularly if said
maintainer is also working on a bunch of other stuff at the same time.

But this is a temporary situation.  Or it should be at any rate
because when you provide a patch for someone to review, I've
always thought it best to:

 1) Describe the problem that your patch is trying to solve.

 2) Show (via a detailed analysis if necessary) why the current
    code is broken.

 3) Likewise, describe why your patch fixes the problem.

 4) Provide an analysis of the side effects (if any) of your
    patch, or other areas where it may be deficient.

In other words, you should attempt to pass along the knowledge that
you've gained while working on a certain piece of code.

You should also provide a summary of any domain specific knowledge
(e.g, g++ name mangling behavior) which you have that bears upon the
problem.  It's okay to provide a reference if it's incredibly
detailed.  (For example, see
http://sources.redhat.com/ml/gdb-patches/2000-08/msg00075.html in
which I provide a reference to a DWARF2 draft document.  Note,
however, that I summarize the salient points in the message proper.)

I know that this creates (substantially) more work for the patch
submitter.  But I also believe it creates higher quality patches.  I
do not always take the time to do everything that I described above,
but when I do, I'm forced to look more critically at my changes.  I
will frequently find problems with them in the process of writing my
analysis.  Even if I find no problems, the analysis will hopefully
give the maintainer the necessary background to see why the proposed
changes are necessary.  My motives are not completely altruistic; I've
discovered that providing a cogent analysis often means that my
patches are approved quicker which cuts down on the merge and retest
time.  (We all *do* test our changes again prior to committing them,
don't we?)  Also, I tend to forget these details relatively quickly,
so writing them down is a way of preserving this knowledge for myself
as well.

In my opinion, it is also useful for maintainers to go through this
exercise for patches in their own area since these patches are still
subject to peer review.

Kevin
From dberlin@redhat.com Wed Oct 11 04:50:00 2000
From: Daniel Berlin <dberlin@redhat.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: Elena Zannoni <ezannoni@cygnus.com>, cgf@cygnus.com, gdb-patches@sources.redhat.com
Subject: Re: [PATCH]: C++ mangling patch that is about to be committed
Date: Wed, 11 Oct 2000 04:50:00 -0000
Message-id: <m366mzzjfa.fsf@dan2.cygnus.com>
References: <m38zrw4w2p.fsf@dan2.cygnus.com> <14819.13803.331153.108644@kwikemart.cygnus.com> <m3k8bgadxp.fsf@dan2.cygnus.com> <39E35F1D.6070@redhat.com> <m3zokc8p3c.fsf@dan2.cygnus.com> <14819.41553.706248.391933@kwikemart.cygnus.com> <m3bsws6wsv.fsf@dan2.cygnus.com> <1001011072037.ZM3267@ocotillo.lan>
X-SW-Source: 2000-10/msg00064.html
Content-length: 2555

Kevin Buettner <kevinb@cygnus.com> writes:

> On Oct 10,  8:33pm, Daniel Berlin wrote:
> 
> > gdb_mangle_name is a perfect example.
> > It has nothing to do with symbols.
> > All it does is take a type structure, a method id, and a signature,
> > and build a new mangled name for it.
> > It involves magical knowledge of how g++ mangles names.
> > It doesn't even have a struct symbol in it, or anywhere near it.
> > It isn't called except from a few C++ specific routines (cp-valprint
> > or typeprint calls it, and one other place that escapes the mind).
> > Of course, it's in symtab.c.
> > 
> > Are you starting to see my point?
> > The people i have to get approval for are really not qualified to
> > examine the patches.
> > No offense, but taking you as an example, it took you a while to
> > figure out what my changes were doing, and they aren't obsfuscated
> > code.
> 
> Hmm... "really not qualified"...?
> 
> Daniel, I don't think that this choice of words is the way to win
> friends and influence people.
> 
> If I read your implications correctly, then I am in complete
> disagreement with you regarding Elena's qualifications.  Specifically,
> when I've asked her for help in this area (gdb symbol tables), she's
> always provided good and useful answers.

No, i'm not implying what you think i'm implying.
I'm trying to say the area of expertise that gdb_mangle_name involves
isn't really "symbol tables", but "C++ mangling". It just happens to
be in symtab.c for no good reason.
AFAICT, it was originally in gdbtypes.c, where it doesn't belong
either.

It has nothing to do with Elena's qualifications, it has to do with
the area of maintainership this particular piece of code falls under.
It falls under the wrong area of maintainership, because of the file
it's in. Elena shouldn't have to waste her valueable time reviewing
patches like that, because trying to understand how to all of the C++
stuff fits together, and what a change to gdb_mangle_name will affect,
takes a *long time* for anyone to understand.  
 I was actually specifically referring to the fact that she mentioned it took
quite a bit of staring to figure out what the changes were doing. It
took me 3 weeks of staring to figure out that it was broken in the
first place. It's complex code, and it has nothing to do with symbol
tables. So why should Elena have to waste her time doing the staring?

Like I said, my point has nothing to do with Elena's qualifications.
She's quite qualified to review any piece of code.
That doesn't mean she should have to.
--Dan