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

[Alejandro Vallejo <alejandro.vallejo@cloud.com>]

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@cloud.com>
---
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?
---
 xen/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index 2e1a925c8417..288b7ac8bb2d 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -394,7 +394,7 @@ CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections
 
 CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wwrite-strings -Wno-pointer-arith
-CFLAGS += -Wdeclaration-after-statement -Wuninitialized
+CFLAGS += -Wuninitialized
 $(call cc-option-add,CFLAGS,CC,-Wvla)
 $(call cc-option-add,CFLAGS,CC,-Wflex-array-member-not-at-end)
 $(call cc-option-add,CFLAGS,CC,-Winit-self)
-- 
2.45.2