[rfc] plans for linespec.c

[rfc] plans for linespec.c

[David Carlton]

I've just submitted the last of a series of patches to linespec.c that
begin cleaning up the function decode_line_1.  So now it's 160 lines
long instead of 780 lines long, or whatever it was before.  (Of
course, the extra lines haven't gone away, they've just moved into
their own functions.)

It still has a long way to go; right now, there are a few different
ways in which progress can be made.

* The C++ stuff should be broken up into smaller functions, just like
  I did with decode_line_1.  It's already divided up into multiple
  functions, but some of them are still too large.

* decode_line_1 should be cleaned up still further.  My goal is to
  have it eventually look as follows:

  decode_line_1 ()
  {
    /* Some functions to initialize all the various flags and
       variables.  */

    /* Several blocks of the following form, for different values of
       XXX.  */

    if (is_XXX ())
      return decode_XXX ()
  }

  So, basically, some of the predicates have to get simpler, and some
  of the initialization code (set_flags, symtab_from_filename) should
  move earlier in the function.

* Trivial fixes for clarity/ARI: make sure comments end in a period
  and two spaces, rename variables so that their name reflects their
  use, define the functions in a consistent order, etc.

These three are all logically independent of each other.  I think
Elena suggested that the trivial fixes could be handled as obvious
patches once decode_line_1 had gotten simplified a bit; it seems to me
that now is an appropriate time to start on that.  And I think I'd
rather do the C++ stuff before cleaning up decode_line_1: the logic
behind those patches is simpler, and I've finished that in my own
private copy of the file whereas I haven't finished all of the further
decode_line_1 cleanup in my private copy.

So what makes sense for me to do is:

1) Start doing some obvious patches that only do stuff like change
   formatting, rename variables, etc.; unless Elena or somebody else
   objects, I'll post these to gdb-patches but I won't wait for
   approval, since they clearly won't change GDB's behavior.

2) Start submitting a series of patches to break up decode_compound
   and functions that it calls into smaller functions, just like I've
   been doing with decode_line_1.  These will, of course, require
   approval.

Once all that's done, I'll then get back to cleaning up decode_line_1
some more.

How does that sound?

David Carlton
carlton@math.stanford.edu

Re: [rfc] plans for linespec.c

[Elena Zannoni]

David Carlton writes:
 > I've just submitted the last of a series of patches to linespec.c that
 > begin cleaning up the function decode_line_1.  So now it's 160 lines
 > long instead of 780 lines long, or whatever it was before.  (Of
 > course, the extra lines haven't gone away, they've just moved into
 > their own functions.)
 > 
 > It still has a long way to go; right now, there are a few different
 > ways in which progress can be made.
 > 
 > * The C++ stuff should be broken up into smaller functions, just like
 >   I did with decode_line_1.  It's already divided up into multiple
 >   functions, but some of them are still too large.
 > 

It's a first step, and eventually can be moved to a separate file.

 > * decode_line_1 should be cleaned up still further.  My goal is to
 >   have it eventually look as follows:
 > 
 >   decode_line_1 ()
 >   {
 >     /* Some functions to initialize all the various flags and
 >        variables.  */
 > 
 >     /* Several blocks of the following form, for different values of
 >        XXX.  */
 > 
 >     if (is_XXX ())
 >       return decode_XXX ()
 >   }
 > 
 >   So, basically, some of the predicates have to get simpler, and some
 >   of the initialization code (set_flags, symtab_from_filename) should
 >   move earlier in the function.
 > 

Sounds sane.

 > * Trivial fixes for clarity/ARI: make sure comments end in a period
 >   and two spaces, rename variables so that their name reflects their
 >   use, define the functions in a consistent order, etc.
 > 

yep.

 > These three are all logically independent of each other.  I think
 > Elena suggested that the trivial fixes could be handled as obvious
 > patches once decode_line_1 had gotten simplified a bit; it seems to me
 > that now is an appropriate time to start on that.  And I think I'd
 > rather do the C++ stuff before cleaning up decode_line_1: the logic
 > behind those patches is simpler, and I've finished that in my own
 > private copy of the file whereas I haven't finished all of the further
 > decode_line_1 cleanup in my private copy.
 > 
 > So what makes sense for me to do is:
 > 
 > 1) Start doing some obvious patches that only do stuff like change
 >    formatting, rename variables, etc.; unless Elena or somebody else
 >    objects, I'll post these to gdb-patches but I won't wait for
 >    approval, since they clearly won't change GDB's behavior.
 > 

This is ok, as long as the patches are simple. I.e. better have more
smaller patches, rather than a big one. But you know this already!

 > 2) Start submitting a series of patches to break up decode_compound
 >    and functions that it calls into smaller functions, just like I've
 >    been doing with decode_line_1.  These will, of course, require
 >    approval.
 > 
 > Once all that's done, I'll then get back to cleaning up decode_line_1
 > some more.
 > 
 > How does that sound?
 > 

Sounds good.
Elena


 > David Carlton
 > carlton@math.stanford.edu

Re: [rfc] plans for linespec.c

[David Carlton]

On Wed, 8 Jan 2003 09:29:46 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> So what makes sense for me to do is:

>> 1) Start doing some obvious patches that only do stuff like change
>> formatting, rename variables, etc.; unless Elena or somebody else
>> objects, I'll post these to gdb-patches but I won't wait for
>> approval, since they clearly won't change GDB's behavior.

> This is ok, as long as the patches are simple. I.e. better have more
> smaller patches, rather than a big one. But you know this already!

Great, I'll make sure to keep those patches small.

>> 2) Start submitting a series of patches to break up decode_compound
>> and functions that it calls into smaller functions, just like I've
>> been doing with decode_line_1.  These will, of course, require
>> approval.

>> Once all that's done, I'll then get back to cleaning up decode_line_1
>> some more.

>> How does that sound?

> Sounds good.

Okay, it's a plan, then.

David Carlton
carlton@math.stanford.edu

Re: [rfc] plans for linespec.c

[Michael Elizabeth Chastain]

Hi David,

Would it be reasonable for you to double your testing and test
stabs+ as well as dwarf-2?

I'm not worried about gcc 2.95.3, gcc HEAD, and so on, because eventually
my dragnet catches that stuff and I feed it back.  But there are enough
differences between dwarf-2 and stabs+ that I would be more comfortable
if you tested with both before committing symbol-manipulation patches.

Other than that, I'm all for your plan of attack on decode_line_1.

Michael C

Re: [rfc] plans for linespec.c

[David Carlton]

On Tue, 7 Jan 2003 18:12:39 -0600, Michael Elizabeth Chastain <mec@shout.net> said:

> Would it be reasonable for you to double your testing and test
> stabs+ as well as dwarf-2?

> I'm not worried about gcc 2.95.3, gcc HEAD, and so on, because
> eventually my dragnet catches that stuff and I feed it back.  But
> there are enough differences between dwarf-2 and stabs+ that I would
> be more comfortable if you tested with both before committing
> symbol-manipulation patches.

Let me give you some more background to the patches that I've been
doing to linespec.c: they're really simple, almost exclusively
consisting of taking an existing coherent chunk of code and extracting
it as a separate function.  I try very hard not to change the flow of
execution at all (or only in ways that are trivially correct); while
some patches will come eventually that do change the flow of execution
somewhat, they'll be more in the nature of tweaking the order in which
stages of parsing happen, rather than anything that gets too close to
symbol manipulation.

So, while it's certainly necessary for me to run the test suite to
make sure that I haven't screwed up (after all, I did screw up
sometimes when I initially did this work; it would be nice if there
were automated tools to handle some of this, but never mind that), I'm
not sure that your level of concern is warranted.  My machine is old,
so it would turn a 10-minute testing process into a 20-minute testing
process; I'm happy to do that if you think it's important, but I'm not
sure that it is in this particular instance.  (I am, however, worried
about problems that the test suite might not catch at all: it would be
nice if somebody were to make sure that the testsuite systematically
tested everything that decode_line_1 does.  But that's another issue
entirely.)

Does that help?  If you still have concerns, let me know why and I'll
be happy to talk further.

David Carlton
carlton@math.stanford.edu

Re: [rfc] plans for linespec.c

[Michael Elizabeth Chastain]

Hi David,

> ... I'm not sure that your level of concern is warranted.  My machine
> is old, so it would turn a 10-minute testing process into a 20-minute
> testing process; I'm happy to do that if you think it's important,
> but I'm not sure that it is in this particular instance.

It's kinda subjective.  I tend to be conservative about these things, and
I tend not to think of testing as getting in the way of my work flow (I
like to proofread while the tests are running).  So it's not "you should I
do this or I predict calamity" issue.  If you choose not to, I won't kick.
If lots of new stabs+ regressions show up then I will want it more.

I know decode_line_1 does not touch symbol table implementation much
but the symbols that come in from the different symbol table readers
are quite, well, different.  In fact I'd like to add coff or xcoff to
my own testing.

> (I am, however, worried about problems that the test suite might not
> catch at all: it would be nice if somebody were to make sure that the
> testsuite systematically tested everything that decode_line_1 does.
> But that's another issue entirely.)

Grin.  You are the expert on coverage cases for decode_line_1 now!
Unfortunately it's very difficult psychologically to work on a piece
of code and then write test cases designed to break it.

Michael C

Re: [rfc] plans for linespec.c

[David Carlton]

On Wed, 8 Jan 2003 22:24:08 -0600, Michael Elizabeth Chastain <mec@shout.net> said:
> Hi David,

>> ... I'm not sure that your level of concern is warranted.  My machine
>> is old, so it would turn a 10-minute testing process into a 20-minute
>> testing process; I'm happy to do that if you think it's important,
>> but I'm not sure that it is in this particular instance.

> It's kinda subjective.  I tend to be conservative about these
> things, and I tend not to think of testing as getting in the way of
> my work flow (I like to proofread while the tests are running).  So
> it's not "you should I do this or I predict calamity" issue.  If you
> choose not to, I won't kick.  If lots of new stabs+ regressions show
> up then I will want it more.

Fair enough.  It's something I should get around to doing eventually
for other reasons; maybe your nudging will move it up some.

>> (I am, however, worried about problems that the test suite might not
>> catch at all: it would be nice if somebody were to make sure that the
>> testsuite systematically tested everything that decode_line_1 does.
>> But that's another issue entirely.)

> Grin.  You are the expert on coverage cases for decode_line_1 now!

There's actually an interesting question here as to whether or not I
even can design coverage tests for the original version of
decode_line_1: my only understanding of the function comes from
decomposing it, so any tests that I'd design would be compromised by
the assumption that I've preserved the behavior by that
decomposition.  Though, obviously, I'd be in a good position for
designing coverage tests for the newer version.

> Unfortunately it's very difficult psychologically to work on a piece
> of code and then write test cases designed to break it.

I'm trying to get better at writing test cases.  My next task for my
non-GDB programming project is to get a unit test system running, both
for new code and for existing code, so hopefully test writing will get
easier for me.

Speaking of which, I had the thought that maybe we could do unit tests
of some of the innards of GDB by writing separate test programs that
we link with -lgdb.  Something to think about.

David Carlton
carlton@math.stanford.edu

Re: [rfc] plans for linespec.c

[Joel Brobecker]

> There's actually an interesting question here as to whether or not I
> even can design coverage tests for the original version of
> decode_line_1: my only understanding of the function comes from
> decomposing it, so any tests that I'd design would be compromised by
> the assumption that I've preserved the behavior by that
> decomposition.  Though, obviously, I'd be in a good position for
> designing coverage tests for the newer version.

In my very modest experience - former engineer for the Canadian Automated
Air Traffic System (CAATS) project -, this is not a problem. If you can
create a set of tests that provide a good coverage of your code, then
your tests will be very helpful later on in any case because they will
allow you to verify that a modification does not introduce any change
in behavior. Any change can then be investigated.

Even if one of your test is wrong because there was a bug in the code
that twisted your understanding of what the function should do, the test
will one day "regress" when you fix the bug in the code. That day, the
regression will be investigated and the test will be corrected.
Eventually, you'll get close to 100% correct base of tests...

Most of the time on CAATS, the author of a given piece of code was also
given the task of doing the "unit testing", which includes writing
enough test drivers to exercice as much code as possible. These unit
test drivers proved to be invaluable. Hard to maintain sometimes (the
software quality was not always the same as in the CAATS code itself,
sigh), but very very very valuable, even today now that the project
has been delivered. Especially today I'd like to say, for the CAATS team
has lost a lot of its talents.

-- 
Joel

Re: [rfc] plans for linespec.c

[Michael Elizabeth Chastain]

David Carlton writes:
> Fair enough.  It's something I should get around to doing eventually
> for other reasons; maybe your nudging will move it up some.

Right.  I have to allocate my time between 'do stuff with tangible
gdb benefits' and 'do stuff that improves my own infrastructure'.
Sometimes I feel like I'm just infrastructuring my test bed too much.

> Speaking of which, I had the thought that maybe we could do unit tests
> of some of the innards of GDB by writing separate test programs that
> we link with -lgdb.  Something to think about.

That is a provocative idea.  I know Jim B wrote one symbol table test
that jumped through a bunch of hoops with the C source code in order
to populate the symbol table in a way that tickled a lookup bug.

On the gloomy side, the test suite can already spot problems faster than
I can file bug reports, and the bug reports are piling up faster than
people can fix them.

Michael C