RE: [Proposal] Level4 Warnings show many shadow vars

De: Stephen Frost
Enviadas: Terça-feira, 10 de Dezembro de 2019 17:52

>There's multiple ways to get there though and I think what you're seeing
>is that the "just change it to something else" answer isn't necessairly
>going to be viewed as an improvement (or, at least, not enough of an
>improvement to accept the cost of the change).
Well, I was trying to apply another non-implied rule, "break nothing".

>Why not change the variables? Changes that also improve the code itself
>along with eliminating the shadowing of the global variable are going to
>be a lot easier to be accepted.
Contrary to what I was initially thinking, it seems to me that changing the names of global variables is more acceptable to the people of the project.

>Sure, but have you looked at how it's used? Instead of just renaming
>the numTables variables in the functions that accept it- could those
>variables just be removed instead of changing their name to make it look
>like they're something different when they aren't actually different?
No. I didn't look.

>I've only spent a bit of time looking at it, but it sure looks like the
>variables could just be removed, and doing so doesn't break the
>regression tests, which supports the idea that maybe there's a better
>way to deal with those particular variables rather than renaming them.
>Another approach to consider might be to move some global variables into
>structures that are then global with better names to indicate that's
>what they are.
It does not seem reasonable to me what you are asking.
Because as I was told here and I agree in part. I do not have the necessary knowledge of structures and logic to propose big changes.
For the work I set out to, find bugs and make minor performance improvements, I believe, can contribute safely and without ruining anything.
By just changing the names of variables to something consistent and readable, the goal will be done without break anything.
Who is best to make these changes, are the authors and reviewers. Once we no longer have the problem of shadow variables, we can turn on the alert without breaking automatic compilation, as Tom Lane is concerned.
That's why I'd like to fix all collisions of variables, even the dumbest.

>In short, a hack-and-slash patch that doesn't really spend much time
>considering the changes beyond "let's just change these to be different
>to avoid shadowing globals" isn't really a good way to go about
>addressing these cases and has a good chance of making things more
>confusing, not less.
This is totally contrary to what I think about it.

regards,
Ranier Vilela