obvious set_cu_language patch

obvious set_cu_language patch

[Per Bothner]

I consider this one fairly obvious.

2001-06-07  Per Bothner  <per@bothner.com>

	* dwarf2read.c (set_cu_language):  Handle DW_LANG_Java.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.24
diff -u -r1.24 dwarf2read.c
--- dwarf2read.c	2001/04/30 10:30:27	1.24
+++ dwarf2read.c	2001/06/08 06:18:51
@@ -3721,6 +3721,9 @@
     case DW_LANG_Mips_Assembler:
       cu_language = language_asm;
       break;
+    case DW_LANG_Java:
+      cu_language = language_java;
+      break;
     case DW_LANG_Ada83:
     case DW_LANG_Cobol74:
     case DW_LANG_Cobol85:

-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: obvious set_cu_language patch

[Daniel Berlin]

Per Bothner <per@bothner.com> writes:

> I consider this one fairly obvious.
> 

Yup. I swear the java guys submitted this before, and we accepted and
committed it.
I know it was submitted *ages* ago.


Ahh, here's the problem with it:
dwarf2.h in gcc defines DW_LANG_Java as 0x000b.
binutils/gdb's include/elf/dwarf2.h defines DW_LANG_Java as 0x9af4

Which is correct?
Please, in addition to committing this patch, which *is* an obvious
fix, fix whichever one of the two above is wrong.

Clearly, one is.

Please tell me it's the include/elf/dwarf2.h, and not gcc, because if
it's gcc, we have to handle *both* in gdb.

> 2001-06-07  Per Bothner  <per@bothner.com>
> 
> 	* dwarf2read.c (set_cu_language):  Handle DW_LANG_Java.
> 
> Index: dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.24
> diff -u -r1.24 dwarf2read.c
> --- dwarf2read.c	2001/04/30 10:30:27	1.24
>+++ dwarf2read.c	2001/06/08 06:18:51
> @@ -3721,6 +3721,9 @@
>      case DW_LANG_Mips_Assembler:
>        cu_language = language_asm;
>        break;
>+    case DW_LANG_Java:
>+      cu_language = language_java;
>+      break;
>      case DW_LANG_Ada83:
>      case DW_LANG_Cobol74:
>      case DW_LANG_Cobol85:
> 
> -- 
> 	--Per Bothner
> per@bothner.com   http://www.bothner.com/per/

-- 
"I was born by Caesarean section...  But not so you'd notice.
It's just that when I leave a house, I go out through the
window.
"-Steven Wright

Re: obvious set_cu_language patch

[Per Bothner]

Daniel Berlin <dan@cgsoftware.com> writes:

> dwarf2.h in gcc defines DW_LANG_Java as 0x000b.
> binutils/gdb's include/elf/dwarf2.h defines DW_LANG_Java as 0x9af4

> Which is correct?

I was going to say that I didn't know.  But then I found:
        http://reality.sgi.com/dehnert_engr/dwarf/d2-summary.html
Draft 6, page 139, lists:
        DW_LANG_Java 0x000b

I'll fix include/elf/dwarf2.h.  I'm sure Stu Groassman just picked a
random number unlikely to conflict with some other assignment.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: obvious set_cu_language patch

[Andrew Cagney]

Per, can you add your self to the MAINTAINERS file as write after approval.

As some know, Per is a long time hacker of GDB so, in a way I should be 
saying ``welcome back''.

	Andrew

Re: obvious set_cu_language patch

[Daniel Berlin]

Per Bothner <per@bothner.com> writes:

> Andrew Cagney <ac131313@cygnus.com> writes:
> 
>> Per, can you add your self to the MAINTAINERS file as write after approval.
> 
> Actually, I was thinking of this instead.  Is this ok?

I'd pay you money to do it.
:)

While i'm redoing the type system, I was just about to ask a java
person the following:
in jv-lang.c, in java_array_type, it says "FIXME: This is bogus! Java
arrays are not gdb arrays!".

Could you please point me to where i can find out what java arrays
*are*, so i can make a type structure for them (the new typesystem is
a hierarchy of structs, rather than a single type struct.  This means
whatever java arrays *are*, can easily be represented and handled,
with no fuss or muss).

> 
> Index: MAINTAINERS
> ===================================================================
> RCS file: /cvs/src/src/gdb/MAINTAINERS,v
> retrieving revision 1.93
> diff -u -r1.93 MAINTAINERS
> --- MAINTAINERS	2001/05/10 21:10:09	1.93
>+++ MAINTAINERS	2001/06/08 20:21:15
> @@ -245,6 +245,7 @@
>  language support	David Taylor		taylor@cygnus.com
>    C++ support		Daniel Berlin		dan@cgsoftware.com
>    Java support		Anthony Green 		green@cygnus.com
>+			Per Bothner		per@bothner.com
>    Objective-C/C++	Klee Dienes		kdienes@apple.com
>  			Jim Ingham		jimgham@apple.com
>    Pascal support	Pierre Muller		muller@sourceware.cygnus.com
> 
> -- 
> 	--Per Bothner
> per@bothner.com   http://www.bothner.com/per/

-- 
"I'd like to sing you a song now about my old girlfriend.  It's
called, "They'll Find Her When the Leaves Blow Away 'Cause I'm
Not Raking 'Til Spring."
"-Steven Wright

Re: obvious set_cu_language patch

[Stan Shebs]

Per Bothner wrote:
> 
> Daniel Berlin <dan@cgsoftware.com> writes:
> 
> > dwarf2.h in gcc defines DW_LANG_Java as 0x000b.
> > binutils/gdb's include/elf/dwarf2.h defines DW_LANG_Java as 0x9af4
> 
> > Which is correct?
> 
> I was going to say that I didn't know.  But then I found:
>         http://reality.sgi.com/dehnert_engr/dwarf/d2-summary.html
> Draft 6, page 139, lists:
>         DW_LANG_Java 0x000b
> 
> I'll fix include/elf/dwarf2.h.  I'm sure Stu Groassman just picked a
> random number unlikely to conflict with some other assignment.

When Stu did this, there was no mention of Java in the still-drafty
Dwarf 2 spec.  What's in GCC includes Dwarf 2.1 draft, so it's more
recent, but beware of many inconsistencies.  (diff of gcc/dwarf2.h
and include/elf/dwarf2.h is positively scary, since they're supposed
to be identical)

Will GDB need to be able to recognize the old language code, perhaps
in an old piece of gcj output?  You could leave the old code in
dwarf2.h, just give it a different symbolic name.

Stan

Re: obvious set_cu_language patch

[Per Bothner]

Tom Tromey <tromey@redhat.com> writes:

> Most instantiations of JArray are actually created at runtime.  Only
> primitive arrays are created statically.

To clarify:  Tom is here talking about the corresponding Class instances
that represent the "run-time type information" of an array type.

Some possibly-helpful terminology:

* A type is a compile-time set of values and operations.
* A class specifies fields, methods and inheritance.
* A class can be viewed as a type, in which case the set of values is
the set of references (pointers) to instances of the class or its
sub-classes.
* All classes inherit from the root java.lang.Object (or Object for short).
* An instance of a class is the same thing as an object;
all Java objects are heap-allocated.
* All Java arrays are objects, hence their classes inherit from Object.
* For each class known to the run-time system (which includes all classes
for which there exist instances), there is a unique java.lang.Class object.
This describes "run-time type information" or "reflective" data, such
as the class name and list of the fields.
* The gcj compiler pre-allocates in static memory Class instances for
normal classes that it compiles.  The run-time system dynamically creates
Class instances for classes that are loaded dynamically (i.e. interpreted).
The run-time system also creates Class instances as needed for array
classes, because there is no way to associate them with a specific
compilation unit.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: obvious set_cu_language patch

[Daniel Berlin]

On Fri, 8 Jun 2001, Stan Shebs wrote:

> Per Bothner wrote:
> >
> > Daniel Berlin <dan@cgsoftware.com> writes:
> >
> > > dwarf2.h in gcc defines DW_LANG_Java as 0x000b.
> > > binutils/gdb's include/elf/dwarf2.h defines DW_LANG_Java as 0x9af4
> >
> > > Which is correct?
> >
> > I was going to say that I didn't know.  But then I found:
> >         http://reality.sgi.com/dehnert_engr/dwarf/d2-summary.html
> > Draft 6, page 139, lists:
> >         DW_LANG_Java 0x000b
> >
> > I'll fix include/elf/dwarf2.h.  I'm sure Stu Groassman just picked a
> > random number unlikely to conflict with some other assignment.
>
> When Stu did this, there was no mention of Java in the still-drafty
> Dwarf 2 spec.  What's in GCC includes Dwarf 2.1 draft, so it's more
> recent, but beware of many inconsistencies.  (diff of gcc/dwarf2.h
> and include/elf/dwarf2.h is positively scary, since they're supposed
> to be identical)
>
> Will GDB need to be able to recognize the old language code, perhaps
> in an old piece of gcj output?

Depends on how you look at it.
We couldn't before, obviously, and nobody complained, so ...
>  You could  leave the old code in
> dwarf2.h, just give it a different symbolic name.
>

> Stan
>

Re: Rewriting the type system

[Daniel Berlin]

Jim Blandy <jimb@zwingli.cygnus.com> writes:

> Daniel Berlin <dan@cgsoftware.com> writes:
>> While i'm redoing the type system, I was just about to ask a java
>> person the following:
>> in jv-lang.c, in java_array_type, it says "FIXME: This is bogus! Java
>> arrays are not gdb arrays!".
>> 
>> Could you please point me to where i can find out what java arrays
>> *are*, so i can make a type structure for them (the new typesystem is
>> a hierarchy of structs, rather than a single type struct.  This means
>> whatever java arrays *are*, can easily be represented and handled,
>> with no fuss or muss).
> 
> If you plan to submit a large, pervasive change for GDB, you will want
> to first explaing your change in detail on gdb@sources.redhat.com.
Errr, I did, a *long* time ago.

> The people who will approve or reject your patch read that list, and
> will almost certainly have some comments on the design.
They did.

>  By making
> sure the fundamental design of your change meets their requirements
> *before* you invest the time to turn your ideas into a patch, you may
> avoid having to rewrite the change from scratch.

Not a problem.  By the time the patch gets reviewed, it'll be rewritten anyway.
Even the very simple patch to add the misc obstack to the objfiles,
and stop putting things in the psymbol obstack that don't belong, from
May 29th, hasn't been reviewed yet.
Hell, the simple bcache change i submitted last year (updating the
starting constant, fix the indenting) still hasn't
been reviewed.

Jim, GDB development is moving a lot slower than it should.  If
someone told me, after just rewriting the typesystem, that i needed to
redo it from scratch, i'd probably just start making my own GDB
releases instead (in effect, forking GDB).  Not out of spite or
anything, just so i could get stuff done without having to keep track
of tons of patches that go months without review.

I'm now up to 15 people who ask me for my current tree weekly/monthly
(which contains  all my outstanding patches that haven't been
reviewed) instead of downloading cvs.

This, IMHO, is amazingly silly. The fact that the patches wait long
enough for others to notice, and get fed up enough with the current
state to ask me for the tree tells me something is very wrong here.

I usually wait a week before tarring up the tree, in hopes that maybe,
just maybe, someone will review whatever patches they are hoping to
get out of it.

Hasn't happened yet.

--Dan
-- 
"One time I went to a museum where all the work in the museum had
been done by children.  They had all the paintings up on
refrigerators.
"-Steven Wright

Re: Rewriting the type system

[Stan Shebs]

Daniel Berlin wrote:
> 
> Jim Blandy <jimb@zwingli.cygnus.com> writes:
> 
> >  By making
> > sure the fundamental design of your change meets their requirements
> > *before* you invest the time to turn your ideas into a patch, you may
> > avoid having to rewrite the change from scratch.
> 
> Not a problem.  By the time the patch gets reviewed, it'll be rewritten anyway.
> Even the very simple patch to add the misc obstack to the objfiles,
> and stop putting things in the psymbol obstack that don't belong, from
> May 29th, hasn't been reviewed yet.

That's two weeks, which is a little long but not unreasonable.

> Hell, the simple bcache change i submitted last year (updating the
> starting constant, fix the indenting) still hasn't
> been reviewed.

That is an unreasonable delay.  As you know from GCC land, the usual
thing there is to post an "Unreviewed patches" messages with xrefs to
the original patch submissions.

> Jim, GDB development is moving a lot slower than it should.  If
> someone told me, after just rewriting the typesystem, that i needed to
> redo it from scratch, i'd probably just start making my own GDB
> releases instead (in effect, forking GDB).  Not out of spite or
> anything, just so i could get stuff done without having to keep track
> of tons of patches that go months without review.

Certainly that is your right according to the GPL, and perhaps even
your duty if GDB maintainers are not doing the job.  However, before
you go that far, I'd at least like to get the official flame that
you have patches X, Y, Z, etc, that have been sitting ignored.
I need the URLs so I can see the followups - perhaps they've been
discussed, but the review proper was poor or incomplete.  Also,
I remember seeing remarks, including some by yourself, to the effect
that some of your patches were incomplete in various ways.  As
someone who's complained about, say, problem code from HP, I'm
sure you'll appreciate the reluctance to take in code which the
author admits has problems.  (I may be mis-remembering - another
reason for pointers to specific messages.)

I admire the energy that you're bringing to GDB development, but
please have some patience.  The review process is our way of keeping
everything working, and because I've been involved with GDB for a
long time and bear a measure of responsibility for its current
state, many times in the past couple of years I've had to watch
somebody struggle with some piece of code, and know that it was
because years earlier it was easy or expedient just to allow it in,
instead of demanding something better.  Not a good feeling,
especially when it's clear that the struggling is consuming time
that could have been used on new development instead.

Stan

Re: Rewriting the type system

[Eli Zaretskii]

On 11 Jun 2001, Daniel Berlin wrote:

> Even the very simple patch to add the misc obstack to the objfiles,
> and stop putting things in the psymbol obstack that don't belong, from
> May 29th, hasn't been reviewed yet.
> Hell, the simple bcache change i submitted last year (updating the
> starting constant, fix the indenting) still hasn't
> been reviewed.
> 
> Jim, GDB development is moving a lot slower than it should.

My experience is very different.  Every change that I suggested until
now, for the past 2 years or so that I'm involved with GDB
maintenance, was usually reviewed within 1-2 weeks of my posting it
as an RFA.  A few times I needed to post a reminder (I usually do that
after more than a week's passed without any replies).  A couple of
times, I needed more than one reminder, but that's an exception rather
than the rule, in my experience.

In most cases, I had my patches reviewed and approved in 2-3 weeks,
sometimes a month.  In one exceptional case, it took 3 or 4 months,
but that was my first large submission, and I failed to ping the
relevant maintainer more than once.

So GDB development is not slow, in my opinion.  More importantly, I'm
always able to get my patches accepted by using the normal channels,
such as pinging people from time to time.

In other words, the development procedures work.

> If someone told me, after just rewriting the typesystem, that i
> needed to redo it from scratch, i'd probably just start making my
> own GDB releases instead (in effect, forking GDB).

As I already wrote elsewhere, if you care about GDB, please stop
talking about a fork, because even talking about it can do a
tremendous damage to the nice cooperative development atmosphere we
have in GDB.  I've been and am involved in quite a few other free
software packages, and I'm telling you that GDB is one of the nicest,
most cooperative environments I had ever experienced.  Please, let's
cherish that!

While there's always place for improvement, let's suggest such
improvements in a constructive way, and let's assume that everybody in
this fine forum has the same goal: making GDB better.  Offending
people by talking about a fork is not a good way, to say the least, of
asking them to be more responsive to your submissions.  It _is_,
however, an efficient way of making cooperation harder.

(I'm sorry about lecturing, but I cannot in good faith look the other
way when people talk about forking.)

Re: Rewriting the type system

[Daniel Berlin]

Eli Zaretskii <eliz@is.elta.co.il> writes:

> On 11 Jun 2001, Daniel Berlin wrote:
> 
>> Even the very simple patch to add the misc obstack to the objfiles,
>> and stop putting things in the psymbol obstack that don't belong, from
>> May 29th, hasn't been reviewed yet.
>> Hell, the simple bcache change i submitted last year (updating the
>> starting constant, fix the indenting) still hasn't
>> been reviewed.
>> 
>> Jim, GDB development is moving a lot slower than it should.
> 
> My experience is very different.  Every change that I suggested until
> now, for the past 2 years or so that I'm involved with GDB
> maintenance, was usually reviewed within 1-2 weeks of my posting it
> as an RFA.  A few times I needed to post a reminder (I usually do that
> after more than a week's passed without any replies).  A couple of
> times, I needed more than one reminder, but that's an exception rather
> than the rule, in my experience.
This is because you work on different parts of GDB then I do.
As I just pointed out to Stan, i'm not alone.   I received 4 replies to the
message I posted saying "I know exactly how you feel" or "Your not
kidding".  These are people who no longer contribute to GDB, saying
they are waiting for a time when they can get, for instance, a simple
Makefile.in patch, reviewed within 3 months.
That's sad.
> 
> In most cases, I had my patches reviewed and approved in 2-3 weeks,
> sometimes a month.  In one exceptional case, it took 3 or 4 months,
> but that was my first large submission, and I failed to ping the
> relevant maintainer more than once.
> 
> So GDB development is not slow, in my opinion.  More importantly, I'm
> always able to get my patches accepted by using the normal channels,
> such as pinging people from time to time.
> 
> In other words, the development procedures work.
Maybe for your area of GDB.
Not for mine.
And apparently, not for some others.

Do you really think i'd complain if I was alone here?
If I thought it was just me?


-- 
"The other day, I was walking my dog around my building...  on
the ledge.  Some people are afraid of heights.  Not me, I'm
afraid of widths.
"-Steven Wright

Re: Rewriting the type system

[Eli Zaretskii]

> From: Daniel Berlin <dan@cgsoftware.com>
> Date: 12 Jun 2001 12:12:49 -0400
> 
> > In other words, the development procedures work.
> Maybe for your area of GDB.
> Not for mine.
> And apparently, not for some others.

It's possible that some maintainers are less responsive than others,
and that I had luck talking to one kind while you to the other.  By
posting my experience I didn't mean to say that you made up this
stuff.  I just wanted to put things into a proper perspective.

> Do you really think i'd complain if I was alone here?

I don't think it's wrong to complain about slow response.  _How_ you
complain is important, however.

Re: Rewriting the type system

[Jim Blandy]

Daniel Berlin <dan@cgsoftware.com> writes:
> Jim, GDB development is moving a lot slower than it should.  If
> someone told me, after just rewriting the typesystem, that i needed to
> redo it from scratch, i'd probably just start making my own GDB
> releases instead (in effect, forking GDB).  Not out of spite or
> anything, just so i could get stuff done without having to keep track
> of tons of patches that go months without review.

I apologize if I have been slow to review patches from time to time.
I don't like to feel that I'm a bottleneck in GDB development.

However, in your case, Dan, I think we can back up and ask a more
fundamental question.  You're a smart guy; you've got a lot of energy;
you have a lot of time, too, apparently.  You've been working on GDB
for at least a year and a half now.  You've taken on large, ambitious
tasks.

So, given all that, why are you in the position of waiting for my
approval for your changes?  Why aren't you listed as a maintainer of
the Dwarf2 reader and the symtab reader?

It's because, for whatever reason, you don't take the time to make
your changes correct.

Here is part of the function gnuv3_rtti_type in gnu-v3-abi.c, from
your GCC V3 C++ ABI support patch:

    coreptr =
      *((CORE_ADDR *) (VALUE_CONTENTS_ALL (v) + VALUE_OFFSET (v) +
                       VALUE_EMBEDDED_OFFSET (v)));
    coreptr -= 12;
    *top =
      value_as_long (value_at
                     (builtin_type_int, coreptr, VALUE_BFD_SECTION (v)));
    coreptr += 8;
    vp = value_at (builtin_type_int, coreptr, VALUE_BFD_SECTION (v));

Here we have, in the space of less than a dozen lines of code:

- host == target assumptions (why are you applying `*' to
  target-format data?)

- sizeof (foo) assumptions (what is 8?  what is 12?)

It makes me angry to be asked to review this kind of patch.  If you're
not going to bother with even the most basic rules of GDB development,
why should I bother to read your patches?

You have an extensive history of reverted changes:

2000-11-07  Daniel Berlin  <dberlin@redhat.com>

	* dwarf2read.c: Revert June 5th change for caching of types,
	as per Jim Blandy's request.

and the changes to lookup_symbol that were broken, the minsym sorting
issue that Peter Schauer called you on, etc.

One of the reasons it took me three or four months to review your GCC
V3 C++ ABI changes was that I had to learn enough C++ to be able to
understand the ABI spec.  Isn't that odd?  Why do we need to have our
C++ maintainer's patches reviewed by someone who doesn't know the
language?  As someone said recently, "This, IMHO, is amazingly silly."

Given your record, I'm not sure I think you should be the GDB C++
maintainer, or the maintainer of any part of GDB.

Re: Rewriting the type system

[Daniel Berlin]

Jim Blandy <jimb@zwingli.cygnus.com> writes:

> Daniel Berlin <dan@cgsoftware.com> writes:
>> Jim, GDB development is moving a lot slower than it should.  If
>> someone told me, after just rewriting the typesystem, that i needed to
>> redo it from scratch, i'd probably just start making my own GDB
>> releases instead (in effect, forking GDB).  Not out of spite or
>> anything, just so i could get stuff done without having to keep track
>> of tons of patches that go months without review.
> 
> I apologize if I have been slow to review patches from time to time.
> I don't like to feel that I'm a bottleneck in GDB development.
> 
> However, in your case, Dan, I think we can back up and ask a more
> fundamental question.  You're a smart guy; you've got a lot of energy;
> you have a lot of time, too, apparently.  You've been working on GDB
> for at least a year and a half now.  You've taken on large, ambitious
> tasks.
> 
> So, given all that, why are you in the position of waiting for my
> approval for your changes?  Why aren't you listed as a maintainer of
> the Dwarf2 reader and the symtab reader?
> 
> It's because, for whatever reason, you don't take the time to make
> your changes correct.

Now that's simply bullshit.

> 
> Here is part of the function gnuv3_rtti_type in gnu-v3-abi.c, from
> your GCC V3 C++ ABI support patch:
> 
>     coreptr =
>       *((CORE_ADDR *) (VALUE_CONTENTS_ALL (v) + VALUE_OFFSET (v) +
>                        VALUE_EMBEDDED_OFFSET (v)));
>     coreptr -= 12;
>     *top =
>       value_as_long (value_at
>                      (builtin_type_int, coreptr, VALUE_BFD_SECTION (v)));
>     coreptr += 8;
>     vp = value_at (builtin_type_int, coreptr, VALUE_BFD_SECTION (v));
> 
> Here we have, in the space of less than a dozen lines of code:
> 
> - host == target assumptions (why are you applying `*' to
>   target-format data?)
> 
> - sizeof (foo) assumptions (what is 8?  what is 12?)
> 

Neat, but that code was written 2 years ago, when i was first starting
gdb development.
It was introduced into value_rtti_type, and copied in the gnuv3 rtti
type because gnuv3-abi.c was based on gnuv2-abi.c, which  was based on
all that code. 


> It makes me angry to be asked to review this kind of patch.  If you're
> not going to bother with even the most basic rules of GDB
> development,

Really?
That's pretty frickin funny.
Please point out the problems with the misc_obstack changes I
submitted 2 weeks ago, unreviewed.

> why should I bother to read your patches?

Because as maintainer, it's your job?
> 
> You have an extensive history of reverted changes:
> 
> 2000-11-07  Daniel Berlin  <dberlin@redhat.com>
> 
> 	* dwarf2read.c: Revert June 5th change for caching of types,
> 	as per Jim Blandy's request.
> 
> and the changes to lookup_symbol that were broken,

Actually, Jim, they changes to lookup_symbol were broken because of
two things.  First, an undocumented change 4 years ago (IE nobody
updated the comments) that caused demangled names to not appear on
partial symbols, only full symbols and minimal symbols.
I submitted a patch to fix the problem by documenting the behavior,
and reverting the binary search on partial symbols, which took until
Jim Ingham noticed the same damn problem 3 months later, and raised
more hell, for you to actually review the  damn patch and approve it.
Remember that part?

>  the minsym sorting
> issue that Peter Schauer called you on, 

What are you talking about?
> etc.

Cute.
Extensive history through 1 example.

> 
> One of the reasons it took me three or four months to review your GCC
> V3 C++ ABI changes was that I had to learn enough C++ to be able to
> understand the ABI spec.  Isn't that odd?  Why do we need to have our
> C++ maintainer's patches reviewed by someone who doesn't know the
> language?
Because they touch the symbol code?
Or the types code?
Or the values code?
Because language support in GDB is hopelessly intertwined with other
parts of GDB, such that you can't fix one thing without touching 18
other parts?
It's nice of you to try to imply that most of my patches are wrong, when they
aren't. 
>   As someone said recently, "This, IMHO, is amazingly silly."
Hey, maybe I should start taking your quotes out of context too.

> 
> Given your record, I'm not sure I think you should be the GDB C++
> maintainer, or the maintainer of any part of GDB.

Okay Jim, you want it, you got it.
I officially give up as C++ maintainer.
I've removed myself from the MAINTAINERS list.
Have fun.
Doesn't change anything, does it?
Don't review my patches, or the patches of others, and i'll complain.


-- 
"My friend Winnie is a procrastinator.  He didn't get his
birthmark until he was eight years old.
"-Steven Wright

Re: Rewriting the type system

[Daniel Berlin]

Jim Blandy <jimb@zwingli.cygnus.com> writes:

I'm also curious Jim, did it make you "angry" to review  Jason
Merril's 3 line fix for dbxread.c and dwarfread.c, that he submitted
12-15-2000?
You know, that completely incorrect code that said "If we are
processing gcc compilation, it must be old abi demangling"?
Oh wait, you never reviewed it.
That's right.
Elena did, over a month (1 month, 1 day to be exact) later. 

Why is it I can get patches that fix crashing bugs in "set
demangle-style" reviewed in hours,  but when i, or others,  submit
something in your area, it's like a black hole? 

I'm pretty sure you don't want me to start dredging out all the
examples where either you've been plain old non-responsive, or taken
months.
Or code that makes me angry to look at, that you've written.
For fun, let's take our recent friend, find_toplevel_char.

2000-01-27 Jim Blandy <jimb@cygnus.com>

* symtab.c (decode_line_1): Don't let commas that are within
        quotes or parenthesis terminate the line spec.  Don't use pp when
        removing the final double quote of a double-quoted string.  Don't
        forget to skip the opening double quote.  I have no clue whether
        this change is correct; probably we've just moved this function
        from one buggy place to another buggy place, and never came within
        an outhouse whiff of correctness.
        (find_toplevel_char): New function.

Funny, doesn't seem to me like you tried to make sure the code was
correct.

In fact, it was your addition that caused the problem.

Like I said, you *really* don't want to start playing that
"correctness" of code game. 
Neither of us will win. in fact, nobody would win.
The changelogs show that, for instance, Andrew had his last patch to
dwarf2read.c reverted.  3 years later, of course.

I've had *one* patch reverted.
The changelogs clearly show this.
We've all had about the same number of patches reverted.
So please, stop trying to pretend i'm a special case here, or that
i'm a bad egg, who submits bad patches.  
It's simply not true.

--Dan

> Daniel Berlin <dan@cgsoftware.com> writes:
>> Jim, GDB development is moving a lot slower than it should.  If
>> someone told me, after just rewriting the typesystem, that i needed to
>> redo it from scratch, i'd probably just start making my own GDB
>> releases instead (in effect, forking GDB).  Not out of spite or
>> anything, just so i could get stuff done without having to keep track
>> of tons of patches that go months without review.
> 
> I apologize if I have been slow to review patches from time to time.
> I don't like to feel that I'm a bottleneck in GDB development.
> 
> However, in your case, Dan, I think we can back up and ask a more
> fundamental question.  You're a smart guy; you've got a lot of energy;
> you have a lot of time, too, apparently.  You've been working on GDB
> for at least a year and a half now.  You've taken on large, ambitious
> tasks.
> 
> So, given all that, why are you in the position of waiting for my
> approval for your changes?  Why aren't you listed as a maintainer of
> the Dwarf2 reader and the symtab reader?
> 
> It's because, for whatever reason, you don't take the time to make
> your changes correct.
> 
> Here is part of the function gnuv3_rtti_type in gnu-v3-abi.c, from
> your GCC V3 C++ ABI support patch:
> 
>     coreptr =
>       *((CORE_ADDR *) (VALUE_CONTENTS_ALL (v) + VALUE_OFFSET (v) +
>                        VALUE_EMBEDDED_OFFSET (v)));
>     coreptr -= 12;
>     *top =
>       value_as_long (value_at
>                      (builtin_type_int, coreptr, VALUE_BFD_SECTION (v)));
>     coreptr += 8;
>     vp = value_at (builtin_type_int, coreptr, VALUE_BFD_SECTION (v));
> 
> Here we have, in the space of less than a dozen lines of code:
> 
> - host == target assumptions (why are you applying `*' to
>   target-format data?)
> 
> - sizeof (foo) assumptions (what is 8?  what is 12?)
> 
> It makes me angry to be asked to review this kind of patch.  If you're
> not going to bother with even the most basic rules of GDB development,
> why should I bother to read your patches?
> 
> You have an extensive history of reverted changes:
> 
> 2000-11-07  Daniel Berlin  <dberlin@redhat.com>
> 
> 	* dwarf2read.c: Revert June 5th change for caching of types,
> 	as per Jim Blandy's request.
> 
> and the changes to lookup_symbol that were broken, the minsym sorting
> issue that Peter Schauer called you on, etc.
> 
> One of the reasons it took me three or four months to review your GCC
> V3 C++ ABI changes was that I had to learn enough C++ to be able to
> understand the ABI spec.  Isn't that odd?  Why do we need to have our
> C++ maintainer's patches reviewed by someone who doesn't know the
> language?  As someone said recently, "This, IMHO, is amazingly silly."
> 
> Given your record, I'm not sure I think you should be the GDB C++
> maintainer, or the maintainer of any part of GDB.

-- 
"I was once arrested for walking in someone else's sleep.
"-Steven Wright

Re: Rewriting the type system

[Stan Shebs]

Daniel Berlin wrote:
> 
> Jim Blandy <jimb@zwingli.cygnus.com> writes:
> 
> > It's because, for whatever reason, you don't take the time to make
> > your changes correct.
> 
> Now that's simply bullshit.

OK OK.  For openers, let's agree not to do personal accusations and
profanity on the list.  Flame each other in private please, it will
feel just as good and not waste everybody else's time.

> > Here we have, in the space of less than a dozen lines of code:
> >
> > - host == target assumptions (why are you applying `*' to
> >   target-format data?)
> >
> > - sizeof (foo) assumptions (what is 8?  what is 12?)
> 
> Neat, but that code was written 2 years ago, when i was first starting
> gdb development.
> It was introduced into value_rtti_type, and copied in the gnuv3 rtti
> type because gnuv3-abi.c was based on gnuv2-abi.c, which  was based on
> all that code.

Dan, if it's mistaken, it's mistaken; excuses and history aren't really
that interesting.  We all have things we're not that proud of, myself
probably more than anybody.  Let's fix them and go on to the next thing.

> > why should I bother to read your patches?
> 
> Because as maintainer, it's your job?

You're right here, Dan.  Everybody here talks about maintenance as
if it's some kind of signal honor, but no, it's just a responsibility,
like peer-reviewing scientific papers.  You don't get to read the
title and say "Oh, Dr. Luser again, straight to the trash it goes."
Gotta read the whole thing and provide meaningful feedback, and
worse, you're expected to do it within a certain time period.  The
worse the patch, the more work it will be to review.

> > You have an extensive history of reverted changes:

Jim, everybody who's worked extensively on GCC or GDB has a history
of reverted changes.  It we waited until we were sure that each change
was perfect, progress would be appallingly slow.  Every day the GCC
mainline gets patches that raise the hairs on the back of my neck,
and I don't relax until I've confirm they don't break the target for
which I'm responsible.  Some of the patches do break GCC; the patch
gets fixed or reverted, and everybody moves on.

> [...on and on...]
> It's nice of you to try to imply that most of my patches are wrong, when they
> aren't.

Both of you are in the wrong here.  Jim, you know as well as anyone
that the C++ symbol handling parts of GDB are not so neatly partitioned
that you can review symbol patches without knowing more than a bit about
C++, and Dan, you should be able to admit mistakes in your patches, fix
them and resubmit, rather than flaming the reviewer of the patch.

There is plenty of room to criticize both maintainer responsiveness
and patch quality without turning it into a spitting match, and I'm
disappointed to see the development process sink to this level.

Stan

Re: Rewriting the type system

[Andrew Cagney]

> Hell, the simple bcache change i submitted last year (updating the
> starting constant, fix the indenting) still hasn't
> been reviewed.


This patch?

I'm suprized you didn't take the initiative here and check it in as an 
obvious fix.

	Andrew


> Index: bcache.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/bcache.c,v
> retrieving revision 1.4
> diff -c -3 -p -r1.4 bcache.c
> *** bcache.c	2000/06/05 20:49:53	1.4
> --- bcache.c	2000/12/08 05:01:18
> ***************
> *** 28,51 ****
>   #include "bcache.h"
>   #include "gdb_string.h"		/* For memcpy declaration */
>   
> ! /* The old hash function was stolen from SDBM. This is what DB 3.0 uses now,
>    * and is better than the old one. 
>    */
>   \f
>   unsigned long
>   hash(void *addr, int length)
>   {
> ! 		const unsigned char *k, *e;
> ! 		unsigned long h;
> ! 		
> ! 		k = (const unsigned char *)addr;
> ! 		e = k+length;
> ! 		for (h=0; k< e;++k)
> ! 		{
> ! 				h *=16777619;
> ! 				h ^= *k;
> ! 		}
> ! 		return (h);
>   }
>   \f
>   /* Growing the bcache's hash table.  */
> --- 28,55 ----
>   #include "bcache.h"
>   #include "gdb_string.h"		/* For memcpy declaration */
>   
> ! /* The old hash function was stolen from SDBM. This is what DB 3.1 uses now,
>    * and is better than the old one. 
> +  * It's the Foller/Noll/Vo hash code, for those who care.
>    */
> + #define FNV0_32_INIT (0) /* To use FNV-0, not recommended */
> + #define FNV1_32_INIT (0x811c9dc5) 
> + #define FNV_32_PRIME (0x01000193)
>   \f
>   unsigned long
>   hash(void *addr, int length)
>   {
> !   const unsigned char *k, *e;
> !   unsigned long h=FNV1_32_INIT;
> !   
> !   k = (const unsigned char *)addr;
> !   e = k+length;
> !   for (; k< e;++k)
> !     {
> !       h *= FNV_32_PRIME;
> !       h ^= *k;
> !     }
> !   return (h);
>   }
>   \f
>   /* Growing the bcache's hash table.  */
> 
> 
> 

Re: Rewriting the type system

[Daniel Berlin]

Andrew Cagney <ac131313@cygnus.com> writes:

>> Hell, the simple bcache change i submitted last year (updating the
>> starting constant, fix the indenting) still hasn't
>> been reviewed.
> 
> 
> This patch?
Yup.

> 
> I'm suprized you didn't take the initiative here and check it in as an
> obvious fix.

Really?
It's in Jim's area.
Jim tells me my patches aren't correct. 
Don't you think "taking the initiative" and just up and checking it in would
step on his toes a little bit?
Believe it or not, I tried to avoid doing this when possible, which it
wasn't always in order to get stuff done.
We also had no clear ruling on the obvious fix rule yet, anyway.

Besides, if it's an obvious fix, why didn't he just take a few minutes to
look it over when it arrived in his inbox (you'll note the original
message was copied to him), and reply "looks okay"? Since if it's
obvious, it shouldn't take him more than probably 1 or 2 minutes to
look over a 20-30 line patch.

Just like a software project becomes late one day at a time, GDB is
improved one patch at a time.
If some of you maintainer guys and gals (i'm no longer a maintainer,
so it's not "we") ignore small patches (and i'm not implying any one
particular person does), 
A. You won't get the effect a piling up of small improvements has.
B. You won't spur the people who made the small changes to make
   larger, more ambitious ones.

I haven't touched the bcache since submitting that patch, and getting
no response. It's not because the bcache is perfect. It's because who
the heck wants to make large changes to anything if they'll just go
off into a void?

> 
> 
> 	Andrew
> 

-- 
"I have two very rare photographs.  One is a picture of Houdini
locking his keys in his car.  The other is a rare photograph of
Norman Rockwell beating up a child.
"-Steven Wright

Re: Rewriting the type system

[Eli Zaretskii]

> Date: Tue, 12 Jun 2001 13:50:16 -0700
> From: Stan Shebs <shebs@apple.com>
> 
> Both of you are in the wrong here.  Jim, you know as well as anyone
> that the C++ symbol handling parts of GDB are not so neatly partitioned
> that you can review symbol patches without knowing more than a bit about
> C++, and Dan, you should be able to admit mistakes in your patches, fix
> them and resubmit, rather than flaming the reviewer of the patch.
> 
> There is plenty of room to criticize both maintainer responsiveness
> and patch quality without turning it into a spitting match, and I'm
> disappointed to see the development process sink to this level.

I agree 110%.

Exchanging accusations will get us nowehere; and if Dan feels his
patches are consistently ignored, then something is not right in the
process of reviewing those patches and providing relevant feedback
back to Dan.

In particular, if there are aspects in Dan's (or anybody else's) code
that the maintainer wants changed, then those aspects should be
pointed out.  I know I had such comments on my submissions from time
to time, and I'm sure everyone else had as well.

But leaving patches unreviwed or the person who submitted them in an
uncertain state is something that I think should be avoided at all
costs.

Re: obvious set_cu_language patch

[Elena Zannoni]

Per Bothner writes:
 > I consider this one fairly obvious.

Per, did you commit this?
If not, go ahead.

Elena


 > 
 > 2001-06-07  Per Bothner  <per@bothner.com>
 > 
 > 	* dwarf2read.c (set_cu_language):  Handle DW_LANG_Java.
 > 
 > Index: dwarf2read.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dwarf2read.c,v
 > retrieving revision 1.24
 > diff -u -r1.24 dwarf2read.c
 > --- dwarf2read.c	2001/04/30 10:30:27	1.24
 > +++ dwarf2read.c	2001/06/08 06:18:51
 > @@ -3721,6 +3721,9 @@
 >      case DW_LANG_Mips_Assembler:
 >        cu_language = language_asm;
 >        break;
 > +    case DW_LANG_Java:
 > +      cu_language = language_java;
 > +      break;
 >      case DW_LANG_Ada83:
 >      case DW_LANG_Cobol74:
 >      case DW_LANG_Cobol85:
 > 
 > -- 
 > 	--Per Bothner
 > per@bothner.com   http://www.bothner.com/per/
 >