Re: Missing errcode() in ereport

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2020-03-19 22:32:30 -0400, Tom Lane wrote:
>> 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.

Yeah, I also grepped the v11 branch for that.

>> (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?

I did a more wide-ranging scan, looking at the 9.4 branch as far back
as 2015. Indeed, pademelon is the only animal that ever reported
not having __VA_ARGS__ in that timeframe.

So I've got mixed emotions about this. On the one hand, it seems
quite unlikely that anyone would ever object if we started requiring
__VA_ARGS__ in the back branches. On the other hand, it's definitely
not project policy to change requirements like that in minor releases.
Also the actual benefit might not be much. If anyone did mistakenly
back-patch a fix that included a paren-free ereport, the buildfarm
would point out the error soon enough.

I thought for a little bit about making the back branches define ereport
with "..." if HAVE__VA_ARGS and otherwise not, but if we did that then
the buildfarm would *not* complain about paren-free ereports in the back
branches. I think that would inevitably lead to there soon being some,
so that we'd effectively be requiring __VA_ARGS__ anyway. (I suppose
I could resurrect pademelon ... but who knows whether that old war
horse will keep working for another four years till v11 is retired.)

On balance I'm leaning towards keeping the parens as preferred style
for now, adjusting v12 so that the macro will allow paren omission
but we don't break ABI, and not touching the older branches. But
if there's a consensus to require __VA_ARGS__ in the back branches,
I won't stand in the way.

regards, tom lane