Re: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> When I compile with gcc -O0, I get one warning with this:

> datetime.c: In function DateTimeParseError:
> datetime.c:3575:1: warning: noreturn function does return [enabled by default]

> That suggests that the compiler didn't correctly deduce that
> ereport(ERROR, ...) doesn't return. With -O1, the warning goes away.

Yeah, I am seeing this too. It appears to be endemic to the
local-variable approach, ie if we have

const int elevel_ = (elevel);
...
(elevel_ >= ERROR) ? pg_unreachable() : (void) 0

then we do not get the desired results at -O0, which is not terribly
surprising --- I'd not really expect the compiler to propagate the
value of elevel_ when not optimizing.

If we don't use a local variable, we don't get the warning, which
I take to mean that gcc will fold "ERROR >= ERROR" to true even at -O0,
and that it does this early enough to conclude that unreachability
holds.

I experimented with some variant ways of phrasing the macro, but the
only thing that worked at -O0 required __builtin_constant_p, which
rather defeats the purpose of making this accessible to non-gcc
compilers.

If we go with the local-variable approach, we could probably suppress
this warning by putting an abort() call at the bottom of
DateTimeParseError. It seems a tad ugly though, and what's a bigger
deal is that if the compiler is unable to deduce unreachability at -O0
then we are probably going to be fighting such bogus warnings for all
time to come. Note also that an abort() (much less a pg_unreachable())
would not do anything positive like give us a compile warning if we
mistakenly added a case that could fall through.

On the other hand, if there's only one such case in our tree today,
maybe I'm worrying too much.

regards, tom lane