#include <funcapi.h>

On 12/26/2010 02:14 PM, Elliot Chance wrote:

In file included from /usr/include/pgsql/server/access/heapam.h:21,
from /usr/include/pgsql/server/nodes/execnodes.h:18,
from /usr/include/pgsql/server/executor/execdesc.h:18,
from /usr/include/pgsql/server/executor/executor.h:17,
from /usr/include/pgsql/server/funcapi.h:21,
from xapian.cpp:4:
/usr/include/pgsql/server/nodes/primnodes.h:1155: error: expected unqualified-id before �using�

You've neglected to mention which version of Pg you're compiling
against, so that line number means nothing. What's the offending line of
code and the surrounding few lines in primnodes.h?

In any case, I think it's very likely the issue is a C/C++
incompatibility in the Pg headers. It fails for me in a different place
using Pg 9.1git and g++ 4.5, complaining about the use of "private" as
an identifier in fmgr.h, because it's a keyword in C++.

This is one of the many reasons you should keep any code that touches
the postgres innards as pure C, and call into your C++ code via "extern
C" functions. The Pg headers aren't really C++ safe. Neither is Pg's
longjmp-based error handling, which really won't mix well with
exceptions or with stack-based objects that have dtors.

--
Craig Ringer

Heres an interesting hack I just thought of that works:

extern "C" {
#include <postgres.h>
#include <fmgr.h>
#define using _using
#define typeid _typeid
#define typename _typename
#include <funcapi.h>
#undef using
#undef typeid
#undef typename

#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif
};

Now there is no collision with the C++ keywords.

On 26/12/2010, at 5:14 PM, Craig Ringer wrote:

Craig Ringer <craig@postnewspapers.com.au> writes:

On 12/26/2010 02:14 PM, Elliot Chance wrote:

/usr/include/pgsql/server/nodes/primnodes.h:1155: error: expected unqualified-id before �using�

You've neglected to mention which version of Pg you're compiling
against, so that line number means nothing. What's the offending line of
code and the surrounding few lines in primnodes.h?

I'm betting 8.4, because there is a field named "using" on that line in
8.4 ...

In any case, I think it's very likely the issue is a C/C++
incompatibility in the Pg headers. It fails for me in a different place
using Pg 9.1git and g++ 4.5, complaining about the use of "private" as
an identifier in fmgr.h, because it's a keyword in C++.

We did clean up C++ keyword uses in the header files in 9.0, but your
report shows it's already gotten broken again. I'm disinclined to fix
it unless someone steps up to create an automated test that will get run
reasonably often. We had that discussion when the patch to rename
keyword-named fields was proposed, and nothing got done, and the current
state of affairs is the entirely predictable result.

regards, tom lane

On 12/26/2010 12:31 PM, Tom Lane wrote:

In any case, I think it's very likely the issue is a C/C++
incompatibility in the Pg headers. It fails for me in a different place
using Pg 9.1git and g++ 4.5, complaining about the use of "private" as
an identifier in fmgr.h, because it's a keyword in C++.

We did clean up C++ keyword uses in the header files in 9.0, but your
report shows it's already gotten broken again. I'm disinclined to fix
it unless someone steps up to create an automated test that will get run
reasonably often. We had that discussion when the patch to rename
keyword-named fields was proposed, and nothing got done, and the current
state of affairs is the entirely predictable result.

Here's a script to play with. It needs some tuning but it's a start. Run
in the top level directory (where configure etc live).

Sample output:

found new in src/timezone/private.h
found private in src/include/fmgr.h
found not in src/include/libpq/pqcomm.h
found typeid in src/include/access/htup.h
found bitor, bitand in src/include/catalog/pg_operator.h
found bitor, bitand in src/include/catalog/pg_proc.h
found bitand in src/include/catalog/pg_aggregate.h
found using in src/include/catalog/indexing.h
found inline in src/include/nodes/pg_list.h
found or in src/include/port/win32.h
found or in src/include/port/cygwin.h
found typeid in src/include/parser/parse_type.h
found inline in src/include/portability/instr_time.h
found inline in src/include/utils/palloc.h
found bitor, bitand in src/include/utils/varbit.h
found wchar_t in src/include/mb/pg_wchar.h
found inline, not, using, this, asm in src/include/storage/s_lock.h
found new in src/interfaces/ecpg/preproc/type.h
found new in src/pl/plpgsql/src/plpgsql.h
found namespace in src/bin/pg_dump/pg_dump.h
found namespace, public in src/bin/pg_dump/pg_backup_archiver.h
found namespace in src/bin/pg_dump/pg_backup.h

cheers

andrew

On sön, 2010-12-26 at 12:31 -0500, Tom Lane wrote:

We did clean up C++ keyword uses in the header files in 9.0, but your
report shows it's already gotten broken again. I'm disinclined to fix
it unless someone steps up to create an automated test that will get
run reasonably often. We had that discussion when the patch to rename
keyword-named fields was proposed, and nothing got done, and the
current state of affairs is the entirely predictable result.

src/tools/pginclude/cpluspluscheck

What's missing is to automate this, but it's unclear in what context,
and perhaps also to what extend this should be a hard requirement.

Excerpts from Peter Eisentraut's message of lun dic 27 12:54:16 -0300 2010:

On sön, 2010-12-26 at 12:31 -0500, Tom Lane wrote:

We did clean up C++ keyword uses in the header files in 9.0, but your
report shows it's already gotten broken again. I'm disinclined to fix
it unless someone steps up to create an automated test that will get
run reasonably often. We had that discussion when the patch to rename
keyword-named fields was proposed, and nothing got done, and the
current state of affairs is the entirely predictable result.

src/tools/pginclude/cpluspluscheck

What's missing is to automate this, but it's unclear in what context,
and perhaps also to what extend this should be a hard requirement.

Maybe we could mention it in src/tools/RELEASE_CHANGES

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Peter Eisentraut <peter_e@gmx.net> writes:

src/tools/pginclude/cpluspluscheck

Ah, I'd forgotten that.

What's missing is to automate this, but it's unclear in what context,
and perhaps also to what extend this should be a hard requirement.

After a bit of experimentation, I can say that this is better than
Andrew's hack, but it's still a good distance shy of something that
should be automated or treated as a hard requirement. The problem with
it is that it doesn't know anything about inclusion-order restrictions;
and to make matters worse, the actual order in which things are included
will vary for different users, because of the unspecified order in which
'find' will find things.

[ lightbulb ] ... although we could improve that quite a bit if we
processed each .h file separately instead of insisting on smashing
everything into one compilation. Let me go try that.

regards, tom lane

Excerpts from Tom Lane's message of lun dic 27 13:54:56 -0300 2010:

[ lightbulb ] ... although we could improve that quite a bit if we
processed each .h file separately instead of insisting on smashing
everything into one compilation. Let me go try that.

FWIW I have this patch lingering about that I wrote months ago, to check
for header problems (not C++ stuff, just things like forgetting to
include some necessary header in some other header). Since it needs a
lot of polish (needs to ignore certain headers, and avoid leave
lingering files around), I didn't commit it; and I haven't updated it to
the new Make recursive stuff, either. Maybe someone else knows what to
do with it, though.

*** a/src/include/Makefile
--- b/src/include/Makefile
*************** uninstall:
*** 60,65 ****
--- 60,72 ----
  # heuristic...
  	rm -rf $(addprefix '$(DESTDIR)$(includedir_server)'/, $(SUBDIRS) *.h)

+ check:
+ 	for dir in $(SUBDIRS); do \
+ 		for header in `find $(srcdir)/$$dir -type f -name \*.h`; do \
+ 			echo $$header; \
+ 			$(CC) $(CFLAGS) $(CPPFLAGS) -include postgres.h -o $$dir/`basename $$header .h`.gch $$header; \
+ 		done; \
+ 	done

clean:
rm -f utils/fmgroids.h parser/gram.h utils/probes.h catalog/schemapg.h

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On 12/27/2010 11:54 AM, Tom Lane wrote:

After a bit of experimentation, I can say that this is better than
Andrew's hack, but it's still a good distance shy of something that
should be automated or treated as a hard requirement.

I'm always happy if someone produces something better than I did :-)

On a more general point, it would be useful to have some infrastructure
for running quality checks like this and publishing the results. We
should be way beyond the point where we rely on individuals doing this
sort of stuff.

cheers

andrew

On Mon, Dec 27, 2010 at 12:33:00PM -0500, Andrew Dunstan wrote:

On 12/27/2010 11:54 AM, Tom Lane wrote:

After a bit of experimentation, I can say that this is better than
Andrew's hack, but it's still a good distance shy of something that
should be automated or treated as a hard requirement.

I'm always happy if someone produces something better than I did :-)

On a more general point, it would be useful to have some
infrastructure for running quality checks like this and publishing
the results. We should be way beyond the point where we rely on
individuals doing this sort of stuff.

This sounds like an excellent early candidate for the bitrot farm.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Tom Lane's message of lun dic 27 13:54:56 -0300 2010:

[ lightbulb ] ... although we could improve that quite a bit if we
processed each .h file separately instead of insisting on smashing
everything into one compilation. Let me go try that.

FWIW I have this patch lingering about that I wrote months ago, to check
for header problems (not C++ stuff, just things like forgetting to
include some necessary header in some other header). Since it needs a
lot of polish (needs to ignore certain headers, and avoid leave
lingering files around), I didn't commit it; and I haven't updated it to
the new Make recursive stuff, either.

src/tools/pginclude/ already contains several scripts for this sort of
thing. Bruce runs them by hand occasionally, although I just found out
that he's evidently not run the does-each-header-compile-standalone
test in awhile. It would probably pay to automate these.

regards, tom lane

On mån, 2010-12-27 at 12:33 -0500, Andrew Dunstan wrote:

On a more general point, it would be useful to have some
infrastructure for running quality checks like this and publishing the
results. We should be way beyond the point where we rely on
individuals doing this sort of stuff.

I had a Hudson service set up for things like this, but the hosting was
unreliable and then the thing faded away. I could try to revive it.

On Mon, Dec 27, 2010 at 18:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Tom Lane's message of lun dic 27 13:54:56 -0300 2010:

[ lightbulb ] ... although we could improve that quite a bit if we
processed each .h file separately instead of insisting on smashing
everything into one compilation.  Let me go try that.

FWIW I have this patch lingering about that I wrote months ago, to check
for header problems (not C++ stuff, just things like forgetting to
include some necessary header in some other header).  Since it needs a
lot of polish (needs to ignore certain headers, and avoid leave
lingering files around), I didn't commit it; and I haven't updated it to
the new Make recursive stuff, either.

src/tools/pginclude/ already contains several scripts for this sort of
thing.  Bruce runs them by hand occasionally, although I just found out
that he's evidently not run the does-each-header-compile-standalone
test in awhile.  It would probably pay to automate these.

Could this at some point be platform dependent? If so, could it be run
on the buildfarm?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus@hagander.net> writes:

On Mon, Dec 27, 2010 at 18:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

src/tools/pginclude/ already contains several scripts for this sort of
thing. �Bruce runs them by hand occasionally, although I just found out
that he's evidently not run the does-each-header-compile-standalone
test in awhile. �It would probably pay to automate these.

Could this at some point be platform dependent? If so, could it be run
on the buildfarm?

It's certainly possible that someone could use a C++ keyword in a
platform-specific, or feature-specific, part of an include file.
The current form of this script will only find problems in active
code; so running it on the buildfarm would be indicated if we want
to have real confidence that there are no problems.

regards, tom lane

BTW, the cpluspluscheck script invokes g++ with -fno-operator-names,
saying

# -fno-operator-names omits the definition of bitand and bitor, which
# collide with varbit.h. Could be fixed, if one were so inclined.

I just confirmed that those two function definitions are the only issues
that currently show up if one removes the switch. Now, I'm not that
concerned about whether C++ users can include varbit.h ... but if we're
really going to use this technique to check whether C++ can include
headers, I think we've got to get rid of that switch, or we'll get
bitten elsewhere.

I propose renaming bitand() and bitor() to bit_and and bit_or() ...
any objections?

regards, tom lane

I hope that we don't make the mistake of not checking for collisions
with C++0x keywords, for which GCC 4.3+ has partial support. The new
standard is almost complete, so it will probably become a lot more
relevant soon. There are quite a few new keywords in C++0x, including:

constexpr
decltype
nullptr
static_assert

Perhaps we should add -std=c++0x to the g++ command in
cpluspluscheck.sh . Since C++0x is “almost 100-percent compatible with
the existing Standard C++” according to no less an authority than
Bjarne Stroustrup, this seems sensible.

--
Regards,
Peter Geoghegan

Peter Geoghegan <peter.geoghegan86@gmail.com> writes:

I hope that we don't make the mistake of not checking for collisions
with C++0x keywords, for which GCC 4.3+ has partial support. The new
standard is almost complete, so it will probably become a lot more
relevant soon. There are quite a few new keywords in C++0x, including:

[ shrug... ] If it's not a keyword according to popularly available
tools, then I really have zero interest in worrying about it. This
is an exercise in making the headers useful in practice, not in academic
standards conformance.

regards, tom lane

On 27 December 2010 19:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[ shrug... ]  If it's not a keyword according to popularly available
tools, then I really have zero interest in worrying about it.  This
is an exercise in making the headers useful in practice, not in academic
standards conformance.

It isn't academic and I'm not just being pedantic, because the
standard introduces many new, useful features. A lot of popular C++
libraries optionally use C++0x through the use of conditional
compilation. For example, my distro's libstdc++ standard library
(which is mostly header-based due to the fact that it is mostly
comprised of templates and inline functions) has many #ifdefs, so that
things like move constructors (a big performance win for standard
library containers) are available. It just seems prudent to assume
that if any of these pg headers are being included in C++ TUs, they
might well be using C++0x.

--
Regards,
Peter Geoghegan

Excerpts from Peter Geoghegan's message of lun dic 27 16:13:33 -0300 2010:

I hope that we don't make the mistake of not checking for collisions
with C++0x keywords, for which GCC 4.3+ has partial support. The new
standard is almost complete, so it will probably become a lot more
relevant soon. There are quite a few new keywords in C++0x, including:

constexpr
decltype
nullptr
static_assert

I think only constexpr is being currently used from this list, and it's
easily fixed because it's not exposed beyond a single file.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Peter Geoghegan's message of lun dic 27 16:13:33 -0300 2010:

constexpr
decltype
nullptr
static_assert

I think only constexpr is being currently used from this list, and it's
easily fixed because it's not exposed beyond a single file.

If you're looking at the usage in predtest.c, it's not an issue since
it's not in a header file. There's no ambition here to make our .c
files compilable in C++.

regards, tom lane

peter_e@gmx.net (Peter Eisentraut) writes:

On mån, 2010-12-27 at 12:33 -0500, Andrew Dunstan wrote:

On a more general point, it would be useful to have some
infrastructure for running quality checks like this and publishing
the results. We should be way beyond the point where we rely on
individuals doing this sort of stuff.

I had a Hudson service set up for things like this, but the hosting
was unreliable and then the thing faded away. I could try to revive
it.

Careful, Oracle has been trying to claim proprietary ownership of
that...
<http://hudson-labs.org/content/whos-driving-thing&gt;
--
``God decided to take the devil to court and settle their differences
once and for all. When Satan heard of this, he grinned and said, "And
just where do you think you're going to find a lawyer?"''

I believe that Dave Page wants to move to building pg for windows
using visual C++ 2010 some time this year. That alone may be enough of
a reason to check for C++0x keywords in headers:

http://blogs.msdn.com/b/vcblog/archive/2010/04/06/c-0x-core-language-features-in-vc10-the-table.aspx

I think that there is likely to be an expectation that the same
compiler that is used to build pg should be able to include pg headers
in C++ TUs.

--
Regards,
Peter Geoghegan

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Excerpts from Tom Lane's message of lun dic 27 13:54:56 -0300 2010:

[ lightbulb ] ... although we could improve that quite a bit if we
processed each .h file separately instead of insisting on smashing
everything into one compilation. Let me go try that.

FWIW I have this patch lingering about that I wrote months ago, to check
for header problems (not C++ stuff, just things like forgetting to
include some necessary header in some other header). Since it needs a
lot of polish (needs to ignore certain headers, and avoid leave
lingering files around), I didn't commit it; and I haven't updated it to
the new Make recursive stuff, either.

src/tools/pginclude/ already contains several scripts for this sort of
thing. Bruce runs them by hand occasionally, although I just found out
that he's evidently not run the does-each-header-compile-standalone
test in awhile. It would probably pay to automate these.

It is true I have not run those tests in a while.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +