Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-01-12 15:39:16 -0500, Tom Lane wrote:
>> This is actually a disadvantage of the proposal to replace the abort()
>> calls with __builtin_unreachable(), too. The gcc boys interpret the
>> semantics of that as "if control reaches here, the behavior is
>> undefined"
> We could make add an Assert() additionally?
I see little value in that. In a non-assert build it will do nothing at
all, and in an assert build it'd just add code bloat.
I think there might be a good argument for defining pg_unreachable() as
abort() in assert-enabled builds, even if the compiler has
__builtin_unreachable(): that way, if the impossible happens, we'll find
out about it in a predictable and debuggable way. And by definition,
we're not so concerned about either performance or code bloat in assert
builds.
> Where my variant is:
> #define ereport_domain(elevel, domain, rest) \
> do { \
> const int elevel_ = elevel; \
> if (errstart(elevel_,__FILE__, __LINE__, PG_FUNCNAME_MACRO, domain))\
> { \
> errfinish rest; \
> } \
> if (elevel_>= ERROR) \
> pg_unreachable(); \
> } while(0)
This is the same implementation I'd arrived at after looking at
Heikki's. I think we should probably use this for non-gcc builds.
However, for recent gcc (those with __builtin_constant_p) I think that
my original suggestion is better, ie don't use a local variable but
instead use __builtin_constant_p(elevel) && (elevel) >= ERROR in the
second test. That way we don't have the problem with unreachability
tests failing at -O0. We may still have some issues with bogus warnings
in other compilers, but I think most PG developers are using recent gcc.
regards, tom lane
