| Message ID | c6fda26e8d134264b04fadc3386d6c32@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile: Yes. Finally remove '-Wdeclaration-after-statement' | expand |
On Sun, Aug 16, 2020 at 04:35:00PM -0000, Michael Witten wrote: > This is not just a matter of style; this is a matter of semantics, > especially with regard to: > > * const Correctness. > A const-declared variable must be initialized when defined. > > * Conditional Compilation. > When there is complex interaction between compile-time > configuration options, it's essential to be able to > make declarations where needed; otherwise unnecessary > gymnastics are required to silence the compiler. > > Gentleman... Just let people say exactly what they mean to say. > > Requiring every declaration to be at the top of a block is an > antiquated, vestigial naivete from a time when C was just a > glorified abstraction over conventional patterns in assembly > programming. > > We are not just programming anymore. We are now encoding our > very thoughts, and thus we need this expressiveness in order > to capture those thoughts with sufficient clarity. You obviously need every bit of help in that task, judging by the amount of clarity (or thoughts, for that matter) visible in the spew above... NAK. And as for letting people say exactly what they mean to say... I am tempted to take you on that invitation, but that would be cruel to gregkh - he would have to reply to inevitable screeds about CoC-violating postings on l-k.
On Sun, 2020-08-16 at 16:35 +0000, Michael Witten wrote: > Requiring every declaration to be at the top of a block is an > antiquated, vestigial naivete from a time when C was just a > glorified abstraction over conventional patterns in assembly > programming. I rather prefer block declarations instead of sprinkling declarations around with code. > We are not just programming anymore. We are now encoding our > very thoughts, and thus we need this expressiveness in order > to capture those thoughts with sufficient clarity. So how does this removal have anything to do with expressiveness and clarity?
Joe Perches (Sun, 16 Aug 2020 10:56:53 -0700): > I rather prefer block declarations instead of > sprinkling declarations around with code. Hey, we all have our guilty pleasures. Fortunately, even with this patch, you'd still be able to indulge in your preferred style, or even enforce it among contributors to the code that you maintain. However, the following statement should hold: If merged code is correct (portable, safe, etc.), then the kernel must build without any warning about that merged code. Sometimes, code is clearest (or indeed safest) when it is written with a variable definition that occurs at a point well within the body of statements. Authors need to have the option to write such code; otherwise, style ceases to be means of clarity, and instead becomes a laborious end unto itself. Matters of style should probably not be enforced by the build infrastructure; style is a matter for the maintainer to enforce: * Perhaps there could be a new build-time switch. By default, the warning can be off for a normal build; a maintainer can flip the switch to turn it on locally, and thereby check whether a patch declares variables unnecessarily hither and thither, as determined by the maintainer's taste. * Perhaps `scripts/checkpatch.pl' could be taught about this issue. Though probably easier said than done, the script could parse every modified block, and warn about declarations after statements (but maybe ignore the declarations that introduce const variables). * Perhaps there is already linting infrastructure that could be put to such use. This way, good code can compile cleanly, and style can just be an ongoing topic of discussion among contributors. Sincerely, Michael Witten
On Mon, 2020-08-17 at 03:37 +0000, Michael Witten wrote: > Matters of style should probably not be enforced by the build > infrastructure; style is a matter for the maintainer to enforce: I rather doubt style advice should be taken from someone who right justifies fixed pitch block text. cheers, Joe
Joe Perches (Sun, 16 Aug 2020 10:56:53 -0700): > On Mon, 2020-08-17 at 03:37 +0000, Michael Witten wrote: >> Matters of style should probably not be enforced by >> the build infrastructure; style is a matter for the >> maintainer to enforce: > > I rather doubt style advice should be taken from someone > who right justifies fixed pitch block text. > > cheers, Joe Clearly, I never offered style advice. Indeed, the very first sentence I wrote in this thread was: > This is not just a matter of style; this is a matter of > semantics[.] Sincerely, Michael Witten As an aside, the venerable 'man' program justifies fixed pitch text, at least by default.
On Sun 2020-08-16 21:19:23, Joe Perches wrote: > On Mon, 2020-08-17 at 03:37 +0000, Michael Witten wrote: > > Matters of style should probably not be enforced by the build > > infrastructure; style is a matter for the maintainer to enforce: > > I rather doubt style advice should be taken from someone who > right justifies fixed pitch block text. Ad hominem? You may be right but this is not fair discussion. Pavel
Hi! > > This is not just a matter of style; this is a matter of semantics, > > especially with regard to: > > > > * const Correctness. > > A const-declared variable must be initialized when defined. > > > > * Conditional Compilation. > > When there is complex interaction between compile-time > > configuration options, it's essential to be able to > > make declarations where needed; otherwise unnecessary > > gymnastics are required to silence the compiler. > > > > Gentleman... Just let people say exactly what they mean to say. .. > You obviously need every bit of help in that task, judging by the amount > of clarity (or thoughts, for that matter) visible in the spew above... > > NAK. And as for letting people say exactly what they mean to say... > I am tempted to take you on that invitation, but that would be cruel > to gregkh - he would have to reply to inevitable screeds about > CoC-violating postings on l-k. We should really get rid of CoC, because I'd really like to see you _not_ resist that temptation. But... he's right. With rust-like programming style with widespread consts, this warning has to go. And it is causing additional/ugly #ifdefs in some cases. Maybe author can show examples in kernel .c where disabling the warning would lead to nicer code. I believe we should give it a try. Pavel
Pavel Machek <pavel@ucw.cz> writes: > Hi! > >> > This is not just a matter of style; this is a matter of semantics, >> > especially with regard to: >> > >> > * const Correctness. >> > A const-declared variable must be initialized when defined. >> > >> > * Conditional Compilation. >> > When there is complex interaction between compile-time >> > configuration options, it's essential to be able to >> > make declarations where needed; otherwise unnecessary >> > gymnastics are required to silence the compiler. >> > >> > Gentleman... Just let people say exactly what they mean to say. > .. > >> You obviously need every bit of help in that task, judging by the amount >> of clarity (or thoughts, for that matter) visible in the spew above... >> >> NAK. And as for letting people say exactly what they mean to say... >> I am tempted to take you on that invitation, but that would be cruel >> to gregkh - he would have to reply to inevitable screeds about >> CoC-violating postings on l-k. > > We should really get rid of CoC, because I'd really like to see you > _not_ resist that temptation. > > But... he's right. > > With rust-like programming style with widespread consts, this warning > has to go. And it is causing additional/ugly #ifdefs in some cases. > > Maybe author can show examples in kernel .c where disabling the > warning would lead to nicer code. I believe we should give it a try. This change came in with 535231e8252e ("[PATCH] add -Wdeclaration-after-statement"). AKA https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=535231e8252eea14abf4f14d28f6c1c03f5e0f02 Does anyone remember why we added this warning? I had always thought it's purpose was to ensure we stayed within our chosen dialect of C. The actual commit says that it was in reaction to gcc miscompiling proc. Which is a far more serious thing. With all of the our bumping of our gcc version lately perhaps it is safe to remove this warning. Eric
On Mon, Aug 17, 2020 at 2:15 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > > Does anyone remember why we added this warning? I had always thought > it's purpose was to ensure we stayed within our chosen dialect of C. As far as I'm concerned, that's the primary motivation. I'm not seeing why we'd suddenly allow the "put variable declarations anywhere" when we've been able to keep from doing it until now. We're still building primarily good old K&R ANSI C, just with extensions. Wild variable placement doesn't seem like a useful extension. (Other variable placement improvements are: block-scope variable declarations inside the "for()" statement is very syntactically useful, for example. THAT would be useful if we can finally enable it without gcc going all wonky on us) Linus
On Mon 2020-08-17 14:29:37, Linus Torvalds wrote: > On Mon, Aug 17, 2020 at 2:15 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > > > > Does anyone remember why we added this warning? I had always thought > > it's purpose was to ensure we stayed within our chosen dialect of C. > > As far as I'm concerned, that's the primary motivation. > > I'm not seeing why we'd suddenly allow the "put variable declarations > anywhere" when we've been able to keep from doing it until now. > > We're still building primarily good old K&R ANSI C, just with > extensions. Wild variable placement doesn't seem like a useful > extension. I certainly hope we are not going back to good old K&R C :-). Submitter believes "wild variable placement" can help with #ifdefs.. and that may be actually good tradeoff. Best regards, Pavel
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. Linus
* 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
> I'm a big fan of -Wdeclaration-after-statement and I think C++ style > mixed variables/statements code has several disadvantages: Agreed. Personally I think declarations should either be either right at the top of a function or in a very small code block. Otherwise they are annoying to find. You also get very hard to spot bugs unless -Wshadow is enabled (I can't remember if the linux kernel has it enabled). C++ (sort of) has to allow definitions in the middle of code blocks because it doesn't allow uninitialised variables - so definitions are best delayed until the copy-constructor can be used. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
I think there's an important distinction to make between
the following 2 kinds of code:
* The curated code people just want to build.
* The new patches that maintainers are reviewing.
Certainly, maintainers should have a wide range of tools
at their disposal to probe the quality of a patch; then,
after bending rules of style to taste, the maintainers
declare the merged code to be curated, after which that
merged code need not be probed so invasively every time
it is built.
To this end, I propose the following new patch, which
introduces some build-time switches that will help
people make this distinction.
As you know, you can save this email to some path:
/path/to/email.eml
Then apply and review the patch as follows:
$ cd /path/to/linux/repo
$ git reset --hard HEAD
$ git checkout --detach origin/master
$ git am --scissors /path/to/email.eml
$ git log -1 -p
At this point, the patch is intended as a[n] RFC;
I haven't tested it, and just wanted to get the
idea out there.
Sincerely,
Michael Witten
---8<------8<------8<------8<------8<------8<------8<------8<---
Subject: [PATCH] kbuild: Introduce "Warnings for maintainers"
This commit introduces the following new Kconfig options:
CONFIG_MAINTAINERS_WARNINGS
|
+-> CONFIG_WARN_DECLARATION_AFTER_STATEMENT
|
+-> CONFIG_WARN_MAYBE_UNINITIALIZED
These produce the following menu items:
-> Kernel hacking
-> Compile-time checks and compiler options
-> Warnings for maintainers [NEW]
* Warn about mixing variable definitions and statements [NEW]
* Warn about use of potentially uninitialized variables [NEW]
In short:
* CONFIG_MAINTAINERS_WARNINGS
is the umbrella control.
* CONFIG_WARN_DECLARATION_AFTER_STATEMENT
determines whether "-Wdeclaration-after-statement" is used.
* CONFIG_WARN_MAYBE_UNINITIALIZED
determines whether "-Wmaybe-uninitialized" is used.
Currently, the default is "y" for each, but it is expected that
eventually the default will be "n" for CONFIG_MAINTAINERS_WARNINGS.
Running "make" with "W=2" should turn both warnings on, unless
they are thwarted by some other Kbuild logic.
Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
arch/arm64/kernel/vdso32/Makefile | 7 +++-
lib/Kconfig.debug | 64 +++++++++++++++++++++++++++++++
scripts/Makefile.extrawarn | 1 +
tools/power/acpi/Makefile.config | 7 +++-
tools/power/cpupower/Makefile | 7 +++-
tools/scripts/Makefile.include | 9 ++++-
6 files changed, 91 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 5139a5f19256..8cc997b9ccef 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -88,7 +88,12 @@ VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-std=gnu89
VDSO_CFLAGS += -O2
# Some useful compiler-dependent flags from top-level Makefile
-VDSO_CFLAGS += $(call cc32-option,-Wdeclaration-after-statement,)
+ifeq($(CONFIG_WARN_DECLARATION_AFTER_STATEMENT), y)
+ VDSO_CFLAGS += $(call cc32-option,-Wdeclaration-after-statement)
+endif
+ifeq($(CONFIG_WARN_MAYBE_UNINITIALIZED), y)
+ VDSO_CFLAGS += $(call cc32-option,-Wmaybe-uninitialized)
+endif
VDSO_CFLAGS += $(call cc32-option,-Wno-pointer-sign)
VDSO_CFLAGS += $(call cc32-option,-fno-strict-overflow)
VDSO_CFLAGS += $(call cc32-option,-Werror=strict-prototypes)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index e068c3c7189a..4e3a87ce77c8 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -308,6 +308,70 @@ config FRAME_WARN
Setting this too low will cause a lot of warnings.
Setting it to 0 disables the warning.
+config MAINTAINERS_WARNINGS
+ bool "Warnings for maintainers"
+ default y
+ help
+ These warnings may be useful to maintainers and contributors
+ when patches are being prepared and reviewed; they may yield
+ false positives, and are therefore intended to be turned off
+ for a normal build.
+
+config WARN_DECLARATION_AFTER_STATEMENT
+ bool "Warn about mixing variable definitions and statements"
+ depends on MAINTAINERS_WARNINGS
+ default y
+ help
+ Turn on the following gcc command-line option:
+
+ -Wdeclaration-after-statement
+
+ Traditionally, C code has been written such that variables
+ are defined at the top of a block (e.g., at the top of a
+ function body); C99 removed this requirement, allowing the
+ mixing of variable definitions and statements, but the
+ tradition has remained a convention of the kernel's coding
+ style.
+
+ When reviewing patches, a maintainer may wish to turn this
+ warning on, in order to catch variable definitions that have
+ been placed unnecessarily among the statements, and thereby
+ enforce the dominant coding style.
+
+ Of course, sometimes it is useful to define a variable among
+ the statements, especially in the following cases:
+
+ * Declaring a const variable.
+ * Dealing with conditional compilation.
+
+ Therefore, this warning is intended to be turned off for a
+ normal build, where all of the code has already been merged
+ succesfully into the repository.
+
+config WARN_MAYBE_UNINITIALIZED
+ bool "Warn about use of potentially uninitialized variables"
+ depends on MAINTAINERS_WARNINGS
+ default y
+ help
+ Turn on the following gcc command-line option:
+
+ -Wmaybe-uninitialized
+
+ Serious bugs can result from using a variable whose value
+ has never been explicitly initialized. When this warning
+ is turned on, the compiler will use heuristic analyses of
+ the code to determine whether a variable has been properly
+ initialized before it is ever used.
+
+ However, the heuristic nature of the analyses has often
+ caused many false positive warnings that programmers find
+ irritating; sometimes, bugs are introduced in the process
+ of simply trying to silence the warning.
+
+ Therefore, this warning is intended to be turned off for a
+ normal build, where all of the code has already been merged
+ succesfully into the repository.
+
config STRIP_ASM_SYMS
bool "Strip assembler-generated symbols during link"
default n
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 62c275685b75..afadbdcc7c53 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -68,6 +68,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)
KBUILD_CFLAGS += -Wmissing-field-initializers
KBUILD_CFLAGS += -Wsign-compare
KBUILD_CFLAGS += -Wtype-limits
+KBUILD_CFLAGS += -Wdeclaration-after-statement
KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
diff --git a/tools/power/acpi/Makefile.config b/tools/power/acpi/Makefile.config
index 54a2857c2510..6fe0b34eddd7 100644
--- a/tools/power/acpi/Makefile.config
+++ b/tools/power/acpi/Makefile.config
@@ -64,7 +64,12 @@ OPTIMIZATION := $(call cc-supports,-Os,-O2)
WARNINGS := -Wall
WARNINGS += $(call cc-supports,-Wstrict-prototypes)
-WARNINGS += $(call cc-supports,-Wdeclaration-after-statement)
+ifeq($(CONFIG_WARN_DECLARATION_AFTER_STATEMENT), y)
+ WARNINGS += $(call cc-supports,-Wdeclaration-after-statement)
+endif
+ifeq($(CONFIG_WARN_MAYBE_UNINITIALIZED), y)
+ WARNINGS += $(call cc-supports,-Wmaybe-uninitialized)
+endif
KERNEL_INCLUDE := $(OUTPUT)include
ACPICA_INCLUDE := $(srctree)/../../../drivers/acpi/acpica
diff --git a/tools/power/cpupower/Makefile b/tools/power/cpupower/Makefile
index c8622497ef23..8d26c2003d7d 100644
--- a/tools/power/cpupower/Makefile
+++ b/tools/power/cpupower/Makefile
@@ -118,7 +118,12 @@ 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)
+ifeq($(CONFIG_WARN_DECLARATION_AFTER_STATEMENT), y)
+ WARNINGS += $(call cc-supports,-Wdeclaration-after-statement)
+endif
+ifeq($(CONFIG_WARN_MAYBE_UNINITIALIZED), y)
+ WARNINGS += $(call cc-supports,-Wmaybe-uninitialized)
+endif
WARNINGS += -Wshadow
override CFLAGS += -DVERSION=\"$(VERSION)\" -DPACKAGE=\"$(PACKAGE)\" \
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index a7974638561c..a9acd52b5e84 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -17,11 +17,18 @@ OUTDIR := $(shell cd $(OUTPUT) && pwd)
$(if $(OUTDIR),, $(error output directory "$(OUTPUT)" does not exist))
endif
+#
+# Maintainers' Warnings
+#
+MAINTAINERS_WARNINGS-y :=
+MAINTAINERS_WARNINGS-$(CONFIG_WARN_DECLARATION_AFTER_STATEMENT) += -Wdeclaration-after-statement
+MAINTAINERS_WARNINGS-$(CONFIG_WARN_MAYBE_UNINITIALIZED) += -Wmaybe-uninitialized
+
#
# Include saner warnings here, which can catch bugs:
#
EXTRA_WARNINGS := -Wbad-function-cast
-EXTRA_WARNINGS += -Wdeclaration-after-statement
+EXTRA_WARNINGS += $(MAINTAINERS_WARNINGS-y)
EXTRA_WARNINGS += -Wformat-security
EXTRA_WARNINGS += -Wformat-y2k
EXTRA_WARNINGS += -Winit-self
The quick RFC patch I just proposed in the parent email is broken in its implementation. I will submit an updated version soon. Michael Witten (Tue, 18 Aug 2020 22:05:00 -0000): > I think there's an important distinction to make between > the following 2 kinds of code: > > * The curated code people just want to build. > * The new patches that maintainers are reviewing. > > Certainly, maintainers should have a wide range of tools > at their disposal to probe the quality of a patch; then, > after bending rules of style to taste, the maintainers > declare the merged code to be curated, after which that > merged code need not be probed so invasively every time > it is built. > > To this end, I propose the following new patch, which > introduces some build-time switches that will help > people make this distinction. > > As you know, you can save this email to some path: > > /path/to/email.eml > > Then apply and review the patch as follows: > > $ cd /path/to/linux/repo > $ git reset --hard HEAD > $ git checkout --detach origin/master > $ git am --scissors /path/to/email.eml > $ git log -1 -p > > At this point, the patch is intended as a[n] RFC; > I haven't tested it, and just wanted to get the > idea out there. > > Sincerely, > Michael Witten > > ---8<------8<------8<------8<------8<------8<------8<------8<--- > Subject: [PATCH] kbuild: Introduce "Warnings for maintainers" > This commit introduces the following new Kconfig options: > > CONFIG_MAINTAINERS_WARNINGS > | > +-> CONFIG_WARN_DECLARATION_AFTER_STATEMENT > | > +-> CONFIG_WARN_MAYBE_UNINITIALIZED > > These produce the following menu items: > > -> Kernel hacking > -> Compile-time checks and compiler options > -> Warnings for maintainers [NEW] > * Warn about mixing variable definitions and statements [NEW] > * Warn about use of potentially uninitialized variables [NEW] > > In short: > > * CONFIG_MAINTAINERS_WARNINGS > is the umbrella control. > > * CONFIG_WARN_DECLARATION_AFTER_STATEMENT > determines whether "-Wdeclaration-after-statement" is used. > > * CONFIG_WARN_MAYBE_UNINITIALIZED > determines whether "-Wmaybe-uninitialized" is used. > > Currently, the default is "y" for each, but it is expected that > eventually the default will be "n" for CONFIG_MAINTAINERS_WARNINGS. > > Running "make" with "W=2" should turn both warnings on, unless > they are thwarted by some other Kbuild logic. > > Signed-off-by: Michael Witten <mfwitten@gmail.com> > > [... PATCH ...]
diff --git a/Makefile b/Makefile index 254e80a96b23..88ad4e430834 100644 --- a/Makefile +++ b/Makefile @@ -900,9 +900,6 @@ endif # arch Makefile may override CC so keep this after arch Makefile is included NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include) -# 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 diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 5139a5f19256..672008c735fb 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -88,7 +88,6 @@ VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -std=gnu89 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 += $(call cc32-option,-fno-strict-overflow) VDSO_CFLAGS += $(call cc32-option,-Werror=strict-prototypes) diff --git a/tools/power/acpi/Makefile.config b/tools/power/acpi/Makefile.config index 54a2857c2510..70ac50cfd7eb 100644 --- a/tools/power/acpi/Makefile.config +++ b/tools/power/acpi/Makefile.config @@ -64,7 +64,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 diff --git a/tools/power/cpupower/Makefile b/tools/power/cpupower/Makefile index c8622497ef23..d389179d4484 100644 --- 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)\" \ diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include index a7974638561c..83c2a5a1ed15 100644 --- 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
This is not just a matter of style; this is a matter of semantics, especially with regard to: * const Correctness. A const-declared variable must be initialized when defined. * Conditional Compilation. When there is complex interaction between compile-time configuration options, it's essential to be able to make declarations where needed; otherwise unnecessary gymnastics are required to silence the compiler. Gentleman... Just let people say exactly what they mean to say. Requiring every declaration to be at the top of a block is an antiquated, vestigial naivete from a time when C was just a glorified abstraction over conventional patterns in assembly programming. We are not just programming anymore. We are now encoding our very thoughts, and thus we need this expressiveness in order to capture those thoughts with sufficient clarity. Therefore, this commit removes: -Wdeclaration-after-statement Signed-off-by: Michael Witten <mfwitten@gmail.com> --- Makefile | 3 --- arch/arm64/kernel/vdso32/Makefile | 1 - tools/power/acpi/Makefile.config | 1 - tools/power/cpupower/Makefile | 1 - tools/scripts/Makefile.include | 1 - 5 files changed, 7 deletions(-)