Re: [Proposal] Level4 Warnings show many shadow vars

On Fri, Dec 6, 2019 at 7:59 AM Ranier Vilela <ranier_gyn@hotmail.com> wrote:
> What did the original author want, use the global variable or not use it by overriding the name.
> If it was to use the global variable, it will affect the behavior of the function, I believe.

Well, you haven't provided any examples, so it's hard to be sure, but
I suspect that the vast majority of these are not actually bugs, but
just name collisions that don't really matter. Some of them could even
be Windows-specific things. For example, if Windows - or any other
platform - happened to have a variable declared in a library header
file someplace that is relatively commonly used within PostgreSQL as a
local variable name (e.g. "lc"), it would produce tons of name
collisions, none of which would be bugs.

The thing is, it's well-known that this is not good programming
practice, and I doubt that any committer would intentionally commit
code that used the same variable name for a file-level global and a
local variable in that same file. Perhaps a few such cases have crept
in by accident, but I bet they are rare. What's probably more likely
is that somebody - either a PostgreSQL developer or a Microsoft
developer - carelessly exposed a global name that's not very
distinctive, and it then collided -- either then or later -- with some
local variables in various places within the PostgreSQL code. If those
are names exposed by PostgreSQL, we should just rename the global
variables we exposed to have more distinctive names. If they're
exposed by Microsoft, we don't have that option, so we either have to
rename the local variables that shadow them, or decide that we don't
care.

Based on previous discussion in this forum, my guess is that popular
sentiment will depend quite a bit on how reasonable it seems that
Microsoft chose to use the name in the first place. If there's an
"extern int i;" declaration someplace in a Windows header file, we are
not for that reason going to abandon our practice of using "i" for
loop variables; we're (probably) just going to say nasty things about
Microsoft and keep doing what we're doing. If there's an "extern int
__msft_ftw;" declaration in a Windows header file and for some reason
we've used that same name in our code, we're going to decide we were
dumb to use that as a name and change it. The amount of code churn
also plays a role. People will be reluctant to change thousands of
lines of PostgreSQL code to work around Microsoft-specific problems,
but if it's only a little bit of code then people won't mind very
much.

Maybe you want to post a few examples. It's hard to discuss in the abstract.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company