[PATCHv2] Rename -Wall to Wsparse-all, so it doesn't get turned on unintentionally

[PATCHv2] Rename -Wall to Wsparse-all, so it doesn't get turned on unintentionally

[Josh Triplett]

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.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

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

diff --git a/cgcc b/cgcc
index 995cc05..8005c3c 100755
--- a/cgcc
+++ b/cgcc
@@ -51,7 +51,7 @@ while (@ARGV) {
 
     my $this_arg = ' ' . &quote_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 {
diff --git a/lib.c b/lib.c
index 622b547..963be08 100644
--- a/lib.c
+++ b/lib.c
@@ -407,7 +407,7 @@ static char **handle_onoff_switch(char *arg, char **next, const struct warning w
 	char *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;
diff --git a/sparse.1 b/sparse.1
index d7fe444..abc75a2 100644
--- a/sparse.1
+++ b/sparse.1
@@ -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.
 
diff --git a/validation/specifiers1.c b/validation/specifiers1.c
index 86db45d..1a4e1d5 100644
--- a/validation/specifiers1.c
+++ b/validation/specifiers1.c
@@ -97,5 +97,5 @@ TEST2(double, long)
 }
 /*
  * check-name: valid specifier combinations
- * check-command: sparse -Wall $file
+ * check-command: sparse $file
  */
-- 
1.6.4.3

Re: [PATCHv2] Rename -Wall to Wsparse-all, so it doesn't get turned on unintentionally

[Christopher Li]

On Sun, Oct 11, 2009 at 3:47 PM, Josh Triplett <josh@joshtriplett.org> wrote:
> 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

Re: [PATCHv2] Rename -Wall to Wsparse-all, so it doesn't get turned on unintentionally

[Josh Triplett]

On Sun, Oct 11, 2009 at 10:46:25PM -0700, Christopher Li wrote:
> On Sun, Oct 11, 2009 at 3:47 PM, Josh Triplett <josh@joshtriplett.org> wrote:
> > 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