Craig Ringer 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

Search Discussions

  • Andrew Dunstan at Dec 26, 2010 at 9:54 pm

    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
  • Peter Eisentraut at Dec 27, 2010 at 3:54 pm

    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.
  • Alvaro Herrera at Dec 27, 2010 at 4:13 pm

    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
  • Tom Lane at Dec 27, 2010 at 4:55 pm

    Peter Eisentraut 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
  • Alvaro Herrera at Dec 27, 2010 at 5:11 pm

    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
  • Tom Lane at Dec 27, 2010 at 5:51 pm

    Alvaro Herrera 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
  • Magnus Hagander at Dec 27, 2010 at 6:00 pm

    On Mon, Dec 27, 2010 at 18:50, 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.
    Could this at some point be platform dependent? If so, could it be run
    on the buildfarm?
  • Tom Lane at Dec 27, 2010 at 6:08 pm

    Magnus Hagander writes:
    On Mon, Dec 27, 2010 at 18:50, Tom Lane 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
  • Tom Lane at Dec 27, 2010 at 6:23 pm
    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
  • Peter Geoghegan at Dec 27, 2010 at 7:15 pm
    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
  • Tom Lane at Dec 27, 2010 at 7:19 pm

    Peter Geoghegan 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
  • Peter Geoghegan at Dec 27, 2010 at 7:48 pm

    On 27 December 2010 19:17, Tom Lane 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
  • Alvaro Herrera at Dec 27, 2010 at 8:04 pm

    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
  • Tom Lane at Dec 27, 2010 at 8:14 pm

    Alvaro Herrera 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 Geoghegan at Jan 2, 2011 at 10:51 pm
    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
  • Bruce Momjian at Jan 13, 2011 at 5:34 pm

    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. +
  • Andrew Dunstan at Dec 27, 2010 at 5:33 pm

    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
  • David Fetter at Dec 27, 2010 at 5:39 pm

    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
  • Peter Eisentraut at Dec 27, 2010 at 5:50 pm

    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.
  • Chris Browne at Dec 30, 2010 at 7:54 pm

    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>
    --
    ``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?"''

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedDec 26, '10 at 5:31p
activeJan 13, '11 at 5:34p
posts21
users9
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase