Re: stat-time.h vs. -Waggregate-return

[Jim Meyering]

Simon Josefsson wrote:
> Eric Blake <address@hidden> writes:
>> On 08/02/2012 04:49 PM, Paul Eggert wrote:
>>> On 08/02/2012 03:40 PM, Eric Blake wrote:
>>>> /* Store *ST's access time into *TS.  */
>>>> static inline void
>>>> get_stat_atime (struct stat const *st, struct timespec *ts)
>>>
>>> I'd rather not go that route, as the functional style is easier
>>> to read -- in particular, it tends to avoid aliasing issues.
>>>
>>> In the GNU apps I'm familiar with (Emacs, coreutils, ...)
>>> we simply disable -Waggregate-return.  It a completely
>>> anachronistic warning, since its motivation was to
>>> support backwards compatibility with C compilers that
>>> did not allow returning structures.  Those compilers
>>> are long dead and are no longer of practical concern.
>>
>> Fair enough; that argues that 'manywarnings' should be customized to
>> easily ditch this and other anachronistic warnings (for example,
>> -Wtraditional, -Wlong-long) in one line, rather than making every single
>> client repeat the same list of customizations.
>
> I agree the current situation could be improved, and we have touched on
> this before.  I would prefer to solve this with a two-step approach:
>
> 1) The goal of the manywarnings module should be to discover ALL warning
> flags that the compiler supports, whether the warning message is useful
> or not [as long as it doesn't break builds].  Having the complete list
> of warnings a compiler support available can be useful for experimenting
> with various coding styles.
>
> 2) There could be a 'reasonablewarnings' module that depended on
> manywarnings, which would do further filtering of the warning list to
> limit it to warning flags relevant to GNU-style project.  This module
> could remove flags like -Wtraditional, -Wsystem-headers and others which
> we consider useless for the majority of projects.
>
> What do you think?
>
> (maybe the module names should be improved though)

I worked on this about a year ago, and have just refreshed
my list and filter against the latest from upstream gcc/master.
Here's what I suggest:

Periodically run this with the very latest gcc in your path.
Assuming you have a cloned gnulib directory in $gl, it will print
out any options that have been added to gcc that are not yet on our list:

    comm -1 -3 \
      <(sed -n 's/^  *\(-[^ ]*\) .*/\1/p' $gl/m4/manywarnings.m4 |sort) \
      <(gcc --help=warnings|sed -n 's/^  \(-[^ ]*\) .*/\1/p' |sort \
        |grep -v --line-regexp -f <(cut -f1 $gl/build-aux/gcc-warning.spec))

That working depends on a new file, tentatively named
build-aux/gcc-warning.spec, that tells how to classify
these warnings (currently binary: use it, or don't).

Here's the file, most of which I wrote months ago.  Considering the
number of FIXME notes, you can see that we are almost certain to want
more than two categories, and probably multiple tags on the RHS.  Some of
the FIXME comments reflect the simple fact that I didn't have a lot of
time to read-the-documentation/understand and experiment with each of
those options.  Having multiple tags will allow packages to customize
filtering, e.g., to enable all c++-related warnings, but to exclude
warnings that are deemed obsolescent or too strict.

Suggestions welcome.

>From 51d2b564fe80e603f4fb801b7265db9c27ce91ac Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 30 Nov 2011 14:25:35 +0100
Subject: [PATCH] manywarnings: update the list of "all" warnings

* m4/manywarnings.m4: Unite lists, and add many new options.
* build-aux/gcc-warning: New file.

Run this command with the latest gcc to see if they have added
options not yet on our list:

comm -1 -3 \
  <(sed -n 's/^  *\(-[^ ]*\) .*/\1/p' $gl/m4/manywarnings.m4 |sort) \
  <(gcc --help=warnings|sed -n 's/^  \(-[^ ]*\) .*/\1/p' |sort \
    |grep -v --line-regexp -f <(cut -f1 $gl/build-aux/gcc-warning.spec))
---
 build-aux/gcc-warning.spec |  77 ++++++++++++++++++++++
 m4/manywarnings.m4         | 159 +++++++++++++++++++++++++++------------------
 2 files changed, 171 insertions(+), 65 deletions(-)
 create mode 100644 build-aux/gcc-warning.spec

diff --git a/build-aux/gcc-warning.spec b/build-aux/gcc-warning.spec
new file mode 100644
index 0000000..d428293
--- /dev/null
+++ b/build-aux/gcc-warning.spec
@@ -0,0 +1,77 @@
+# options to filter out, and why
+--all-warnings                         alias for -Wall
+--extra-warnings                       alias for -Wextra
+-Waliasing                             fortran
+-Walign-commons                                fortran
+-Wampersand                            fortran
+-Warray-temporaries                    fortran
+-Wassign-intercept                     objc/objc++
+-Wc++0x-compat                         c++
+-Wc++11-compat                         c++
+-Wcharacter-truncation                 fortran
+-Wconversion-extra                     fortran
+-Wconversion-null                      c++ and objc++
+-Wctor-dtor-privacy                    c++
+-Wdeclaration-after-statement           obsolescent
+-Wdelete-non-virtual-dtor              c++
+-Weffc++                               c++
+-Werror-implicit-function-declaration  deprecated
+-Wformat                               covered by -Wformat=2
+-Wformat=                              gcc --help=warnings artifact
+-Wfunction-elimination                 fortran
+-Wimplicit-interface                   fortran
+-Wimplicit-procedure                   fortran
+-Wintrinsic-shadow                     fortran
+-Wintrinsics-std                       fortran
+-Winvalid-offsetof                     c++ and objc++
+-Wlarger-than-                         gcc --help=warnings artifact
+-Wlarger-than=<number>                 FIXME: choose something sane?
+-Wline-truncation                      fortran
+-Wnoexcept                             c++
+-Wnon-template-friend                  c++
+-Wnon-virtual-dtor                     c++
+-Wnormalized=<id|nfc|nfkc>             FIXME: choose something sane?
+-Wold-style-cast                       c++ and objc++
+-Woverloaded-virtual                   c++
+-Wpadded                                FIXME maybe?  warns about "stabil" 
member in /usr/include/bits/timex.h
+-Wpmf-conversions                      c++ and objc++
+-Wproperty-assign-default              objc++
+-Wprotocol                             objc++
+-Wreal-q-constant                      fortran
+-Wredundant-decls                      FIXME maybe? many _gl_cxxalias_dummy FPs
+-Wreorder                              c++ and objc++
+-Wselector                             objc and objc++
+-Wsign-promo                           c++ and objc++
+-Wstack-usage=                         FIXME: choose something sane?
+-Wstrict-aliasing=                     FIXME: choose something sane?
+-Wstrict-null-sentinel                 c++ and objc++
+-Wstrict-overflow=                     FIXME: choose something sane?
+-Wstrict-selector-match                        objc and objc++
+-Wsurprising                           fortran
+-Wsynth                                        deprecated
+-Wtabs                                 fortran
+-Wtraditional                          obsolescent
+-Wtraditional-conversion               obsolescent
+-Waggregate-return                     obsolescent
+-Wundeclared-selector                  objc and objc++
+-Wundef                                        FIXME maybe? too many false 
positives
+-Wunderflow                            fortran
+-Wunused-dummy-argument                        fortran
+-Wzero-as-null-pointer-constant                c++ and objc++
+-frequire-return-statement             go
+-Wcast-qual                            FIXME maybe? too much noise; encourages 
bad changes
+-Wconversion                           FIXME maybe? too much noise; encourages 
bad changes
+-Wlong-long                            obsolescent
+-Wc++-compat                           FIXME maybe? borderline.  some will 
want this
+-Wsign-conversion                      FIXME maybe? borderline.  some will 
want this
+-Wsign-compare                         FIXME maybe? borderline.  some will 
want this
+-Wswitch-enum                          FIXME maybe? borderline.  some will 
want this
+-Wfloat-equal                          FIXME maybe? borderline.  some will 
want this
+-Wdeclaration-after-statement          FIXME: do not want.  others may
+-Wpadded                               FIXME: dunno
+-Wrealloc-lhs                          fortran
+-Wrealloc-lhs-all                      fortran
+-Wc-binding-type                       fortran
+-Wliteral-suffix                       c++ and objc++
+-Wuseless-cast                         c++ and objc++
+-Wpedantic                             FIXME: too strict?
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 864fc85..c3a51b0 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -81,95 +81,124 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],

   gl_manywarn_set=
   for gl_manywarn_item in \
-    -Wall \
     -W \
-    -Wformat-y2k \
-    -Wformat-nonliteral \
-    -Wformat-security \
-    -Winit-self \
-    -Wmissing-include-dirs \
-    -Wswitch-default \
-    -Wswitch-enum \
-    -Wunused \
-    -Wunknown-pragmas \
-    -Wstrict-aliasing \
-    -Wstrict-overflow \
-    -Wsystem-headers \
-    -Wfloat-equal \
-    -Wtraditional \
-    -Wtraditional-conversion \
-    -Wdeclaration-after-statement \
-    -Wundef \
-    -Wshadow \
-    -Wunsafe-loop-optimizations \
-    -Wpointer-arith \
+    -Wabi \
+    -Waddress \
+    -Wall \
+    -Warray-bounds \
+    -Wattributes \
     -Wbad-function-cast \
-    -Wc++-compat \
-    -Wcast-qual \
-    -Wcast-align \
-    -Wwrite-strings \
-    -Wconversion \
-    -Wsign-conversion \
-    -Wlogical-op \
-    -Waggregate-return \
-    -Wstrict-prototypes \
-    -Wold-style-definition \
-    -Wmissing-prototypes \
-    -Wmissing-declarations \
-    -Wmissing-noreturn \
-    -Wmissing-format-attribute \
-    -Wpacked \
-    -Wpadded \
-    -Wredundant-decls \
-    -Wnested-externs \
-    -Wunreachable-code \
-    -Winline \
-    -Winvalid-pch \
-    -Wlong-long \
-    -Wvla \
-    -Wvolatile-register-var \
-    -Wdisabled-optimization \
-    -Wstack-protector \
-    -Woverlength-strings \
     -Wbuiltin-macro-redefined \
-    -Wmudflap \
-    -Wpacked-bitfield-compat \
-    -Wsync-nand \
-    ; do
-    gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
-  done
-  # The following are not documented in the manual but are included in
-  # output from gcc --help=warnings.
-  for gl_manywarn_item in \
-    -Wattributes \
+    -Wc-binding-type \
+    -Wcast-align \
+    -Wchar-subscripts \
+    -Wclobbered \
+    -Wcomment \
+    -Wcomments \
     -Wcoverage-mismatch \
-    -Wunused-macros \
-    ; do
-    gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
-  done
-  # More warnings from gcc 4.6.2 --help=warnings.
-  for gl_manywarn_item in \
-    -Wabi \
     -Wcpp \
     -Wdeprecated \
     -Wdeprecated-declarations \
+    -Wdisabled-optimization \
     -Wdiv-by-zero \
     -Wdouble-promotion \
+    -Wempty-body \
     -Wendif-labels \
+    -Wenum-compare \
     -Wextra \
     -Wformat-contains-nul \
     -Wformat-extra-args \
+    -Wformat-nonliteral \
+    -Wformat-security \
+    -Wformat-y2k \
     -Wformat-zero-length \
     -Wformat=2 \
+    -Wfree-nonheap-object \
+    -Wignored-qualifiers \
+    -Wimplicit \
+    -Wimplicit-function-declaration \
+    -Wimplicit-int \
+    -Winit-self \
+    -Winline \
+    -Wint-to-pointer-cast \
+    -Winvalid-memory-model \
+    -Winvalid-pch \
+    -Wjump-misses-init \
+    -Wliteral-suffix \
+    -Wlogical-op \
+    -Wmain \
+    -Wmaybe-uninitialized \
+    -Wmissing-braces \
+    -Wmissing-declarations \
+    -Wmissing-field-initializers \
+    -Wmissing-format-attribute \
+    -Wmissing-include-dirs \
+    -Wmissing-noreturn \
+    -Wmissing-parameter-type \
+    -Wmissing-prototypes \
+    -Wmudflap \
     -Wmultichar \
+    -Wnarrowing \
+    -Wnested-externs \
+    -Wnonnull \
     -Wnormalized=nfc \
+    -Wold-style-declaration \
+    -Wold-style-definition \
     -Woverflow \
+    -Woverlength-strings \
+    -Woverride-init \
+    -Wpacked \
+    -Wpacked-bitfield-compat \
+    -Wparentheses \
+    -Wpedantic \
+    -Wpointer-arith \
+    -Wpointer-sign \
     -Wpointer-to-int-cast \
     -Wpragmas \
+    -Wrealloc-lhs \
+    -Wrealloc-lhs-all \
+    -Wreturn-type \
+    -Wsequence-point \
+    -Wshadow \
+    -Wstack-protector \
+    -Wstrict-aliasing \
+    -Wstrict-overflow \
+    -Wstrict-prototypes \
     -Wsuggest-attribute=const \
+    -Wsuggest-attribute=format \
     -Wsuggest-attribute=noreturn \
     -Wsuggest-attribute=pure \
+    -Wswitch \
+    -Wswitch-default \
+    -Wsync-nand \
+    -Wsystem-headers \
     -Wtrampolines \
+    -Wtrigraphs \
+    -Wtype-limits \
+    -Wuninitialized \
+    -Wunknown-pragmas \
+    -Wunreachable-code \
+    -Wunsafe-loop-optimizations \
+    -Wunsuffixed-float-constants \
+    -Wunused \
+    -Wunused-but-set-parameter \
+    -Wunused-but-set-variable \
+    -Wunused-function \
+    -Wunused-label \
+    -Wunused-local-typedefs \
+    -Wunused-macros \
+    -Wunused-parameter \
+    -Wunused-result \
+    -Wunused-value \
+    -Wunused-variable \
+    -Wuseless-cast \
+    -Wvarargs \
+    -Wvariadic-macros \
+    -Wvector-operation-performance \
+    -Wvla \
+    -Wvolatile-register-var \
+    -Wwrite-strings \
+    \
     ; do
     gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
   done
--
1.7.12.rc1.10.g97c7934