Re: warnings in unit tests

[Bruno Haible <bruno@clisp.org>]

Hi Collin,

> > For test cases this is more a judgment call, but I prefer doing either
> > the above or adjusting the warning flags, to ignoring warnings, as the
> > other warnings can be useful at time.
> 
> Yeah, I could see these warnings making it hard to see ones that
> actually matter. Lets see what Bruno thinks.

Different warning policies need to apply in these fours sets code:

1) Code that originates in the package that uses gnulib.
   Example: coreutils/src/*

2) Code from public header files in gnulib/lib/
   Example: lib/vasnprintf.h because module 'vasnprintf' designates this file
   as its public header file.

3) All other code from gnulib/lib/
   Example: lib/argp-namefrob.h which is not a public header file, lib/*.c.

4) All code in gnulib/tests/


Note that different warning policies may contradict each other. For example,
some people want to see a warning for

    int *table = malloc (n * sizeof (int));

because it has an implicit conversion / "lacks a cast". While other people
want to see a warning for

    int *table = (int *) malloc (n * sizeof (int));

because it has a cast and "casts are dubious". It is impossible to satisfy
both of these policies at the same time.


Back to the four sets of code:

1) This warning policy is the responsibility of that package's maintainer,
   obviously.

2) These header files are used in compilation units of the package, with
   CFLAGS or AM_CFLAGS set by the package's maintainer for that package.
   Therefore in these files we need to avoid even -Wundef, -Wvla, and
   other kinds of warnings that we wouldn't enable in our code.

3) The rest of the lib/ code is under our responsibility, not the
   responsibility of a package's maintainer. We try to avoid warnings
   from "reasonable" warning options. More details in the HACKING file.

4) The unit tests are also in our responsibility, not the responsibility
   of a package's maintainer. Here, the primary concern is that is must
   be *easy* to contribute new unit tests. -Wmissing-variable-declarations
   warnings _could_ — as Paul wrote — be avoided by adding an 'extern'
   declaration for each global variable. But this is extra effort that
   would hinder the addition of new unit tests.

> If we decide to follow the coding style you mentioned

No. It must be possible to contribute a unit test with a simple
global variable. Therefore -Wmissing-variable-declarations is not
adequate for unit tests.

Collin, if you want to find relevant findings in the unit tests, by
using gcc or clang warning options, do *not* use a coreutils build
for this purpose, but a gnulib testdir instead. (Because the latter
is not biased by coding style preferences of any package maintainer.)

Or if you really want to use a coreutils build, first update the
GL_CFLAG_GNULIB_WARNINGS definition in m4/gnulib-common.m4, so that
it eliminates useless kinds of warnings.

Bruno