Setting -Werror in CFLAGS

From: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
To: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Setting -Werror in CFLAGS
Date: 2012-01-04 00:39:54
Message-ID: CAEYLb_Wj7o8JmX9mqsh70YDerxCmVU2g5pLbBSmxAKB9QM6=EA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

During the talk "How To Get Your PostgreSQL Patch Accepted" during
PgCon last year, I raised the idea of making a -Werror build option
easily available. I think it was Robert that pointed out that the
problem with that was that there is a warning due to an upstream Flex
bug that we can't do anything about. He was right about that; I
subsequently tried fairly hard to get the Flex guys to fix it a few
months back, but it seems that Flex isn't that well maintained, so
even though I convinced someone to write a patch to fix it, it ended
up going nowhere. Looking back through the commit log, I can see that
this is an old problem.

i thought that we could get most of the benefit of a -Werror option by
excluding the relevant class of error in CLAGS; gcc allows for this
(pity it doesn't have MSVC's much more granular ability to
discriminate against diagnostic messages). This should work:

./configure --prefix=/home/peter/pgsql CFLAGS="-Werror
-Wno-error=unused-but-set-variable"

However, it does not (with or without the -Wno-error):

checking whether getpwuid_r takes a fifth argument... no
checking whether strerror_r returns int... no
checking test program... ok
checking whether long int is 64 bits... no
checking whether long long int is 64 bits... no
configure: error: Cannot find a working 64-bit integer type.

Obviously it breaks this one configure test. However, I got it to work
in 2 minutes by hacking up config.log. It would be nice if someone
could fix the test (and possibly later ones) so that it doesn't
produce a warning/error when invoking the compiler, and it finds a
64-bit int type as expected. That way, it could sort of become folk
wisdom that you should build Postgres with those flags during
development, and maybe even, on some limited basis, on the build farm,
and we'd all be sightly better off.

Giving that many people build with CFLAGS="-O0 -g" on a day-to-day
basis, which doesn't show some warnings, the folk wisdom might end up
being: "Just before you submit your patch, see how an -O2 -Werror
build goes".

There is an entry in the parser Makefile to support this sort of use,
which makes it look like my -Wno-error=unused-but-set-variable use
would be redundant if it worked:

# Latest flex causes warnings in this file.
ifeq ($(GCC),yes)
gram.o: CFLAGS += -Wno-error
endif

(although we could discriminate better within the parse here by using my flag).

As if to prove my point, I found this warning which I didn't
previously notice, just from 5 minutes of playing around:

hashovfl.c: In function ‘_hash_freeovflpage’:
hashovfl.c:394:10: error: variable ‘bucket’ set but not used
[-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors

(plus others)

Yes, I know that these only appeared in GCC 4.6+ and as such are a
relatively recent phenomenon, but there has been some effort to
eliminate them, and if I could get a non-hacked -Werror build I'd feel
happy enough about excluding them as already outlined.

--
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Farina 2012-01-04 01:13:24 pg_internal.init and an index file have the same inode
Previous Message Brad Davis 2012-01-04 00:24:06 Re: [patch] Improve documentation around FreeBSD Kernel Tuning