Fix a recent "shadow warning" in subscriptioncmds.c
I noticed that recently, a shadow warning has crept into the code I am building.
----------
subscriptioncmds.c:1125:30: warning: declaration of ‘rel’ shadows a
previous local [-Wshadow]
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
../../../src/include/nodes/pg_list.h:482:20: note: in definition of
macro ‘foreach_internal’
for (type pointer var = 0, pointer var##__outerloop = (type pointer) 1; \
^
subscriptioncmds.c:1125:3: note: in expansion of macro ‘foreach_ptr’
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
subscriptioncmds.c:934:11: warning: shadowed declaration is here [-Wshadow]
Relation rel = NULL;
^
----------
This seems to have been introduced recently by the REFRESH SEQUENCES
commit f0b3573.
IIUC, this particular case is harmless, but IMO it's still worth
fixing to avoid recurring doubts about potential issues every time it
appears.
PSA patch v1 to address the warning.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v1-0001-Fix-shadow-warning-in-subscriptioncmds.c.patchapplication/octet-stream; name=v1-0001-Fix-shadow-warning-in-subscriptioncmds.c.patchDownload
From 9553785759d05c0cba05074ed08e0fb733cf4d19 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 28 Nov 2025 13:52:44 +1100
Subject: [PATCH v1] Fix shadow warning in subscriptioncmds.c
---
src/backend/commands/subscriptioncmds.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 24b7023..8c856af 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1122,10 +1122,10 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data,
* to be at the end because otherwise if there is an error while doing
* the database operations we won't be able to rollback dropped slots.
*/
- foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
+ foreach_ptr(SubRemoveRels, sub_remove_rel, sub_remove_rels)
{
- if (rel->state != SUBREL_STATE_READY &&
- rel->state != SUBREL_STATE_SYNCDONE)
+ if (sub_remove_rel->state != SUBREL_STATE_READY &&
+ sub_remove_rel->state != SUBREL_STATE_SYNCDONE)
{
char syncslotname[NAMEDATALEN] = {0};
@@ -1139,7 +1139,7 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data,
* dropped slots and fail. For these reasons, we allow
* missing_ok = true for the drop.
*/
- ReplicationSlotNameForTablesync(sub->oid, rel->relid,
+ ReplicationSlotNameForTablesync(sub->oid, sub_remove_rel->relid,
syncslotname, sizeof(syncslotname));
ReplicationSlotDropAtPubNode(wrconn, syncslotname, true);
}
--
1.8.3.1
On Nov 28, 2025, at 11:59, Peter Smith <smithpb2250@gmail.com> wrote:
I noticed that recently, a shadow warning has crept into the code I am building.
----------
subscriptioncmds.c:1125:30: warning: declaration of ‘rel’ shadows a
previous local [-Wshadow]
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
../../../src/include/nodes/pg_list.h:482:20: note: in definition of
macro ‘foreach_internal’
for (type pointer var = 0, pointer var##__outerloop = (type pointer) 1; \
^
subscriptioncmds.c:1125:3: note: in expansion of macro ‘foreach_ptr’
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
subscriptioncmds.c:934:11: warning: shadowed declaration is here [-Wshadow]
Relation rel = NULL;
^
----------This seems to have been introduced recently by the REFRESH SEQUENCES
commit f0b3573.IIUC, this particular case is harmless, but IMO it's still worth
fixing to avoid recurring doubts about potential issues every time it
appears.PSA patch v1 to address the warning.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
<v1-0001-Fix-shadow-warning-in-subscriptioncmds.c.patch>
The fix just renamed the loop variable name “rel” that hides the other local variable to “sub_remove_rel”, totally makes sense.
I’m just curious why my compile doesn’t warn on that? I am on M4 MacOS 15.6.1, Apple clang 17.0.0.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Fri, Nov 28, 2025 at 3:50 PM Chao Li <li.evan.chao@gmail.com> wrote:
...
I’m just curious why my compile doesn’t warn on that? I am on M4 MacOS 15.6.1, Apple clang 17.0.0.
It's not the default build settings. I have this extra warning enabled
locally so that any code I write or review will not accidentally
introduce things like this.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Nov 28, 2025 at 9:29 AM Peter Smith <smithpb2250@gmail.com> wrote:
I noticed that recently, a shadow warning has crept into the code I am building.
----------
subscriptioncmds.c:1125:30: warning: declaration of ‘rel’ shadows a
previous local [-Wshadow]
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
../../../src/include/nodes/pg_list.h:482:20: note: in definition of
macro ‘foreach_internal’
for (type pointer var = 0, pointer var##__outerloop = (type pointer) 1; \
^
subscriptioncmds.c:1125:3: note: in expansion of macro ‘foreach_ptr’
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
subscriptioncmds.c:934:11: warning: shadowed declaration is here [-Wshadow]
Relation rel = NULL;
^
----------This seems to have been introduced recently by the REFRESH SEQUENCES
commit f0b3573.IIUC, this particular case is harmless, but IMO it's still worth
fixing to avoid recurring doubts about potential issues every time it
appears.
Sounds reasonable and it appears to bring some code clarity as well.
I'll push this early next week unless someone thinks differently.
--
With Regards,
Amit Kapila.
On Fri, 28 Nov 2025 at 09:29, Peter Smith <smithpb2250@gmail.com> wrote:
I noticed that recently, a shadow warning has crept into the code I am building.
----------
subscriptioncmds.c:1125:30: warning: declaration of ‘rel’ shadows a
previous local [-Wshadow]
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
../../../src/include/nodes/pg_list.h:482:20: note: in definition of
macro ‘foreach_internal’
for (type pointer var = 0, pointer var##__outerloop = (type pointer) 1; \
^
subscriptioncmds.c:1125:3: note: in expansion of macro ‘foreach_ptr’
foreach_ptr(SubRemoveRels, rel, sub_remove_rels)
^
subscriptioncmds.c:934:11: warning: shadowed declaration is here [-Wshadow]
Relation rel = NULL;
^
----------This seems to have been introduced recently by the REFRESH SEQUENCES
commit f0b3573.IIUC, this particular case is harmless, but IMO it's still worth
fixing to avoid recurring doubts about potential issues every time it
appears.PSA patch v1 to address the warning.
Thanks, your changes look good to me, I have verified this by
including the -Wshadow option.
Regards,
Vignesh