Jim Meyering wrote: > Simon Josefsson wrote: >> Eric Blake <ebl...@redhat.com> 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 <meyer...@redhat.com> > Date: Wed, 30 Nov 2011 14:25:35 +0100 > Subject: [PATCH] manywarnings: update the list of "all" warnings
No one replied, so I've updated the two lists from the latest gcc (built from git/svn yesterday) and have tested the result by building coreutils, grep and diffutils using that new set of warnings. Only one small change was needed to make coreutils compile with --enable-gcc-warnings: [though the factor complaint was about vfprintf, which you'd think would have the attribute in glibc's stdio.h -- it does not ] src/factor.c: In function 'debug': src/factor.c:62:7: error: function might be possible candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format] vfprintf (stderr, fmt, ap); ^ cc1: all warnings being treated as errors make: *** [src/factor.o] Error 1 diff --git a/configure.ac b/configure.ac index 51782a5..627920d 100644 --- a/configure.ac +++ b/configure.ac @@ -128,6 +128,7 @@ if test "$gl_gcc_warnings" = yes; then nw="$nw -Wmissing-format-attribute" # copy.c nw="$nw -Wunsafe-loop-optimizations" # a few src/*.c nw="$nw -Winline" # system.h's readdir_ignoring_dot_and_dotdot + nw="$nw -Wsuggest-attribute=format" # warns about copy.c and factor.c # Using -Wstrict-overflow is a pain, but the alternative is worse. # For an example, see the code that provoked this report: I made one change to grep: http://thread.gmane.org/gmane.comp.gnu.grep.bugs/4664 and diffutils required no change. >From dd44da552f3f158a55b04fbe11ef1d0faf5ee5ba Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> 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: gl=.; 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 | 85 +++++++++++++++++++++++++ m4/manywarnings.m4 | 153 ++++++++++++++++++++++++++------------------- 2 files changed, 173 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..74c6503 --- /dev/null +++ b/build-aux/gcc-warning.spec @@ -0,0 +1,85 @@ +# options to filter out, and why +--all-warnings alias for -Wall +--extra-warnings alias for -Wextra +-Waggregate-return obsolescent +-Waliasing fortran +-Walign-commons fortran +-Wampersand fortran +-Warray-temporaries fortran +-Wassign-intercept objc/objc++ +-Wc++-compat FIXME maybe? borderline. some will want this +-Wc++0x-compat c++ +-Wc++11-compat c++ +-Wc-binding-type fortran +-Wc-binding-type fortran +-Wcast-qual FIXME maybe? too much noise; encourages bad changes +-Wcharacter-truncation fortran +-Wcompare-reals fortran +-Wconversion FIXME maybe? too much noise; encourages bad changes +-Wconversion-extra fortran +-Wconversion-null c++ and objc++ +-Wctor-dtor-privacy c++ +-Wdeclaration-after-statement FIXME: do not want. others may +-Wdeclaration-after-statement obsolescent +-Wdelete-non-virtual-dtor c++ +-Weffc++ c++ +-Werror-implicit-function-declaration deprecated +-Wfloat-equal FIXME maybe? borderline. some will want this +-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 +-Wliteral-suffix c++ and objc++ +-Wliteral-suffix c++ and objc++ +-Wlong-long obsolescent +-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: dunno +-Wpadded FIXME maybe? warns about "stabil" member in /usr/include/bits/timex.h +-Wpedantic FIXME: too strict? +-Wpmf-conversions c++ and objc++ +-Wproperty-assign-default objc++ +-Wprotocol objc++ +-Wreal-q-constant fortran +-Wrealloc-lhs fortran +-Wrealloc-lhs fortran +-Wrealloc-lhs-all fortran +-Wrealloc-lhs-all fortran +-Wredundant-decls FIXME maybe? many _gl_cxxalias_dummy FPs +-Wreorder c++ and objc++ +-Wselector objc and objc++ +-Wsign-compare FIXME maybe? borderline. some will want this +-Wsign-conversion FIXME maybe? borderline. some will want this +-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 +-Wswitch-enum FIXME maybe? borderline. some will want this +-Wsynth deprecated +-Wtabs fortran +-Wtarget-lifetime fortran +-Wtraditional obsolescent +-Wtraditional-conversion obsolescent +-Wundeclared-selector objc and objc++ +-Wundef FIXME maybe? too many false positives +-Wunderflow fortran +-Wunsuffixed-float-constants triggers warning in gnulib's timespec.h +-Wunused-dummy-argument fortran +-Wuseless-cast c++ and objc++ +-Wuseless-cast c++ and objc++ +-Wzero-as-null-pointer-constant c++ and objc++ +-frequire-return-statement go diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4 index 864fc85..2760efb 100644 --- a/m4/manywarnings.m4 +++ b/m4/manywarnings.m4 @@ -81,95 +81,118 @@ 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 \ + -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 \ + -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 \ + -Wpointer-arith \ + -Wpointer-sign \ -Wpointer-to-int-cast \ -Wpragmas \ + -Wreturn-type \ + -Wsequence-point \ + -Wshadow \ + -Wsizeof-pointer-memaccess \ + -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 \ + -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 \ + -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.146.g16d26b1