On Wed, Dec 8, 2021 at 7:48 AM Bruno Haible <[email protected]> wrote: > > > especially given it's in the gcc -Wextra set, not some random flag > > Warnings in the '-Wall' category are typically worth eliminating: they > regularly point to real bugs. > > Eliminating '-Wsign-compare' warnings, OTOH, is usually a waste of time, in > my experience: I have hardly ever found a bug while chasing them. > > Robbie Harwood wrote: > > > As you point out with coreutils, gnulib taking a position like this on > > > flags has the knock-on effect that, for our grub, we'll need to do one > > > of the following: > > > > > > - Carry a gnulib patch to make the flag work (what we've been doing). > > > - Change flags in the outer work (i.e., change build options for grub) > > > - Maintain logic to keep flags gnulib-disliked flags out when building > > > the gnulib modules > > Paul Eggert replied: > > Every other Gnulib-using project I know takes the third approach. > > So, how about adding the third approach to gnulib-tool? > > Rationale: > Gnulib does not want to dictate their preferred GCC flags to coreutils, > grub, etc. > And similarly, we don't want coreutils, grub, etc. to dictate the coding > style in which gnulib is written. > > Implementation idea: > Since 2021-06-10, gnulib-tool already ensures that the Gnulib modules are > compiled with '-Wno-error'. This code could be extended to add > '-Wno-sign-compare' and other warning-disabling options. > > Question: > Which warning options should we disable this way? > 1) We have some comments in build-aux/gcc-warning.spec. > 2) When I collect all warnings in the gllib/ directory of a full testdir, > I get (with gcc-9.3): > > $ grep warning: gllib.log | sed -e 's/^.*\[\(-W.*\)\]$/\1/' | LC_ALL=C sort | > LC_ALL=C uniq -c > 72 -Wattribute-warning > 169 -Wcast-qual > 457 -Wconversion > 1 -Wempty-body > 10 -Wfloat-conversion > 24 -Wfloat-equal > 7 -Wimplicit-fallthrough= > 9 -Wmaybe-uninitialized > 5 -Wmissing-field-initializers > 1 -Wpedantic > 7 -Wredundant-decls > 8 -Wrestrict > 156 -Wsign-compare > 4324 -Wsign-conversion > 2 -Wtype-limits > 888 -Wundef > 139 -Wunsuffixed-float-constants > 2 -Wunused-function > 129 -Wunused-parameter > > And with gcc-11.2.0: > $ grep warning: gllib.log | sed -e 's/^.*\[\(-W.*\)\]$/\1/' | LC_ALL=C sort | > LC_ALL=C uniq -c > 72 -Wattribute-warning > 172 -Wcast-qual > 509 -Wconversion > 4 -Wcpp > 1 -Wempty-body > 9 -Wfloat-conversion > 26 -Wfloat-equal > 7 -Wimplicit-fallthrough= > 9 -Wmaybe-uninitialized > 5 -Wmissing-field-initializers > 432 -Wpedantic > 7 -Wredundant-decls > 8 -Wrestrict > 2 -Wreturn-local-addr > 168 -Wsign-compare > 4362 -Wsign-conversion > 2 -Wtype-limits > 878 -Wundef > 155 -Wunsuffixed-float-constants > 2 -Wunused-function > 131 -Wunused-parameter > > I therefore propose to disable these warnings: > -Wattribute-warning > -Wcast-qual > -Wconversion > -Wfloat-conversion > -Wfloat-equal > -Wimplicit-fallthrough > -Wmaybe-uninitialized > -Wpedantic > -Wrestrict > -Wsign-compare > -Wsign-conversion > -Wtype-limits > -Wundef > -Wunsuffixed-float-constants > -Wunused-function > -Wunused-parameter
Hi Bruno, I don't have terribly strong feelings here, but I would be inclined to retain some of those. This exposes low-maint-cost ways to improve APIs, both to help the compiler help us (e.g., via better optimization or better static analysis) and to help readers know more at a glance about a labeled function: > 72 -Wattribute-warning, The following are issues that are usually easy/safe to address and have so few violations that they may be worth addressing or explicitly suppressing (I haven't looked): > 7 -Wimplicit-fallthrough= > 5 -Wmissing-field-initializers This one is special: historically low-value, but has been known to catch real bugs. Mark each FP instance to suppress the warning there? > 9 -Wmaybe-uninitialized
