Re: Missing errcode() in ereport

I wrote:
> 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.

Hearing no objections, I started to review Andres' patchset with
that plan in mind. I noted two things that I don't agree with:

1. I think we should write the ereport macro as

if (errstart(...)) \
__VA_ARGS__, errfinish(...); \

as I had it, not

if (errstart(...)) \
{ \
__VA_ARGS__; \
errfinish(...); \
} \

as per Andres. The reason is that I don't have as much faith in the
latter construct producing warnings for no-op expressions.

2. We cannot postpone the passing of the "domain" argument as Andres'
0003 patch proposes, because that has to be available to the auxiliary
error functions so they do message translation properly. Maybe it'd
be possible to finagle things to postpone translation to the very end,
but the provisions for errcontext messages being translated in a different
domain would make that pretty ticklish. Frankly I don't think it'd be
worth the complication. There is a clear benefit in delaying the passing
of filename (since we can skip that strchr() call) but beyond that it
seems pretty marginal.

Other than that it looks pretty good. I wrote some documentation
adjustments and re-split the patch into 0001, which is proposed for
back-patch into v12, and 0002 which would have to be HEAD only.

One thing I'm not totally sure about is whether we can rely on
this change:

-extern void errfinish(int dummy,...);
+extern void errfinish(void);

being fully ABI-transparent. One would think that as long as errfinish
doesn't inspect its argument(s) it doesn't matter whether any are passed,
but maybe somewhere there's an architecture where the possible presence
of varargs arguments makes for a significant difference in the calling
convention? We could leave that change out of the v12 patch if we're
worried about it.

regards, tom lane