Re: Missing errcode() in ereport - Mailing list pgsql-hackers

Andres Freund <andres@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.
(I suspect the last buildfarm animal that would've complained about
this was pademelon, which I retired more than a year ago IIRC.)

> 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(); also, does the default
for errcode come out the same?

> I think it might also be a good idea to move __FILE__, __LINE__,
> PG_FUNCNAME_MACRO, domain from being parameters to errstart to
> errfinish. For elevel < ERROR its sad that we currently pass them even
> if we don't emit the message.  This is patch 0003.

Oh, that's a good idea that hadn't occurred to me.

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

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

            regards, tom lane