RE: [Proposal] Level4 Warnings show many shadow vars - Mailing list pgsql-hackers

From Ranier Vilela
Subject RE: [Proposal] Level4 Warnings show many shadow vars
Date
Msg-id MN2PR18MB2927A03176C52BBA02D449AFE3580@MN2PR18MB2927.namprd18.prod.outlook.com
Whole thread Raw
In response to Re: [Proposal] Level4 Warnings show many shadow vars  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [Proposal] Level4 Warnings show many shadow vars  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers
>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.

>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.

>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
becorrected very early. 

>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
thepatch would lead to a removal of the variable declaration, maintaining the same logic and functionality, which would
leadto 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 ...

>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.

regards,
Ranier Vilela


pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: RE: [Proposal] Level4 Warnings show many shadow vars
Next
From: Craig Ringer
Date:
Subject: Re: ssl passphrase callback