Rename -Wall to Wsparse-all, so it doesn't get turned on unintentionally
PATCH v2 1/1 Rename -Wall to Wsparse-all, so it doesn't get turned on unintentionally
sparse's -Wall option turns on all sparse warnings, including those that many projects will not want; for instance, warnings that enforce particular stylistic choices, or behavior allowed by a standard but considered questionable or error-prone. Furthermore, using -Wall means accepting all future warnings sparse may start issuing, not just those intentionally turned on by default.
Other compilers like GCC also use -Wall, and interpret it to mean "turn on a sensible set of warnings". Since sparse exists to emit warnings, it already defaults to emitting a sensible set of warnings. Many projects pass the same options to both sparse and the C compiler, including warning options like -Wall; this results in turning on excessive amounts of sparse warnings.
cgcc already filtered out -Wall, but many projects invoke sparse directly rather than using cgcc. Remove that filter, now that -Wall does not change sparse's behavior.
Projects almost certainly don't want to use the new -Wsparse-all option; they should choose the specific set of warnings they want, or just go with sparse's defaults.
Also update cgcc to know about Wsparse-all and not pass it to GCC, and update a test case that unnecessarily used -Wall.
---
v2: Update cgcc and remove unnecessary (and now ignored) -Wall from a test case.
My -Wdesignated-init patch also needs to include an update for cgcc, and thus it will conflict with this one. I'll rebase the Wdesignated-init patch on this one, to avoid the conflict. In the future, perhaps sparse could have some kind of --print-warning-options option, and the Makefile could generate cgcc from cgcc.in and that, avoiding the need to update cgcc when adding a new Sparse option.
cgcc | 17 ++--------------- lib.c | 2 +- sparse.1 | 4 ++++ validation/specifiers1.c | 2 +- 4 files changed, 8 insertions(+), 17 deletions(-)
cgcc +2 -15
@@ -51,7 +51,7 @@ while (@ARGV) { my $this_arg = ' ' . "e_arg ($_); $cc .= $this_arg unless &check_only_option ($_); - $check .= $this_arg unless &cc_only_option ($_); + $check .= $this_arg; } if ($gendeps) {
@@ -88,25 +88,12 @@ exit 0; sub check_only_option { my ($arg) = @_; - return 1 if $arg =~ /^-W(no-?)?(default-bitfield-sign|one-bit-signed-bitfield|cast-truncate|bitwise|typesign|context|undef|ptr-subtraction-blows|cast-to-as|decl|transparent-union|address-space|enum-mismatch|do-while|old-initializer|non-pointer-null|paren-string|return-void)$/; + return 1 if $arg =~ /^-W(no-?)?(default-bitfield-sign|one-bit-signed-bitfield|cast-truncate|bitwise|typesign|context|undef|ptr-subtraction-blows|cast-to-as|decl|transparent-union|address-space|enum-mismatch|do-while|old-initializer|non-pointer-null|paren-string|return-void|sparse-all)$/; return 1 if $arg =~ /^-v(no-?)?(entry|dead)$/; return 0; } # ----------------------------------------------------------------------------- -# Check if an option is for "cc" only. - -sub cc_only_option { - my ($arg) = @_; - # -Wall turns on all Sparse warnings, including experimental and noisy - # ones. Don't include it just because a project wants to pass -Wall to cc. - # If you really want cgcc to run sparse with -Wall, use - # CHECK="sparse -Wall". - return 1 if $arg =~ /^-Wall$/; - return 0; -} - -# ----------------------------------------------------------------------------- # Simple arg-quoting function. Just adds backslashes when needed. sub quote_arg {
lib.c +1 -1
@@ -407,7 +407,7 @@ static char **handle_onoff_switch(char *arg, char **next, const struct warning wchar *p = arg +1 ;unsigned i; -if (!strcmp (p,"all" )) { +if (!strcmp (p,"sparse-all" )) {for (i =0 ; i < n; i++) {if (*warnings[i].flag !=WARNING_FORCE_OFF ) *warnings[i].flag =WARNING_ON ;
sparse.1 +4 -0
@@ -20,6 +20,10 @@ off those warnings, pass the negation of the associated warning option, . .SH WARNING OPTIONS .TP +.B \-Wsparse\-all +Turn on all sparse warnings, except for those explicitly disabled via +\fB\-Wno\-something\fR. +.TP .B \-Waddress\-space Warn about code which mixes pointers to different address spaces.
validation/specifiers1.c +1 -1
@@ -97,5 +97,5 @@ TEST2(double, long) }/* * check-name: valid specifier combinations -* check-command: sparse -Wall $file +* check-command: sparse $file */
Signature · 1 line
1.6.4.3
Quoted 2 lines · Josh Triplett
v2: Update cgcc and remove unnecessary (and now ignored) -Wall from a test case.
Applied. BTW, what do you think about moving the logic from cgcc into sparse directly?
Chris
Quoted 6 lines · Christopher Li
Quoted 2 lines · Josh Triplett
v2: Update cgcc and remove unnecessary (and now ignored) -Wall from a test case.
Applied. BTW, what do you think about moving the logic from cgcc into sparse directly?
Definitely a good idea.
The logic in cgcc seems to fall in two main categories:
architecture-specific code and code to launch the C compiler. The former should definitely live in sparse itself, because otherwise sparse doesn't have all the proper built-in definitions when launched directly; I'd like to *avoid* the GCC approach of only supporting one architecture (or architecture family) at a time, though, and instead let the caller specify any architecture sparse supports. As for launching the compiler, it would prove fairly straightforward for sparse to strip any sparse-specific warning options from its command line and then launch $CC; in fact, if sparse did so right before exiting, it could just call exec and avoid the need to fork.
- Josh Triplett