Skip site navigation (1) Skip section navigation (2)

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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Date: 2013-01-13 00:47:18
Message-ID: 27074.1358038038@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackers
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


In response to

Responses

pgsql-hackers by date

Next:From: Amit kapilaDate: 2013-01-13 04:49:38
Subject: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Previous:From: Noah MischDate: 2013-01-13 00:28:51
Subject: Re: logical changeset generation v3 - comparison to Postgres-Rchange set format

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group