Re: CVS commit: src/usr.bin/make

[Roland Illig <roland.illig%gmx.de@localhost>]

On 02.08.2020 13:06, Simon Burge wrote:
> "Roland Illig" wrote:
>
>> Module Name:	src
>> Committed By:	rillig
>> Date:		Sun Aug  2 09:43:22 UTC 2020
>>
>> Modified Files:
>>
>> 	src/usr.bin/make: var.c
>>
>> Log Message:
>>
>> make(1): use shorter local variable names
>>
>> The c in cp was redundant since the context makes it obvious that this
>> is a character pointer. In a tight loop where lots of characters are
>> compared, every letter counts.
>
> I don't understand the intent of this commit.  Are you saying the length
> of a C variable name has some sort of impact on code execution speed?

No, I know that it has absolutely no influence on execution speed.

My concern was about the ease of reading by humans.  In var.c, there are
several functions that do low-level parsing by reading the input byte by
byte and comparing the current character.  These functions are
ParseModifierPart, ApplyModifier_Defined, ApplyModifier_Match,
ModifyWord_SubstRegex.

In these functions, there are often 3 to 4 comparisons in a single line.
The comparisons all have the same left-hand side, so that expression
becomes uninteresting to the reader and should therefore be as short as
possible, to leave room for the more interesting details, which are the
right-hand sides of the comparisons.

Therefore I renamed the cp to p, since the c did not add any valuable
information to the code.  It's pretty obvious that in a comparison like
*p == '$', the p must be a pointer to a character.

Only in these very tight loops, and only because these have only a
single interesting pointer, do I prefer this short variable name.  For
comparison, in ApplyModifier_Match, in the second big loop, I named the
variables src and dst, to keep them apart.

Another reason for renaming cp and cp2 is that the make(1) source code
uses these names in a lot of places, and these names do not add any
valuable information to the reader, while a more expressive variable
name could do that very well.  Therefore I'm generally against this
variable name, especially when these variables are used as
general-purpose storage that serves 3 or 4 completely distinct purposes
during its lifetime, happily mixing between storing const char * and
char * and maybe a void * in between, followed by UNCONST to free the cp
at the end.  How should a casual reader know at the end what exactly is
freed here?  It's obviously cp, but what does this mean?

In summary: By the variable name p, I mean a moving pointer in a parsing
function, therefore it's usually the only moving pointer in that
function.  I'm using that variable name consistently for this single
purpose.  This is unlike the original make(1) code, which uses cp and
cp2 for everything.

I notice I'm getting too much into rant mode right now, therefore I'd
rather stop here. :)

Roland