kbuild: drop -Wdeclaration-after-statement

Message ID Y1w031iI6Ld29IVT@p183
State New
Headers
Series kbuild: drop -Wdeclaration-after-statement |

Commit Message

Alexey Dobriyan Oct. 28, 2022, 8 p.m. UTC
  Putting declarations in the beginning of the block is an afterfact from
single pass compiler era. Compiler would parse all declarations, layout
stack frame and proceed to generate code.

In C initialisers can be arbitrarily complex so there is no fundamental
distinction between initialiser and regular code.
-Wno-declaration-after-statement creates such distinction which is
entirely artificial.

This will save LOC in the long run because people would write code like
this:

	int a = f();

This will make one rare class of bugs even more rare:

	int a;
	...
	f(&a);	// bug, typo, should be f(&x)
	...
	a = g();

If declarations are allowed anywhere, the above would be written as

	f(&a);
	int a = g();

and it would not compile because "a" lives for less LOC window.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 Makefile                          |    6 +-----
 arch/arm64/kernel/vdso32/Makefile |    2 --
 tools/power/acpi/Makefile.config  |    1 -
 tools/power/cpupower/Makefile     |    1 -
 tools/scripts/Makefile.include    |    1 -
 5 files changed, 1 insertion(+), 10 deletions(-)
  

Comments

Linus Torvalds Oct. 28, 2022, 8:29 p.m. UTC | #1
On Fri, Oct 28, 2022 at 1:00 PM Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> Putting declarations in the beginning of the block is an afterfact from
> single pass compiler era. Compiler would parse all declarations, layout
> stack frame and proceed to generate code.

No, putting declarations at the beginning is still kernel syntax.

Don't declare variables in multiple places. It gets really confusing.
Put all declarations at the top of the block they are contained in.

IOW, -Wdeclaration-after-statement does exactly the right thing, and stays.

This is not about "old compilers", this is about coding rules.

                Linus
  
Alexey Dobriyan Oct. 28, 2022, 8:55 p.m. UTC | #2
On Fri, Oct 28, 2022 at 01:29:08PM -0700, Linus Torvalds wrote:
> On Fri, Oct 28, 2022 at 1:00 PM Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> > Putting declarations in the beginning of the block is an afterfact from
> > single pass compiler era. Compiler would parse all declarations, layout
> > stack frame and proceed to generate code.
> 
> No, putting declarations at the beginning is still kernel syntax.
> 
> Don't declare variables in multiple places. It gets really confusing.

It is not. Somehow millions of programmers manage to find their
variables just fine in C and other programming languages.

> Put all declarations at the top of the block they are contained in.

I tried it the other way after years of LK style. Universe didn't collapse.

> IOW, -Wdeclaration-after-statement does exactly the right thing, and stays.
> 
> This is not about "old compilers", this is about coding rules.
> 
>                 Linus
  
Linus Torvalds Oct. 28, 2022, 9:11 p.m. UTC | #3
On Fri, Oct 28, 2022 at 1:55 PM Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> It is not. Somehow millions of programmers manage to find their
> variables just fine in C and other programming languages.

That's fine. Go work with those projects.

The kernel also does lots of other things that people disagree with in
order to be a more cohesive body of work. Millions of programmers
manage with 2-space indentation and other horrors.

The kernel has its rules. Not having variable declarations in random
places is one of those rules. One of many.

Deal with it, and the fact that I won't apply your patch.

                Linus
  
David Laight Oct. 29, 2022, 11:47 a.m. UTC | #4
From: Alexey Dobriyan
> Sent: 28 October 2022 21:55
> 
> On Fri, Oct 28, 2022 at 01:29:08PM -0700, Linus Torvalds wrote:
> > On Fri, Oct 28, 2022 at 1:00 PM Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > >
> > > Putting declarations in the beginning of the block is an afterfact from
> > > single pass compiler era. Compiler would parse all declarations, layout
> > > stack frame and proceed to generate code.
> >
> > No, putting declarations at the beginning is still kernel syntax.
> >
> > Don't declare variables in multiple places. It gets really confusing.
> 
> It is not. Somehow millions of programmers manage to find their
> variables just fine in C and other programming languages.

Have you ever tried it when -Wshadow isn't enabled and variables
with the same name are redefined in the middle of blocks?

C++ has to allow it (and it is annoying to find definitions)
because the initialiser has to be called.
But you can't use a 'goto' to jump past a declaration.

> > Put all declarations at the top of the block they are contained in.

Or better, either at the top of the function or in a small block
(where the limited scope is absolutely obvious).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
  

Patch

--- a/Makefile
+++ b/Makefile
@@ -452,8 +452,7 @@  HOSTRUSTC = rustc
 HOSTPKG_CONFIG	= pkg-config
 
 KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
-			 -O2 -fomit-frame-pointer -std=gnu11 \
-			 -Wdeclaration-after-statement
+			 -O2 -fomit-frame-pointer -std=gnu11
 KBUILD_USERCFLAGS  := $(KBUILD_USERHOSTCFLAGS) $(USERCFLAGS)
 KBUILD_USERLDFLAGS := $(USERLDFLAGS)
 
@@ -1011,9 +1010,6 @@  endif
 # arch Makefile may override CC so keep this after arch Makefile is included
 NOSTDINC_FLAGS += -nostdinc
 
-# warn about C99 declaration after statement
-KBUILD_CFLAGS += -Wdeclaration-after-statement
-
 # Variable Length Arrays (VLAs) should not be used anywhere in the kernel
 KBUILD_CFLAGS += -Wvla
 
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -68,11 +68,9 @@  VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
                -fno-strict-aliasing -fno-common \
                -Werror-implicit-function-declaration \
                -Wno-format-security \
-               -Wdeclaration-after-statement \
                -std=gnu11
 VDSO_CFLAGS  += -O2
 # Some useful compiler-dependent flags from top-level Makefile
-VDSO_CFLAGS += $(call cc32-option,-Wdeclaration-after-statement,)
 VDSO_CFLAGS += $(call cc32-option,-Wno-pointer-sign)
 VDSO_CFLAGS += -fno-strict-overflow
 VDSO_CFLAGS += $(call cc32-option,-Werror=strict-prototypes)
--- a/tools/power/acpi/Makefile.config
+++ b/tools/power/acpi/Makefile.config
@@ -63,7 +63,6 @@  OPTIMIZATION := $(call cc-supports,-Os,-O2)
 
 WARNINGS := -Wall
 WARNINGS += $(call cc-supports,-Wstrict-prototypes)
-WARNINGS += $(call cc-supports,-Wdeclaration-after-statement)
 
 KERNEL_INCLUDE := $(OUTPUT)include
 ACPICA_INCLUDE := $(srctree)/../../../drivers/acpi/acpica
--- a/tools/power/cpupower/Makefile
+++ b/tools/power/cpupower/Makefile
@@ -118,7 +118,6 @@  OPTIMIZATION := $(call cc-supports,-Os,-O2)
 
 WARNINGS := -Wall -Wchar-subscripts -Wpointer-arith -Wsign-compare
 WARNINGS += $(call cc-supports,-Wno-pointer-sign)
-WARNINGS += $(call cc-supports,-Wdeclaration-after-statement)
 WARNINGS += -Wshadow
 
 override CFLAGS += -DVERSION=\"$(VERSION)\" -DPACKAGE=\"$(PACKAGE)\" \
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -21,7 +21,6 @@  endif
 # Include saner warnings here, which can catch bugs:
 #
 EXTRA_WARNINGS := -Wbad-function-cast
-EXTRA_WARNINGS += -Wdeclaration-after-statement
 EXTRA_WARNINGS += -Wformat-security
 EXTRA_WARNINGS += -Wformat-y2k
 EXTRA_WARNINGS += -Winit-self