Re: [PATCH] Makefile: Yes. Finally remove '-Wdeclaration-after-statement'

From: Ingo Molnar <mingo@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Pavel Machek <pavel@ucw.cz>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Michael Witten <mfwitten@gmail.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Sam Ravnborg <sam@ravnborg.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	John Levon <john.levon@joyent.com>,
	John Levon <levon@movementarian.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] Makefile: Yes. Finally remove '-Wdeclaration-after-statement'
Date: Tue, 18 Aug 2020 07:17:17 +0200	[thread overview]
Message-ID: <20200818051717.GA3134537@gmail.com> (raw)
In-Reply-To: <CAHk-=wh7Rn=8mVi_KWV71ZaQ2HrCz240DbjEJMDdARTwB3CYvA@mail.gmail.com>

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, Aug 17, 2020 at 3:09 PM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > Submitter believes "wild variable placement" can help with
> > #ifdefs.. and that may be actually good tradeoff.
> 
> I agree that it can help in some cases.
> 
> But it can also make it really hard to find the variable declarations
> in other cases. I've seen a lot of code that ends up actively
> declaring the variable close to where it's used (because people find
> that to be locally more legible) and then it just means that people
> who arent' familiar with the code have a much harder time finding it.
> 
> I'd instead try to discourage people from using #ifdef's inside code.

I'm a big fan of -Wdeclaration-after-statement and I think C++ style 
mixed variables/statements code has several disadvantages:

- One advantage of -Wdeclaration-after-statement is that it can detect 
  mismerges that can happen with the 'patch' tool when it applies a 
  patch with fuzz.

- Also, enforcing -Wdeclaration-after-statement means we have the nice 
  symmetry that local variable declarations are always at the 
  beginning of curly brace blocks, which includes function 
  definitions. This IMO is a very helpful visual clue that allows the 
  quick reading of kernel code.

- A third advantage is that the grouping of local variables at the 
  beginning of curly brace blocks encourages smaller, better 
  structured functions: a large function would look automatically ugly 
  due to the many local variables crammed at the beginning of it.

So the gentle code structure message is: you can declare new local 
variables in a loop construct or branch, at the cost of losing one 
level of indentation. If it gets too deep, you are encouraged to split 
your logic up better with helper functions. The kind of run-on 
mega-functions that C++ style mixed variables often allow looks 
*automatically* uglier under -Wdeclaration-after-statement and quickly 
breaks simple kernel style rules such as col80 or indentation level 
depth or the too high visual complexity of variable definition lines.

Basically the removal of -Wdeclaration-after-statement removes a 
helpful symmetry & allows the addition of random noise to our code 
base, with very little benefits offered. I'd be sad to see it go.

Thanks,

	Ingo