Andres Freund <andres(at)2ndquadrant(dot)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

In response to

• Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4) at 2013-01-13 17:51:26 from Andres Freund

pgsql-hackers by date

Next:	From: Andres Freund	Date: 2013-01-13 19:06:37
	Subject: Re: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Previous:	From: Andrew Dunstan	Date: 2013-01-13 18:18:58
	Subject: psql binary output