Обсуждение: 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

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

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