Re: lots of unused variable warnings in assert-free builds

From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: lots of unused variable warnings in assert-free builds
Date: 2012-01-21 18:06:15
Message-ID: 1327169175.4482.11.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On ons, 2012-01-18 at 21:16 +0200, Peter Eisentraut wrote:
> On sön, 2012-01-15 at 08:14 -0500, Andrew Dunstan wrote:
> > It would possibly have some documentary value too. Just looking very
> > quickly at Peter's patch, I don't really understand his assertion that
> > this would significantly butcher the code. The worst effect would be
> > that in a few cases we'd have to break up multiple declarations where
> > one of the variables was in this class. That doesn't seem like a
> > tragedy.
>
> Well, I'll prepare a patch like that and then you can judge.

So, here are three patches that could solve this issue.

pg-cassert-unused-attribute.patch, the one I already showed, using
__attribute__((unused).

pg-cassert-unused-ifdef.patch, using only additional #ifdef
USE_ASSERT_CHECKING. This does have additional documentation value, but
you can see that it gets bulky and complicated. (I introduced several
bugs merely while creating this patch.)

pg-cassert-unused-void.patch is an alternative approach that avoids the
warnings by casting the arguments of Assert() to void if assertions are
disabled. This has the least code impact, but it changes the
traditional semantics of asserts, which is that they disappear
completely when not enabled. You can see how this creates a problem in
src/backend/replication/syncrep.c, where the nontrivial call to
SyncRepQueueIsOrderedByLSN() now becomes visible even in non-assert
builds. I played around with some other options like
__attribute__((pure)) to make the compiler optimize the function away in
that case, but that didn't appear to work. So this might not be a
workable solution (and it would be GCC-specific anyway).

Attachment Content-Type Size
pg-cassert-unused-attribute.patch text/x-patch 7.9 KB
pg-cassert-unused-ifdef.patch text/x-patch 14.0 KB
pg-cassert-unused-void.patch text/x-patch 2.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2012-01-21 19:12:39 Re: PATCH: tracking temp files in pg_stat_database
Previous Message Peter Eisentraut 2012-01-21 17:48:39 Re: review: psql tab completion for GRANT role