Hi, On 2020-03-19 22:32:30 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > I wonder if it'd become a relevant backpatch pain if we started to have > > some ereports() without the additional parens in 13+. > > Yeah, it would be a nasty backpatch hazard. > > > Would it perhaps > > make sense to backpatch just the part that removes the need for the > > parents, but not the return type changes? > > I was just looking into that. It would be pretty painless to do it > in v12, but before that we weren't requiring C99. Having said that, > trolling the buildfarm database shows exactly zero active members > that don't report having __VA_ARGS__, in the branches where we test > that. (And pg_config.h.win32 was assuming that MSVC had that, too.) > > Could we get away with moving the compiler goalposts for the back > branches? I dunno, but it's a fact that we aren't testing anymore > with any compilers that would complain about unconditional use of > __VA_ARGS__. So it might be broken already and we wouldn't know it.
FWIW, I did grep for unprotected uses, and didn't find anything. > (I suspect the last buildfarm animal that would've complained about > this was pademelon, which I retired more than a year ago IIRC.) I guess a query that searches the logs backwards for animals without __VA_ARGS__ would be a good idea? > > We can also remove elog() support code now, because with __VA_ARGS__ > > support it's really just a wrapper for ereport(elevel, > > errmsg(__VA_ARGS_)). This is patch 0002. > > Yeah, something similar had occurred to me but I didn't write the patch > yet. Note it should be errmsg_internal(); Oh, right. > also, does the default for errcode come out the same? I think so - there's no distinct code setting sqlerrcode in elog_start/finish. That already relied on errstart(). > > I wonder if its worth additionally ensuring that errcode, errmsg, > > ... are only called within errstart/errfinish? > > Meh. That's wrong at least for errcontext(), and I'm not sure it's > really worth anything to enforce it for the others. Yea, I'm not convinced either. Especially after changing the err* return types to void, as that presumably will cause errors with a lot of incorrect parens, e.g. about a function with void return type as an argument to errmsg(). > I think the key decision we'd have to make to move forward on this > is to decide whether it's still project style to prefer the extra > parens, or whether we want new code to do without them going > forward. If we don't want to risk requiring __VA_ARGS__ for the > old branches then I'd vote in favor of keeping the parens as > preferred style, at least till v11 is out of support. Agreed. > If we do change that in the back branches then there'd be reason to > prefer to go without parens. New coders might still be confused about > why there are all these calls with the useless parens, though. That seems to be an acceptable pain, from my POV. Greetings, Andres Freund
