Re: [RFC PATCH] xen: Remove -Wdeclaration-after-statement

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.08.2024 15:04, 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.

Right, these are the aspects that would improve. The potential downside is
that you no longer have a fixed place (set of places) where to look for
which variables are actually in scope. For people having worked with C89
(and not e.g. C++) for a very long time, mixing of declarations and
statements may be irritating. In fact, having used C++ quite a lot in the
(meanwhile distant) past, I have developed a mental C mode and a mental
C++ one - when in the former I expect declarations at the start of scopes,
while when in the latter I know to expect them everywhere.

All in all - I'm afraid I'm split on this.

Jan