Re: lots of unused variable warnings in assert-free builds - Mailing list pgsql-hackers

On tis, 2012-01-24 at 13:18 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > Yes, that's what I meant when I suggested it originally.  I'm just not
> > sure it's any nicer than adding ifdefs for USE_ASSERT_CHECKING.
>
> I'm inclined to think that it probably is nicer, just because of less
> vertical space used.  But again, this opinion is contingent on what it
> will look like after pgindent gets done with it ...

Here is a demo diff of what pgindent would do with the various
approaches (btw., nice job on making pgindent easy to use for everyone
now).

As you can see, pgindent adds whitespace on top of #ifdef
USE_ASSERT_CHECKING, and messes up the vertical alignment of variable
definitions that contain extra attributes.

All things considered, I like the PG_USED_FOR_ASSERTS_ONLY solution best.

diff --git i/src/backend/access/hash/hashovfl.c w/src/backend/access/hash/hashovfl.c
index 130c296..b61c8ee 100644
--- i/src/backend/access/hash/hashovfl.c
+++ w/src/backend/access/hash/hashovfl.c
@@ -391,7 +391,10 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
     uint32        ovflbitno;
     int32        bitmappage,
                 bitmapbit;
+
+#ifdef USE_ASSERT_CHECKING
     Bucket        bucket;
+#endif

     /* Get information from the doomed page */
     _hash_checkpage(rel, ovflbuf, LH_OVERFLOW_PAGE);
@@ -400,7 +403,9 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
     ovflopaque = (HashPageOpaque) PageGetSpecialPointer(ovflpage);
     nextblkno = ovflopaque->hasho_nextblkno;
     prevblkno = ovflopaque->hasho_prevblkno;
+#ifdef USE_ASSERT_CHECKING
     bucket = ovflopaque->hasho_bucket;
+#endif

     /*
      * Zero the page for debugging's sake; then write and release it. (Note:
diff --git i/src/backend/executor/execCurrent.c w/src/backend/executor/execCurrent.c
index b07161f..2c8929b 100644
--- i/src/backend/executor/execCurrent.c
+++ w/src/backend/executor/execCurrent.c
@@ -151,7 +151,7 @@ execCurrentOf(CurrentOfExpr *cexpr,
     {
         ScanState  *scanstate;
         bool        lisnull;
-        Oid            tuple_tableoid;
+        Oid tuple_tableoid PG_USED_FOR_ASSERTS_ONLY;
         ItemPointer tuple_tid;

         /*
diff --git i/src/backend/executor/nodeMaterial.c w/src/backend/executor/nodeMaterial.c
index b320b54..3a6bfec 100644
--- i/src/backend/executor/nodeMaterial.c
+++ w/src/backend/executor/nodeMaterial.c
@@ -66,7 +66,7 @@ ExecMaterial(MaterialState *node)
              * Allocate a second read pointer to serve as the mark. We know it
              * must have index 1, so needn't store that.
              */
-            int            ptrno;
+            int ptrno    PG_USED_FOR_ASSERTS_ONLY;

             ptrno = tuplestore_alloc_read_pointer(tuplestorestate,
                                                   node->eflags);
diff --git i/src/backend/executor/nodeSetOp.c w/src/backend/executor/nodeSetOp.c
index 7fa5730..ad2e80d 100644
--- i/src/backend/executor/nodeSetOp.c
+++ w/src/backend/executor/nodeSetOp.c
@@ -344,7 +344,7 @@ setop_fill_hash_table(SetOpState *setopstate)
     SetOp       *node = (SetOp *) setopstate->ps.plan;
     PlanState  *outerPlan;
     int            firstFlag;
-    bool        in_first_rel;
+    bool in_first_rel __attribute__((unused));

     /*
      * get state info from node
diff --git i/src/include/c.h w/src/include/c.h
index 82acd14..2dd5c67 100644
--- i/src/include/c.h
+++ w/src/include/c.h
@@ -850,4 +850,10 @@ extern int    fdatasync(int fildes);
 /* /port compatibility functions */
 #include "port.h"

+#ifdef USE_ASSERT_CHECKING
+#define PG_USED_FOR_ASSERTS_ONLY
+#else
+#define PG_USED_FOR_ASSERTS_ONLY __attribute__((unused))
+#endif
+
 #endif   /* C_H */