Hi, On 2019-12-09 11:59:23 -0500, Robert Haas wrote: > On Mon, Dec 9, 2019 at 11:48 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > I think it depends a lot on the particular identifiers in use. You > > mentioned examples like "i" and "lc", and I'd add other obviously > > nonce variable names like "oldcxt". I'm not particularly concerned > > about shadowing arising from somebody writing a five-line loop using > > a local "i" inside a much larger loop also using "i" --- yeah, in > > theory there could be an issue, but in practice there isn't. Being > > picky about that just adds difficulty when writing/reviewing a patch > > that adds such a five-line loop. > > I think I would take the contrary view here. I think reusing the same > variable names in a single function is confusing, and if I noticed it > while reviewing, I would ask for it to be changed. It's not a > five-alarm fire, but it's not good, either.
+1. For me it leaves mildly bad taste seing code like that. > > > To me, the grey > > > area is in conflicts between stuff in our code and stuff in system > > > header files. I'm not sure I'd want to try to have precisely 0 > > > conflicts with every crazy decision by every OS / libc maintainer out > > > there, and I suspect on that point at least we are in agreement. > > > > I believe the relevant C standards (and practice) are that random > > names exposed by system headers ought to start with some underscores. > > If there's a conflict there, it's a bug in the header and cause > > for a bug report to the OS vendor, not us. > > Sure. I mean we'd have to look at individual cases, but in general I agree. We do have a few files where we have names starting with underscores ourselves, imo not a great idea for most of them. > > Now, if a conflict of that sort exists and is causing a live bug in PG > > on a popular OS, then I'd likely be on board with adjusting our code > > to dodge the problem. But not with doing so just to silence a > > compiler warning. > > Sounds reasonable. FWIW, I've had bugs in code submitted to PG (both by myself and me merging other people's work IIRC) that were related to such naming conflicts. > > The only thing I think is really a substantial bug risk here is your > > point about our own macros referencing our own global variables. > > We might be better off fixing that in a localized way by establishing > > a policy that any such macros should be converted to static inlines. > > That would be a lot of work, but it would probably have some side > benefits, like making things more type-safe. It's also not always possible in C99, as we have plenty macros with essentially dynamic types. And there's no typeof() in standard C, unfortunately (C11's _Generic can help, but isn't great either). Greetings, Andres Freund
