Hello. At Mon, 9 Dec 2019 01:30:33 +0000, Ranier Vilela <ranier_...@hotmail.com> wrote in > >I don't think I'm actually on board with the goal here. > Ok, I understand. > > >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 general I think the use global variables its a bad design. But I > understand the use.
The file-scoped variable is needed to be process-persistent in any way. If we inhibit them, the upper-modules need to create the persistent area instead, for example, by calling XLogInitGlobals() or such, which makes things messier. Globality doens't necessarily mean evil and there're reasons for -Wall doesn't warn the case. I believe we, and especially committers are not who should be kept away from knives for the reason that knives generally have a possibility to injure someone. > >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. > I would rather fix everything, including days name. I might be too accustomed there, but the functions that define overriding locals don't modify the local variables and only the functions that don't override the globals modifies the glboals. I see no significant confusion here. By the way changes like "conf_file" -> "conffile" seems really useless as a fix patch. > >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. > This is why I suggested activating the alert in the development and review > process, so that any cases that arose would be corrected very early. I don't think it contributes to the argument on programming style in any way. > >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. > Ok, right. > But I was working on the second class of shadow variables, which are local > variables, within the function itself, where the patch would lead to a > removal of the variable declaration, maintaining the same logic and > functionality, which would lead to better performance and reduction. of > memory usage as well as very small. > In that case, too, would it have to be case by case? > Wow, there are many and many shadow variables ... As Robert said, they are harmless as far as we notice. Actual bugs caused by variable overriding would be welcomed to fix. I don't believe "lead to better performance and reduction (of code?)" without an evidence since modern compilers I think are not so stupid. Even if any, performance change in such extent doesn't support the proposal to remove variable overrides that way. > >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 > pRedoRecPtr, It's perfect for me. Anyway I strongly object to the name 'pRedoRecPtr', which suggests as if it is a C-pointer to some variable. (And I believe we use Hungarian notation only if we don't have a better way...) LatestRedoRecPtr looks better to me. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
