Re: shadow variables - pg15 edition

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org, Tomas Vondra <tomas.vondra@postgresql.org>, Peter Smith <smithpb2250@gmail.com>
Date: 2022-10-05T21:40:52Z
Lists: pgsql-hackers
Hi,

On 2022-10-06 10:21:41 +1300, David Rowley wrote:
> Also pushed.  (Thanks for saving me on that one.)

Your commit message said the last shadowed variable. But building with
-Wshadow=compatible-local triggers a bunch of warnings for me (see trimmed at
the end).  Looks like it "only" fixed it for src/, without optional
dependencies like gssapi and python.

I think we should add -Wshadow=compatible-local to our sets of warning flags
after fixing those.


[237/1827 42  12%] Compiling C object src/interfaces/libpq/libpq.a.p/fe-secure-gssapi.c.o
../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-secure-gssapi.c: In function ‘pg_GSS_write’:
../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-secure-gssapi.c:138:41: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=compatible-local]
  138 |                         ssize_t         ret;
      |                                         ^~~
../../../../home/andres/src/postgresql/src/interfaces/libpq/fe-secure-gssapi.c:92:25: note: shadowed declaration is here
   92 |         ssize_t         ret = -1;
      |                         ^~~


[1283/1827 42  70%] Compiling C object src/pl/plpython/plpython3.so.p/plpy_cursorobject.c.o
In file included from ../../../../home/andres/src/postgresql/src/include/postgres.h:48,
                 from ../../../../home/andres/src/postgresql/src/pl/plpython/plpy_cursorobject.c:7:
../../../../home/andres/src/postgresql/src/pl/plpython/plpy_cursorobject.c: In function ‘PLy_cursor_plan’:
../../../../home/andres/src/postgresql/src/include/utils/elog.h:325:29: warning: declaration of ‘_save_exception_stack’ shadows a previous local [-Wshadow=compatible-local]
  325 |                 sigjmp_buf *_save_exception_stack##__VA_ARGS__ = PG_exception_stack; \
      |                             ^~~~~~~~~~~~~~~~~~~~~
...

[1289/1827 42  70%] Compiling C object src/pl/plpython/plpython3.so.p/plpy_exec.c.o
../../../../home/andres/src/postgresql/src/pl/plpython/plpy_exec.c: In function ‘PLy_exec_trigger’:
../../../../home/andres/src/postgresql/src/pl/plpython/plpy_exec.c:378:46: warning: declaration of ‘tdata’ shadows a previous local [-Wshadow=compatible-local]
  378 |                                 TriggerData *tdata = (TriggerData *) fcinfo->context;
      |                                              ^~~~~
../../../../home/andres/src/postgresql/src/pl/plpython/plpy_exec.c:310:22: note: shadowed declaration is here
  310 |         TriggerData *tdata;
      |                      ^~~~~

[1291/1827 42  70%] Compiling C object src/pl/plpython/plpython3.so.p/plpy_spi.c.o
In file included from ../../../../home/andres/src/postgresql/src/include/postgres.h:48,
                 from ../../../../home/andres/src/postgresql/src/pl/plpython/plpy_spi.c:7:
../../../../home/andres/src/postgresql/src/pl/plpython/plpy_spi.c: In function ‘PLy_spi_execute_plan’:
../../../../home/andres/src/postgresql/src/include/utils/elog.h:325:29: warning: declaration of ‘_save_exception_stack’ shadows a previous local [-Wshadow=compatible-local]
  325 |                 sigjmp_buf *_save_exception_stack##__VA_ARGS__ = PG_exception_stack; \
      |                             ^~~~~~~~~~~~~~~~~~~~~


[1344/1827 42  73%] Compiling C object contrib/bloom/bloom.so.p/blinsert.c.o
../../../../home/andres/src/postgresql/contrib/bloom/blinsert.c: In function ‘blinsert’:
../../../../home/andres/src/postgresql/contrib/bloom/blinsert.c:235:33: warning: declaration of ‘page’ shadows a previous local [-Wshadow=compatible-local]
  235 |                 Page            page;
      |                                 ^~~~
../../../../home/andres/src/postgresql/contrib/bloom/blinsert.c:210:25: note: shadowed declaration is here
  210 |         Page            page,
      |                         ^~~~

[1415/1827 42  77%] Compiling C object contrib/file_fdw/file_fdw.so.p/file_fdw.c.o
../../../../home/andres/src/postgresql/contrib/file_fdw/file_fdw.c: In function ‘get_file_fdw_attribute_options’:
../../../../home/andres/src/postgresql/contrib/file_fdw/file_fdw.c:453:29: warning: declaration of ‘options’ shadows a previous local [-Wshadow=compatible-local]
  453 |                 List       *options;
      |                             ^~~~~~~
../../../../home/andres/src/postgresql/contrib/file_fdw/file_fdw.c:443:21: note: shadowed declaration is here
  443 |         List       *options = NIL;
      |                     ^~~~~~~

[1441/1827 42  78%] Compiling C object contrib/hstore/hstore.so.p/hstore_io.c.o
In file included from ../../../../home/andres/src/postgresql/contrib/hstore/hstore_io.c:12:
../../../../home/andres/src/postgresql/contrib/hstore/hstore_io.c: In function ‘hstorePairs’:
../../../../home/andres/src/postgresql/contrib/hstore/hstore.h:131:21: warning: declaration of ‘buflen’ shadows a parameter [-Wshadow=compatible-local]
  131 |                 int buflen = (ptr_) - (buf_);                                                           \
      |                     ^~~~~~
../../../../home/andres/src/postgresql/contrib/hstore/hstore_io.c:411:9: note: in expansion of macro ‘HS_FINALIZE’
  411 |         HS_FINALIZE(out, pcount, buf, ptr);
      |         ^~~~~~~~~~~
../../../../home/andres/src/postgresql/contrib/hstore/hstore_io.c:388:47: note: shadowed declaration is here
  388 | hstorePairs(Pairs *pairs, int32 pcount, int32 buflen)
      |                                         ~~~~~~^~~~~~

[1564/1827 42  85%] Compiling C object contrib/postgres_fdw/postgres_fdw.so.p/deparse.c.o
../../../../home/andres/src/postgresql/contrib/postgres_fdw/deparse.c: In function ‘foreign_expr_walker’:
../../../../home/andres/src/postgresql/contrib/postgres_fdw/deparse.c:946:53: warning: declaration of ‘lc’ shadows a previous local [-Wshadow=compatible-local]
  946 |                                         ListCell   *lc;
      |                                                     ^~
../../../../home/andres/src/postgresql/contrib/postgres_fdw/deparse.c:904:45: note: shadowed declaration is here
  904 |                                 ListCell   *lc;
      |                                             ^~

[1575/1827 38  86%] Compiling C object src/test/modules/test_integerset/test_integerset.so.p/test_integerset.c.o
../../../../home/andres/src/postgresql/src/test/modules/test_integerset/test_integerset.c: In function ‘test_huge_distances’:
../../../../home/andres/src/postgresql/src/test/modules/test_integerset/test_integerset.c:588:33: warning: declaration of ‘x’ shadows a previous local [-Wshadow=compatible-local]
  588 |                 uint64          x = values[i];
      |                                 ^
../../../../home/andres/src/postgresql/src/test/modules/test_integerset/test_integerset.c:526:25: note: shadowed declaration is here
  526 |         uint64          x;
      |                         ^

[1633/1827 3  89%] Compiling C object contrib/postgres_fdw/postgres_fdw.so.p/postgres_fdw.c.o
../../../../home/andres/src/postgresql/contrib/postgres_fdw/postgres_fdw.c: In function ‘postgresGetForeignPlan’:
../../../../home/andres/src/postgresql/contrib/postgres_fdw/postgres_fdw.c:1344:37: warning: declaration of ‘lc’ shadows a previous local [-Wshadow=compatible-local]
 1344 |                         ListCell   *lc;
      |                                     ^~
../../../../home/andres/src/postgresql/contrib/postgres_fdw/postgres_fdw.c:1238:21: note: shadowed declaration is here
 1238 |         ListCell   *lc;
      |                     ^~


Greetings,

Andres Freund



Commits

  1. Fix shadow variable in postgres.c

  2. Add -Wshadow=compatible-local to the standard compilation flags

  3. Fix final warnings produced by -Wshadow=compatible-local

  4. Fix final compiler warning produced by -Wshadow=compatible-local

  5. Add optional parameter to PG_TRY() macros

  6. Small refactor to get rid of -Wshadow=compatible-local warning

  7. More -Wshadow=compatible-local warning fixes

  8. Further -Wshadow=compatible-local warning fixes

  9. Further reduce warnings with -Wshadow=compatible-local

  10. Reduce warnings with -Wshadow=compatible-local builds

  11. Remove shadowed local variables that are new in v15