Marius Strobl schrieb:
On Sat, Mar 21, 2009 at 12:03:16PM +0100, Christoph Mallon wrote:
These variables are not redundant. Before the variables had clear names
(like rid or nregs, nintr), now they are called i and j, which carries
no semantics at all.
IMO the code is simple enough so that the variable names do not
need to carry semantics for clearity, the fact that just two
reused temporary variables are enough for the whole function
underlines this.
The function is about 170 lines long, so allow me to disagree that it is
"simple enough". Also the number of variables, which are live at once,
is to my experience not a good measure for code complexity (A high
number usually indicates high complexity, but a low number does not
indicate low complexity).
Also j has its address taken AND you use this variable in several
different contexts (replacement for rid, later as loop counter). This
makes it impossible[0] for the compiler to do many useful optimisations
like keeping the value of the variable in a register!
It's good to know that doing so may costs performance in general,
as this is an bus attach function performance totally isn't an
issue here though.
I'm equally concerned with code clarity. A reader has to figure out the
def-use-chains when he tries to understand the code. Distinct variables
can make this task easier (ideally every variable, except for loop
counters, is const so there is only one point where it is assigned).
Also re-using address-taken variables in contexts where the address is
not necessary usually results in larger machine code.
It's interesting to note that here the needs for a compiler to be able
to produce "good" code and clarity of code for a human reader nicely
correlate.
More local variables cost nothing - especially no stack space, if this
is your concern - on any somewhat modern compiler. Use as many of them
as you want for code clarity. Do not reuse variables, whose address is
taken, because often it is impossible for the compiler to do any
optimisations.
My motivation for using fewer local variables is that style(9)
mandates to not define variables in nested scope and to sort
them, so reusing them reduces code churn in the long term.
IIRC you said elsewhere that you're unhappy with at least the
former rule but you should work on getting style(9) then.
You are right, I disagree with quite some rules in style(9) and this
particular rule I disagree with the most. But looking at the history,
making any suggestions here to change it will only result in the biggest
bikeshed ever built. Hm, seeing here that these rules encourage people
to actively reduce code clarity, I'll contemplate about making a suggestion.
Christoph
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
