On Thu, Aug 08, 2024 at 01:32:20PM +1000, Peter Hutterer wrote:
> On Sun, Jul 28, 2024 at 05:48:45PM -0700, Alan Coopersmith wrote:
> > Back when we first added a default set of compiler warning flags to our
> > xorg-macros.m4 for autoconf in 2008, we included
> > -Wdeclaration-after-statement
> > since not all the compilers at the time supported this C99 feature (most
> > notably some OpenBSD ports that used gcc 2.95, but also MSVC versions before
> > 2013).
> >
> > Since then we've widely adopted other C99 features such as struct
> > initializers
> > and declaring variables in for loops ("for (int i = 0; ....)"), but last
> > time we discussed this warning in 2013, decided to leave it in place as
> > a style choice, not a technical constraint - see the thread starting at:
> > https://lists.freedesktop.org/archives/xorg-devel/2013-September/037735.html
> >
> > But we didn't carry that flag over in our meson conversions - apparently
> > intentionally so in the case of the X server code base:
> > https://gitlab.freedesktop.org/xorg/xserver/-/commit/db465bae533f85e7f900deb96efecc831c9d550b
> > so people patching the X server don't get told that we don't like that
> > style,
> > and so now Enrico has submitted a number of merge requests that use it to
> > simplify some of our previous code, such as:
> > https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1601
> >
> > Do we want to keep insisting on this as part of our style, or have people
> > gotten used to it from other languages/projects now and are willing to
> > accept it in X.Org code?
> >
> > Is it time to re-apply
> > https://gitlab.freedesktop.org/xorg/util/macros/-/commit/689ea0ec5d8b7594ba2fa9e27b2458cea8a58724
> > to util-macros?
>
> I feel fairly strongly about removing that warning, i.e. allowing
> declarations after statements. There's a reason all modern languages
> allow this. It makes the code clearer and less buggy in many instances,
> esp. in regards to variables that don't need to exist until various
> checks have been performed. And it allows those variables to be
> initialized immediately, so one source of bugs less. Doubly so with
> __attribute__((cleanup)) but that's a conversation for another day...
>
> Having said that, sticking with the local code style still trumps
> anything, if you have 6 temp variables declared at the top mixed with 5
> declared later that's more confusing than if all were at the top.
>
But also try to avoid gratuitous churn. I'm opposed to commit to just
move variable declaration around "because we can".
If someone plans to do some substential changes in a function then it
is worth to adapt the declaration style to improve the readability of
that function.
OpenBSD/m88k still uses gcc 2.95 and I'm not sure if it supports
mixing declarations with code, but don't consider this a a blocker.
--
Matthieu Herrb
