Ranier Vilela <ranier_...@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