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).
