[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: stat-time.h vs. -Waggregate-return
From: |
Jim Meyering |
Subject: |
Re: stat-time.h vs. -Waggregate-return |
Date: |
Sun, 02 Sep 2012 11:03:02 +0200 |
Jim Meyering wrote:
> 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
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 <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:
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
- Re: stat-time.h vs. -Waggregate-return,
Jim Meyering <=