Feedback-ID: id4a34324:Fastmail
Date: Wed, 3 Dec 2025 10:17:28 -0500
From: Andres Freund <andres@anarazel.de>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Cleanup shadows variable warnings, round 1
Message-ID: <ug6ny3be4sk7khawrekndfh4nnfwvghvhs7zj3rfdoydatr3hs@lv344e263jqd>
References: 
 <CAEoWx2kQ2x5gMaj8tHLJ3=jfC+p5YXHkJyHrDTiQw2nn2FJTmQ@mail.gmail.com>
 <c981068f-008a-4951-992a-4d2159b9d656@eisentraut.org>
 <CAEoWx2kjuQTQg=ZO76R7yKSj27zEHqehhL15UaK2D=vAw2JgYw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAEoWx2kjuQTQg=ZO76R7yKSj27zEHqehhL15UaK2D=vAw2JgYw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ug6ny3be4sk7khawrekndfh4nnfwvghvhs7zj3rfdoydatr3hs%40lv344e263jqd>
Precedence: bulk

Hi,

On 2025-12-03 10:28:36 +0800, Chao Li wrote:
> I know -Wshadow=compatible-local is not supported by all compilers, some
> compilers may fallback to -Wshadow and some may just ignore it. This patch
> set has cleaned up all shadow-variable warnings, once the cleanup is done,
> in theory, we should be able to enable -Wshadow.
> 
> 0001 - simple cases of local variable shadowing local variable by changing
> inner variable to for loop variable (also need to rename the for loop var)
> 0002 - simple cases of local variable shadowing local variable by renaming
>  inner variable
> 0003 - simple cases of local variable shadowing local variable by renaming
> outer variable. In this commit, outer local variables are used much less
> than inner variables, thus renaming outer is simpler than renaming inner.
> 0004 -  still local shadows local, but caused by a macro definition, only
> in inval.c
> 0005 - cases of global wal_level and wal_segment_size shadow local ones,
> fixed by renaming local variables
> 0006 - in xlogrecovery.c, some static file-scope variables shadow local
> variables, fixed by renaming local variables
> 0007 - cases of global progname shadows local, fixed by renaming local to
> myprogname
> 0008 - in file_ops.c, some static file-scope variables shadow local, fixed
> by renaming local variables
> 0009 - simple cases of local variables are shadowed by global, fixed by
> renaming local variables
> 0010 - a few more cases of static file-scope variables shadow local
> variables, fixed by renaming local variables
> 0011 - cases of global conn shadows local variables, fixed by renaming
> local conn to myconn
> 0012 - fixed shadowing in ecpg.header
> 0013 - fixed shadowing in all time-related modules. Heikki had a concern
> where there is a global named “days”, so there could be some discussions
> for this commit. For now, I just renamed local variables to avoid shadowing.

This seems like a *lot* of noise / backpatching pain for relatively little
gain.


> From 0cddee282a08c79214fa72a21a548b73cc6256fe Mon Sep 17 00:00:00 2001
> From: "Chao Li (Evan)" <lic@highgo.com>
> Date: Tue, 2 Dec 2025 13:45:05 +0800
> Subject: [PATCH v2 05/13] cleanup: avoid local wal_level and wal_segment_size
>  being shadowed by globals
> 
> This commit fixes cases where local variables named wal_level and
> wal_segment_size were shadowed by global identifiers of the same names.
> The local variables are renamed so they are no longer shadowed by the
> globals.
> 
> Author: Chao Li <lic@highgo.com>
> Discussion: https://postgr.es/m/CAEoWx2kQ2x5gMaj8tHLJ3=jfC+p5YXHkJyHrDTiQw2nn2FJTmQ@mail.gmail.com
> ---
>  src/backend/access/rmgrdesc/xlogdesc.c  | 18 +++++++++---------
>  src/backend/access/transam/xlogreader.c |  4 ++--
>  src/bin/pg_controldata/pg_controldata.c |  4 ++--
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
> index cd6c2a2f650..6241d87c647 100644
> --- a/src/backend/access/rmgrdesc/xlogdesc.c
> +++ b/src/backend/access/rmgrdesc/xlogdesc.c
> @@ -34,24 +34,24 @@ const struct config_enum_entry wal_level_options[] = {
>  };
>  
>  /*
> - * Find a string representation for wal_level
> + * Find a string representation for wallevel
>   */
>  static const char *
> -get_wal_level_string(int wal_level)
> +get_wal_level_string(int wallevel)
>  {
>  	const struct config_enum_entry *entry;
> -	const char *wal_level_str = "?";
> +	const char *wallevel_str = "?";

This sounds like it's talking about walls not, the WAL.

Greetings,

Andres Freund