2022-10-28 20:11:10

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH] kbuild: drop -Wdeclaration-after-statement

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 <[email protected]>
---

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(-)

--- 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


2022-10-28 20:57:17

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop -Wdeclaration-after-statement

On Fri, Oct 28, 2022 at 1:00 PM Alexey Dobriyan <[email protected]> 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

2022-10-28 21:16:40

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop -Wdeclaration-after-statement

On Fri, Oct 28, 2022 at 01:29:08PM -0700, Linus Torvalds wrote:
> On Fri, Oct 28, 2022 at 1:00 PM Alexey Dobriyan <[email protected]> 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

2022-10-28 21:18:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] kbuild: drop -Wdeclaration-after-statement

On Fri, Oct 28, 2022 at 1:55 PM Alexey Dobriyan <[email protected]> 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

2022-10-29 11:53:58

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] kbuild: drop -Wdeclaration-after-statement

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 <[email protected]> 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)