enumerate explicitly what is enabled by Wall

enumerate explicitly what is enabled by Wall

[Manuel López-Ibáñez]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

:ADDPATCH doc:

With the current text in invoke.texi, it is sometimes difficult to
tell which options are enabled by Wall. This would be easier if we
followed the approach for describing -OX, as this patch implements. I
have also reviewed all the flags actually enabled by Wall in the code.

OK to commit?

Cheers,

Manuel.

2007-09-23  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  * doc/invoke.texi: List the options enabled by Wall.

[-- Attachment #2: invoke-wall.diff --]
[-- Type: text/plain, Size: 3106 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 128662)
+++ gcc/doc/invoke.texi	(working copy)
@@ -2592,6 +2592,52 @@
 @opindex w
 Inhibit all warning messages.
 
+@item -Wfatal-errors
+@opindex Wfatal-errors
+This option causes the compiler to abort compilation on the first error
+occurred rather than trying to keep going and printing further error
+messages.
+
+@item -Wall
+@opindex Wall
+This enables all the warnings about constructions that some users
+consider questionable, and that are easy to avoid (or modify to
+prevent the warning), even in conjunction with macros.  This also
+enables some language-specific warnings described in @ref{C++ Dialect
+Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
+
+@option{-Wall} turns on the following warning flags:
+@gccoptlist{ 
+-Waddress @gol
+-Warray-bounds (only with @option{-O2}) @gol
+-Wc++0x-compat @gol
+-Wchar-subscripts @gol
+-Wimplicit-int @gol
+-Wimplicit-function-declaration @gol
+-Wcomment @gol
+-Wformat  @gol
+-Wmain (only for C/ObjC and unless @option{-ffreestanding}) @gol
+-Wmissing-braces @gol
+-Wnonnull @gol
+-Wparentheses @gol
+-Wpointer-sign
+-Wreorder  @gol
+-Wreturn-type @gol
+-Wsequence-point @gol
+-Wsign-compare (only in C++) @gol
+-Wstrict-aliasing @gol
+-Wstrict-overflow @gol
+-Wswitch @gol
+-Wtrigraphs @gol
+-Wuninitialized (only with @option{-O1}, @option{-O2}, @option{-O3}) @gol
+-Wunknown-pragmas @gol
+-Wunused-function @gol
+-Wunused-label    @gol
+-Wunused-value    @gol
+-Wunused-variable @gol
+}
+
+
 @item -Wno-import
 @opindex Wno-import
 Inhibit warning messages about the use of @samp{#import}.
@@ -2609,12 +2655,6 @@
 comment, or whenever a Backslash-Newline appears in a @samp{//} comment.
 This warning is enabled by @option{-Wall}.
 
-@item -Wfatal-errors
-@opindex Wfatal-errors
-This option causes the compiler to abort compilation on the first error
-occurred rather than trying to keep going and printing further error
-messages.
-
 @item -Wformat
 @opindex Wformat
 @opindex ffreestanding
@@ -3073,7 +3113,7 @@
 compiler is using for optimization.  The warning does not catch all
 cases, but does attempt to catch the more common pitfalls.  It is
 included in @option{-Wall}.
-It is equivalent to -Wstrict-aliasing=3
+It is equivalent to @option{-Wstrict-aliasing=3}
 
 @item -Wstrict-aliasing=n
 @opindex Wstrict-aliasing=n
@@ -3168,14 +3208,6 @@
 (default for -O2 and above). It warns about subscripts to arrays
 that are always out of bounds. This warning is enabled by @option{-Wall}.
 
-@item -Wall
-@opindex Wall
-All of the above @samp{-W} options combined.  This enables all the
-warnings about constructions that some users consider questionable, and
-that are easy to avoid (or modify to prevent the warning), even in
-conjunction with macros.  This also enables some language-specific
-warnings described in @ref{C++ Dialect Options} and
-@ref{Objective-C and Objective-C++ Dialect Options}.
 @end table
 
 The following @option{-W@dots{}} options are not implied by @option{-Wall}.

Re: enumerate explicitly what is enabled by Wall

[Manuel López-Ibáñez]

PING: http://gcc.gnu.org/ml/gcc-patches/2007-09/msg01761.html

On 23/09/2007, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> :ADDPATCH doc:
>
> With the current text in invoke.texi, it is sometimes difficult to
> tell which options are enabled by Wall. This would be easier if we
> followed the approach for describing -OX, as this patch implements. I
> have also reviewed all the flags actually enabled by Wall in the code.
>
> OK to commit?
>
> Cheers,
>
> Manuel.
>
> 2007-09-23  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>
>   * doc/invoke.texi: List the options enabled by Wall.
>
>

Re: enumerate explicitly what is enabled by Wall

[Diego Novillo]

On 9/29/07, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> PING: http://gcc.gnu.org/ml/gcc-patches/2007-09/msg01761.html

Looks fine to me.  Though we are now in stage 3, I think it's always
good to improve the documentation.  I'll defer the final decision to
Mark and the doc maintainers.

Re: enumerate explicitly what is enabled by Wall

[Gerald Pfeifer]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1547 bytes --]

On Sun, 23 Sep 2007, Manuel López-Ibáñez wrote:
> With the current text in invoke.texi, it is sometimes difficult to
> tell which options are enabled by Wall. This would be easier if we
> followed the approach for describing -OX, as this patch implements. I
> have also reviewed all the flags actually enabled by Wall in the code.

This certainly brings some nice benefits.  There is, however, one use 
case that I am worried about us losing by this change:  When I started
to use GCC, and a few times thereafter, I went through the list of all 
options below the -Wall description to see whether to explicitly add
some. Recently, in GCC terms ;-), a kind volunteer added a reference to
-Wall to relevant options.  Are you confident this list is correct?  In
that case, consider my comment moot, and this part of the patch is fine.

Your patch also moves -Wfatal-errors which is not listed in the ChangeLog.
Is this change intentional, or ment to be separate?

Please, also note your fixing of @option{-Wstrict-aliasing=3} in the
ChangeLog.

:REVIEWMAIL:

On Sat, 29 Sep 2007, Diego Novillo wrote:
> Looks fine to me.  Though we are now in stage 3, I think it's always
> good to improve the documentation.  I'll defer the final decision to
> Mark and the doc maintainers.

Documentation improvements are generally fine also throughout stage 3
as long as they don't break the bootstrap. ;-)  To make sure we document 
this properly, I had a look and indeed http://gcc.gnu.org/develop.html
already refers to "(non-documentation) changes".

Gerald

Re: enumerate explicitly what is enabled by Wall

[Manuel López-Ibáñez]

On 30/09/2007, Gerald Pfeifer <gerald@pfeifer.com> wrote:
> On Sun, 23 Sep 2007, Manuel López-Ibáñez wrote:
> > With the current text in invoke.texi, it is sometimes difficult to
> > tell which options are enabled by Wall. This would be easier if we
> > followed the approach for describing -OX, as this patch implements. I
> > have also reviewed all the flags actually enabled by Wall in the code.
>
> This certainly brings some nice benefits.  There is, however, one use
> case that I am worried about us losing by this change:  When I started
> to use GCC, and a few times thereafter, I went through the list of all
> options below the -Wall description to see whether to explicitly add
> some. Recently, in GCC terms ;-), a kind volunteer added a reference to
> -Wall to relevant options.  Are you confident this list is correct?  In
> that case, consider my comment moot, and this part of the patch is fine.

Sorry, I cannot understand what you mean. Could you elaborate a bit
further? I went to gcc/c-opts.c and checked that the list of options
is correct.

Actually, I have this little patch that is neutral (do not change
behaviour), bootstrapped and regression tested. I didn't submit it
because we are in stage3, but perhaps an exception can be made here.

2007-09-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

 * c-opts.c(c_common_handle_option): -Wnontemplate-friend,
-Wwrite-strings and -Wmultichar are enabled by default, so Wall
enabling them is redundant. Don't check two times for c_dialect_cxx.

Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c        (revision 128662)
+++ gcc/c-opts.c        (working copy)
@@ -400,8 +400,6 @@
       warn_parentheses = value;
       warn_return_type = value;
       warn_sequence_point = value;     /* Was C only.  */
-      if (c_dialect_cxx ())
-       warn_sign_compare = value;
       warn_switch = value;
       set_Wstrict_aliasing (value);
       warn_address = value;
@@ -425,17 +423,14 @@
       else
        {
          /* C++-specific warnings.  */
+          warn_sign_compare = value;
          warn_reorder = value;
-         warn_nontemplate_friend = value;
           warn_cxx0x_compat = value;
-          if (value > 0)
-            warn_write_strings = true;
        }

       cpp_opts->warn_trigraphs = value;
       cpp_opts->warn_comments = value;
       cpp_opts->warn_num_sign_change = value;
-      cpp_opts->warn_multichar = value;        /* Was C++ only.  */

       if (warn_pointer_sign == -1)
        warn_pointer_sign = 1;


> Your patch also moves -Wfatal-errors which is not listed in the ChangeLog.
> Is this change intentional, or ment to be separate?

It didn't make sense to me to move Wall before Wfatal-errors. But I
will submit that change separately.

> Please, also note your fixing of @option{-Wstrict-aliasing=3} in the
> ChangeLog.

True!

> :REVIEWMAIL:

Thanks!

Manuel.

Re: enumerate explicitly what is enabled by Wall

[Gerald Pfeifer]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1637 bytes --]

On Sun, 30 Sep 2007, Manuel López-Ibáñez wrote:
>> This certainly brings some nice benefits.  There is, however, one use
>> case that I am worried about us losing by this change:  When I started
>> to use GCC, and a few times thereafter, I went through the list of all
>> options below the -Wall description to see whether to explicitly add
>> some. Recently, in GCC terms ;-), a kind volunteer added a reference to
>> -Wall to relevant options.  Are you confident this list is correct?  In
>> that case, consider my comment moot, and this part of the patch is fine.
> Sorry, I cannot understand what you mean. Could you elaborate a bit
> further? I went to gcc/c-opts.c and checked that the list of options
> is correct.

This is about documentation: Previously, one could go to the manual,
read the description of -Wall and all options _not_ covered by -Wall
would be below that point.  This won't be as easy with your patch,
will it?  That said, if you are confident that all options implied
by -Wall are marked thusly, I have no objections to this aspect of
your patch which means the overall patch is fine.

> Actually, I have this little patch that is neutral (do not change
> behaviour), bootstrapped and regression tested. I didn't submit it
> because we are in stage3, but perhaps an exception can be made here.
> 
> 2007-09-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
> 
>  * c-opts.c(c_common_handle_option): -Wnontemplate-friend,
> -Wwrite-strings and -Wmultichar are enabled by default, so Wall
> enabling them is redundant. Don't check two times for c_dialect_cxx.

This looks more like a bug fix, really.  Mark?

Gerald

Re: enumerate explicitly what is enabled by Wall

[Mark Mitchell]

Gerald Pfeifer wrote:

>> 2007-09-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>>
>>  * c-opts.c(c_common_handle_option): -Wnontemplate-friend,
>> -Wwrite-strings and -Wmultichar are enabled by default, so Wall
>> enabling them is redundant. Don't check two times for c_dialect_cxx.
> 
> This looks more like a bug fix, really.  Mark?

Please send me a URL for the full patch, and I shall review.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: enumerate explicitly what is enabled by Wall

[Manuel López-Ibáñez]

On 08/10/2007, Mark Mitchell <mark@codesourcery.com> wrote:
> Gerald Pfeifer wrote:
>
> >> 2007-09-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
> >>
> >>  * c-opts.c(c_common_handle_option): -Wnontemplate-friend,
> >> -Wwrite-strings and -Wmultichar are enabled by default, so Wall
> >> enabling them is redundant. Don't check two times for c_dialect_cxx.
> >
> > This looks more like a bug fix, really.  Mark?
>
> Please send me a URL for the full patch, and I shall review.
>

The patch is inlined here:
http://gcc.gnu.org/ml/gcc-patches/2007-09/msg02104.html

Cheers,

Manuel.

Re: enumerate explicitly what is enabled by Wall

[Mark Mitchell]

Manuel López-Ibáñez wrote:
> On 08/10/2007, Mark Mitchell <mark@codesourcery.com> wrote:
>> Gerald Pfeifer wrote:
>>
>>>> 2007-09-30  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>>>>
>>>>  * c-opts.c(c_common_handle_option): -Wnontemplate-friend,
>>>> -Wwrite-strings and -Wmultichar are enabled by default, so Wall
>>>> enabling them is redundant. Don't check two times for c_dialect_cxx.
>>> This looks more like a bug fix, really.  Mark?
>> Please send me a URL for the full patch, and I shall review.
>>
> 
> The patch is inlined here:
> http://gcc.gnu.org/ml/gcc-patches/2007-09/msg02104.html

This patch is OK.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713