Re: [rfc] clean up linespec.c

[David Carlton <carlton at math dot stanford dot edu>]

On Thu, 7 Nov 2002 12:30:26 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> For various reasons, I've been cleaning up linespec.c on a branch;
>> I find the resulting version of linespec.c enough more
>> legible/maintainable that I suspect I'm not the only person who
>> would find it useful.

> You bet!

Excellent; glad to be of service.

>> It turns out that decode_line_1 isn't quite as crazy a function as
>> I'd feared.  There's a reasonable flow of control underneath (well,
>> at least there is once you get rid of the unnecessary goto's),
>> though admittedly the C++ part of the function is still pretty
>> complicated, and the function will always consist of a bunch of
>> special cases.

> Having c++ separated in functions, is a first step in moving C++
> support to its own files.....:-)

Yeah, there's definitely something funny going on with the division of
labor there.  At some point I have to understand whose role it should
be do what, and in particular what part of the C++ stuff should be
moved.

>> I hope I didn't break anything, though my only real evidence for
>> that is that I didn't get any new regressions on the testsuite.  (I
>> have no idea how comprehensive the testsuite's coverage of linespec
>> is.)

> ah, gcov data would be useful here... :-)

Hmm.  Learn something new every day.

>> So: should I submit an RFA?  Or, more likely, should I start
>> submitting a bunch of RFA's, one at a time?  I suspect it will take
>> a double-digit number of RFA's to break up the changes into
>> manageable chunks, though if you get sick of reading them, you can
>> always approve the changes all at once. :-)

> Yes a bunch of rfa's in rapid succession is the way to go.  I've
> looked at your version below, and it looks like the patches would be
> fairly obvious.

Yeah, they should be.  The only tricky bit is making sure that I've
been careful about the following: if you have a block of code as
follows

  (code that involves a local variable 'char *var')

that you want to pull it out into a function 'f', then, in general,
you need 'f' to look like

  void f (char **var)
  {
    (same code, but with all uses of 'var' replaced by '*var')
  }

and you replace the original code by f(&var).  But, of course, this is
only necessary if the value of var is used later on in the original
function; and decode_line_1 is full of places where it looks like
variables' uses span blocks, but it turns out that people were just
lazy and reused the names 'p', 'copy', etc. over and over again.

I think I've got it all worked out correctly, though; I'll make notes
in the RFA's when there's something delicate to deal with along those
lines.

> I'd reccommend that the other fixes, like reformatting, adding
> spaces after periods, etc. could be done after the code movement
> patches (or before, whatever is less work for you), as a single
> patch that spans the whole file. That would just be obvious.

I don't know if I'll do that as a single monolithic patch or not, but
I'll make sure to break patches up into RFA's for code movement and
obvious patches for other stuff.

Prepare to be inundated with patches.

David Carlton
carlton@math.stanford.edu