Re: proposed patches to fix local variable naming glitches, etc.

[Akim Demaille]

 >> From: Akim Demaille
 >> Date: Tue, 21 Jan 2003 14:23:43 +0100

 >> My hope is that no inconsistency was introduced in the variable names,
 >> and that `s' is *always* the name of a state, not of anything else.

 Paul> I went through the latest CVS looking for instances of "s" that are
 Paul> neither states nor state numbers, and similarly for "r" and rules or
 Paul> rule_numbers, and came up with the following proposed patch.

Wow, that's brave.  I still cannot digest the huge name changes you
introduced, nor the shift in conventions, but I would never have asked
to someone to perform this kind of job.

 Paul> After looking at the code some more, I think it's OK to have "state
 Paul> *s" in some functions, and "state_number s" in others.  The two uses
 Paul> do not collide in practice, and even if they did collide the compiler
 Paul> would catch misuses of state* values in state_number contexts, or vice
 Paul> versa.  

Definitely.

 Paul> It's convenient to think of "s" as a state, even though it is
 Paul> never exactly the C type "state" in practice.

Agreed.

 >> and it took me months to have some form of homogeneity.  But more
 >> was needed, and now I'm just depressed thinking about this.

 Paul> After looking at the code more closely, I see what you mean about the
 Paul> code being nonhomogeneous.  

Well, have a look at 1.28 to have the full picture.  Observe that
there is no state_t nor rule_t in the original code.  Err, I meant
state and rule <grin>.

 Paul> There are still many instances of "i" and "j", for example,
 Paul> that are actually state numbers or something like that.  But
 Paul> I'm not sure that we should insist on extreme homogeneity here;
 Paul> at some point there are diminishing returns.

Agreed too.  This kind of cleaning was performed only when the code
was to be worked on for other reasons.

 Paul> By the way, while generating this proposed patch I noticed that
 Paul> src/gram.h has an unused decl; this patch removes it.

It would be good to have Automake support for some lint.

Please, install.