Hi,
If you use PG_ENSURE_ERROR_CLEANUP with nested blocks, the compiler warns about
shadow locals. There is no harm since each PG_TRY and PG_CATCH uses its own
context stack. The current code doesn't have an example of such usage but
pglogical [1] does. I don't know if other extensions have the same issue too.
The PG_TRY and PG_CATCH already accept an optional parameter (commit
112f0225dbfe) to avoid this exact issue. Since PG_ENSURE_ERROR_CLEANUP /
PG_END_ENSURE_ERROR_CLEANUP already use PG_TRY / PG_CATCH, add an optional
parameter too. (I opted to a variadic argument argument -- mimicking how PG_TRY
already handles its optional suffix parameter -- because it avoids changing
existing callers).
[1] https://github.com/2ndQuadrant/pglogical/issues/521#issuecomment-4925354899
--
Euler Taveira
EDB https://www.enterprisedb.com/
From 6a96281fa49a3a6de135d16e977069a7dad258b2 Mon Sep 17 00:00:00 2001
From: Euler Taveira <[email protected]>
Date: Mon, 27 Jul 2026 10:50:39 -0300
Subject: [PATCH v1] Avoid shadowing variables for nested
PG_ENSURE_ERROR_CLEANUP
PG_ENSURE_ERROR_CLEANUP() and PG_END_ENSURE_ERROR_CLEANUP() macros
expand to PG_TRY(), PG_CATCH() and PG_END_TRY() blocks. Nesting two
blocks in the same function makes the PG_TRY (inner block) generate
locals shadow the outer block. This is purely cosmetic. Each PG_TRY()
only touches its own copy of the saved context so the shadowing has no
runtime effect. The PG_TRY macros already have an additional parameter
(commit 112f0225dbfe), let's use it for PG_ENSURE_ERROR_CLEAUP macros
too.
---
src/include/storage/ipc.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index b205b00e7a1..4ffed378a08 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -34,7 +34,8 @@ typedef void (*shmem_startup_hook_type) (void);
*
* where the cleanup code is in a function declared per pg_on_exit_callback.
* The Datum value "arg" can carry any information the cleanup function
- * needs.
+ * needs. If an optional argument is informed, the nested blocks use distinct
+ * variable names (avoid shadowing and keep the compiler quiet).
*
* This construct ensures that cleanup_function() will be called during
* either ERROR or FATAL exits. It will not be called on successful
@@ -44,20 +45,20 @@ typedef void (*shmem_startup_hook_type) (void);
* Note: the macro arguments are multiply evaluated, so avoid side-effects.
*----------
*/
-#define PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg) \
+#define PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg, ...) \
do { \
before_shmem_exit(cleanup_function, arg); \
- PG_TRY()
+ PG_TRY(__VA_ARGS__)
-#define PG_END_ENSURE_ERROR_CLEANUP(cleanup_function, arg) \
+#define PG_END_ENSURE_ERROR_CLEANUP(cleanup_function, arg, ...) \
cancel_before_shmem_exit(cleanup_function, arg); \
- PG_CATCH(); \
+ PG_CATCH(__VA_ARGS__); \
{ \
cancel_before_shmem_exit(cleanup_function, arg); \
cleanup_function (0, arg); \
PG_RE_THROW(); \
} \
- PG_END_TRY(); \
+ PG_END_TRY(__VA_ARGS__); \
} while (0)
--
2.39.5
