Ranier Vilela <ranier_gyn@hotmail.com> writes:
> This is the first part of the variable shadow fixes.
> Basically it consists of renaming the variables in collision with the global ones, with the minimum change in the
semantics.
I don't think I'm actually on board with the goal here.
Basically, if we take this seriously, we're throwing away the notion of
nested variable scopes and programming as if we had just a flat namespace.
That wasn't any fun when we had to do it back in assembly-code days, and
I don't see a good reason to revert to that methodology today.
In a few of these cases, like the RedoRecPtr changes, there *might* be
an argument that there's room for confusion about whether the code could
have meant to reference the similarly-named global variable. But it's
just silly to make that argument in places like your substitution of
/days/ndays/ in date.c.
Based on this sample, I reject the idea that we ought to be trying to
eliminate this class of warnings just because somebody's compiler can be
induced to emit them. If you want to make a case-by-case argument that
particular situations of this sort are bad programming style, let's have
that argument by all means. But it needs to be case-by-case, not just
dropping a large patch on us containing a bunch of unrelated changes
and zero specific justification for any of them.
IOW, I don't think you've taken to heart Robert's upthread advice that
this needs to be case-by-case and based on literary not mechanical
considerations.
BTW, if we do go forward with changing the RedoRecPtr uses, I'm not
in love with "XRedoRecPtr" as a replacement parameter name; it conveys
nothing much, and the "X" prefix is already overused in that area of
the code. Perhaps "pRedoRecPtr" would be a better choice? Or maybe
make the local variables be all-lower-case "redorecptr", which would
fit well in context in places like
-RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
+RemoveXlogFile(const char *segname, XLogRecPtr XRedoRecPtr, XLogRecPtr endptr)
regards, tom lane