Skip site navigation (1) Skip section navigation (2)

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 (view raw or flat)
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: pg-cassert-unused-void.patch
Description: text/x-patch (2.5 KB)
Attachment: pg-cassert-unused-ifdef.patch
Description: text/x-patch (14.0 KB)
Attachment: pg-cassert-unused-attribute.patch
Description: text/x-patch (7.9 KB)

In response to

Responses

pgsql-hackers by date

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

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group