[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH] xen: Remove -Wdeclaration-after-statement
On 09/08/24 21:25, Stefano Stabellini wrote:
Adding Roberto
Does MISRA have a view on this? I seem to remember this is discouraged?
As far as I know, there is nothing in MISRA C against or in favor of
mixing declaration with statements. The only (slightly) relevant
guideline is advisory Rule 8.9 (An object should be declared at block
scope if its identifier only appears in a single function), which advises
to declare objects at function scope when this is possible.
The rationale of the same rule says, among other things:
Within a function, whether objects are declared at the outermost
or innermost block is largely a matter of style.
On the other hand, MISRA C recognizes that reduction of scope
is, all other things being equal, to be preferred, but there
are no guidelines similar to -Wdeclaration-after-statement.
Rather, the point is "declare it at block scope, if you can;
otherwise declare it at file scope, if you can; otherwise,
declare it at global scope, if you must."
Kind regards,
Roberto
On Fri, 9 Aug 2024, Alejandro Vallejo wrote:
This warning only makes sense when developing using a compiler with C99
support on a codebase meant to be built with C89 compilers too, and
that's no longer the case (nor should it be, as it's been 25 years since
C99 came out already).
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
---
Yes, I'm opening this can of worms. I'd like to hear others people's
thoughts on this and whether this is something MISRA has views on. If
there's an ulterior non-obvious reason besides stylistic preference I
think it should be documented somewhere, but I haven't seen such an
explanation.
IMO, the presence of this warning causes several undesirable effects:
1. Small functions are hampered by the preclusion of check+declare
patterns that improve readability via concision. e.g: Consider a
silly example like:
/* with warning */ /* without warning */
void foo(uint8_t *p) void foo(uint8_t *p)
{ {
uint8_t tmp1; if ( !p )
uint16_t tmp2; return;
uint32_t tmp3;
uint8_t tmp1 = OFFSET1 + *p;
if ( !p ) uint16_t tmp2 = OFFSET2 + *p;
return; uint32_t tmp3 = OFFSET3 + *p;
tmp1 = OFFSET1 + *p; /* Lots of uses of `tmpX` */
tmp2 = OFFSET2 + *p; }
tmp2 = OFFSET2 + *p;
/* Lots of uses of tmpX */
}
2. Promotes scope-creep. On small functions it doesn't matter much,
but on bigger ones to prevent declaring halfway through the body
needlessly increases variable scope to the full scope in which they
are defined rather than the subscope of point-of-declaration to
end-of-current-scope. In cases in which they can be trivially
defined at that point, it also means they can be trivially misused
before they are meant to. i.e: On the example in (1) assume the
conditional in "with warning" is actually a large switch statement.
3. It facilitates a disconnect between time-of-declaration and
time-of-use that lead very easily to "use-before-init" bugs.
While a modern compiler can alleviate the most egregious cases of
this, there's cases it simply cannot cover. A conditional
initialization on anything with external linkage combined with a
conditional use on something else with external linkage will squash
the warning of using an uninitialised variable. Things are worse
where the variable in question is preinitialised to something
credible (e.g: a pointer to NULL), as then that can be misused
between its declaration and its original point of intended use.
So... thoughts? yay or nay?
In my opinion, there are some instances where mixing declarations and
statements would enhance the code, but these are uncommon. Given the
choice between:
1) declarations always first
2) declarations always mixed with statements
I would choose 1).
One could say that mixing declarations with statements should be done
only when appropriate, but the challenge lies in the subjective nature
of "when appropriate." Different individuals have varying
interpretations of this, which could lead to unnecessary bikeshedding.
For these reasons, I do not support mixing declarations and statements.
|
