Stanislav Sedov schrieb:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Fri, 01 May 2009 14:02:50 +0200
Christoph Mallon <christoph.mal...@gmx.de> mentioned:
[Don't parenthesize return values]
Removed, because it does not improve maintainability in any way.
This change could be made and tested mechanically. But there is
no justification for making it - stating that the existing rule
is no better than the proposed rule is no reason to change.
Just remove the rule. There's no need to add more redundant parentheses.
Again: There is no need to change all existing code at once, but the
rule is removed so that not anymore redundant parentheses are added.
If only the rule gets removed this will lead to inconsistent code. Currently
it is much easier to experienced leader to notice return statements with
parenthesis around the value than without. Recall that people's eyes are
build in way that they recognized entire expressions and not letter
combinations.
I don't buy this for a simple reason: Parentheses are in many statements
(if, while, for). The only thing which distinguishs a return statement
from others is the word "return".
+.Sh LOCAL VARIABLES
Last, but definitely not least, I added this paragraph about the use of
local variables. This is to clarify, how today's compilers handle
unaliased local variables.
Every version of gcc that FreeBSD has ever used would do this for
primitive types when optimisation was enabled. This approach can
become expensive in stack (and this is an issue for kernel code) when
using non-primitive types or when optimisation is not enabled (though
I'm not sure if this is supported). Assuming that gcc (and icc and
clang) behaves as stated in all supported optimisation modes, this
change would appear to be quite safe to make.
Most local variables are scalars (pointers, ints, ...). A struct or
array as local variable is rare and then usually used in one context. So
IMO this is fine. Even considereing the extremly rare case of multiple
non-scalar declarations in a function, then a compiler can coalesce them
if their life ranges are disjoint. It is easier to do so for a compiler
to do so, if they are declared with smaller life ranges, example:
struct Foo { int x[16]; };
struct Bar { int* y[16]; };
void f(struct Foo*);
void g(struct Bar*);
void e(int x)
{
struct Foo a;
struct Bar b;
if (x) {
f(&a);
} else {
g(&b);
}
}
Stack usage:
subl $140, %esp
moving the declarations into the branches:
subl $76, %esp
So, apart from improved readability, it also can lead to better code.
But I consider the latter way less important the benefits observed by a
reader of the code.
I particulary like this change.
Why?
Aliasing behavior is stritcly described in
ISO C99 standard, so there's a good point to enforcing strict-aliasing clear
code in our kernel.
If you like this addition because of this reason, I have to disappoint
you: This addition has absolutly *nothing* to do with strict-aliasing.
On the other hand the big work should be done on clearing
the existing code before any rule on this can be enforced.
This addition is about improving readability for humans, because it
simplifies the def-use-chains, and as a side effect sometimes leads to
better generated code. It is not sensible to check millions of lines of
code whether a variables are used in different contexts within a
function before this can added.
Anyway, this is moot, because this thread was dead because every
suggested improvement was rejected - especially this last improvement
was rejected by the guy who requested it in the first place.
Christoph
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
