On 2013-01-11 16:28:13 -0500, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On 2013-01-11 16:16:58 -0500, Tom Lane wrote:
> >> Uh ... because it's *not* unreachable if elevel < ERROR. Otherwise we'd
> >> just mark errfinish as __attribute((noreturn)) and be done. Of course,
> >> that's a gcc-ism too.
>
> > Well, I mean with the double evaluation risk.
>
> Oh, are you saying you don't want to make the __builtin_constant_p
> addition?
Yes, that was what I wanted to say.
> I'm not very satisfied with that answer. Right now, Peter's
> patch has added a class of bugs that never existed before 9.3, and yours
> would add more. It might well be that those classes are empty ... but
> *we can't know that*. I don't think that keeping a new optimization for
> non-gcc compilers is worth that risk. Postgres is already full of
> gcc-only optimizations, anyway.
ISTM that ereport() already has so strange behaviour wrt evaluation of
arguments (i.e doing it only when the message is really logged) that its
doesn't seem to add a real additional risk.
It also means that we potentially need to add more quieting returns et
al to keep the warning level on non-gcc compatible compilers low
(probably only msvc matters here) which we all don't see on our primary
compilers.
Anyway, youve got a strong opinion and I don't, so ...
Andres
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services