Thread: SPI-header-files safe for C++-compiler
This is a request I posted in February. The thread was named "Writing triggers in C++". However I did not supply a patch then, and some people misunderstood my problem. I will try to explain it again: My problem is, I wrote some triggers in C using the SPI-API. Those triggers call some functions defined in an external C++ library. In order to use name-mangled functions, namespaces and C++-header-files, provided by this library, I have to use a C++ compiler to compile my trigger-functions. But the C++-compiler rejects to compile legal C code, because some of the included Postgres-headers, ie. postgres.h, executor/spi.h, commands/trigger.h, fmgr.h use a few C++ keywords to defined a some struct members and function arguments. The incriminating C++-keywords used in the Postgres-headers are: 'typeid', 'typename' and 'using'. It would do no harm to the Postgres-sources if these keywords would be replaced with a similar identifier. I wrote a patch which applies cleanly onto version 8.2.4 (and 8.2.3) and keeps the Postgres binary compatible to an unpatched version. I would appreciate to see this patch applied onto the Postgres-sources. Other authors using the SPI-API together with a C++-compiler would also benefit from this patch. Regards, Jacob
Attachment
On Thu, 2007-06-28 at 00:43 +0200, Jacob Rief wrote:
> But the C++-compiler rejects to compile legal C code,
> because some of the included Postgres-headers, ie. postgres.h,
> executor/spi.h, commands/trigger.h, fmgr.h use a few C++ keywords to
> defined a some struct members and function arguments. The incriminating
> C++-keywords used in the Postgres-headers are: 'typeid', 'typename' and
> 'using'.
>
> It would do no harm to the Postgres-sources if these keywords would be
> replaced with a similar identifier.
No objection, although it would be nice if we could find something nicer
to rename "using" to than "using_". What about "using_clause" or
"using_list"?
You also changed "namespace" => "name_space" in builtins.h; is that
necessary?
> I wrote a patch which applies cleanly onto version 8.2.4
Patches should be submitted against the CVS HEAD code: we wouldn't want
to apply a patch like this to the REL8_2_STABLE branch, in any case.
BTW, I notice the patch also adds 'extern "C" { ... }' statements to a
few random header files. Can't client programs do this before including
the headers, if necessary?
-Neil
Neil Conway wrote:
>
> BTW, I notice the patch also adds 'extern "C" { ... }' statements to a
> few random header files. Can't client programs do this before including
> the headers, if necessary?
>
>
>
He's used the usual "#ifdef __cplusplus" wrapper - isn't that good enough?
cheers
andrew
On Wed, 2007-06-27 at 20:51 -0400, Andrew Dunstan wrote: > He's used the usual "#ifdef __cplusplus" wrapper - isn't that good enough? Well, there's nothing wrong with it per se, I'm just wondering (1) what the proper set of headers to add it to is -- the patch just does a handful (2) whether we need to bother doing it at all -- can't clients do the extern "C" trick before including any Postgres headers? -Neil
Neil Conway <neilc@samurai.com> writes:
> On Wed, 2007-06-27 at 20:51 -0400, Andrew Dunstan wrote:
>> He's used the usual "#ifdef __cplusplus" wrapper - isn't that good enough?
> Well, there's nothing wrong with it per se, I'm just wondering (1) what
> the proper set of headers to add it to is -- the patch just does a
> handful (2) whether we need to bother doing it at all -- can't clients
> do the extern "C" trick before including any Postgres headers?
The whole thing has been proposed before and rejected before.
The patch as given merely renames some random identifiers that happen to
be keywords in some non-C language (thereby breaking not only a lot of
core backend code, which we can fix, but who knows what other
user-written extensions that we can't fix so easily). The *real*
problem here is that to make this useful, we have to buy into the
assumption that C++ should work in the backend. The error handling
assumptions are completely incompatible (setjmp and throw do not usually
interoperate), and there are a lot of other bits of infrastructure that
C++ expects that are not likely to be in a C-based executable either.
I might be willing to accept this patch if I thought it were the end of
the story, but I know that it is only the camel's nose poking under the
tent.
regards, tom lane
On Thu, 2007-28-06 at 01:15 -0400, Tom Lane wrote: > The patch as given merely renames some random identifiers that happen to > be keywords in some non-C language (thereby breaking not only a lot of > core backend code, which we can fix, but who knows what other > user-written extensions that we can't fix so easily). The fact is, any user-written extensions that depend on types defined in parsenodes.h and primnodes.h are going to get broken all the time *anyway*, so I don't see this as a major disadvantage. Doing "s/typename/type_name/g" is likely to be the least of your concerns if your extension integrates with Postgres that closely. It would be one thing if we made a significant effort to preserve internal API stability -- but we plainly do not. (See the varlena API changes made in 8.3 for an example of an API change that will break far more user-written extensions.) > The *real* problem here is that to make this useful, we have to buy into > the assumption that C++ should work in the backend. I agree that C++ in the backend is always going to be a little hokey, but (a) I don't agree that it is completely impossible (b) if we can make people's lives a bit easier, I don't see a good reason not to. Like it or not, people have called into C++ libraries from C UDFs in the past, and likely will do so in the future. > The error handling assumptions are completely incompatible (setjmp and > throw do not usually interoperate) AFAIK this is resolvable with some degree of pain: before entering C++ land, wrap the call site in a C++ exception handler, and before calling back into Postgres, use a PG_TRY() block to rethrow elog(ERROR) as a C++ exception (and then rethrow as an elog(ERROR) once you've unwound the C++ portion of the stack) ... hey, I didn't say it was clean ;-) It's also worth noting that some people use C++ as "C with classes", and disallow the use of exceptions, RTTI, and that sort of stuff. Calling into such code from the backend is marginally more sane. -Neil
Neil Conway <neilc@samurai.com> writes:
> On Thu, 2007-28-06 at 01:15 -0400, Tom Lane wrote:
>> The patch as given merely renames some random identifiers that happen to
>> be keywords in some non-C language ...
> The fact is, any user-written extensions that depend on types defined in
> parsenodes.h and primnodes.h are going to get broken all the time
> *anyway*, so I don't see this as a major disadvantage.
Sure, but we don't break them just on a whim. The bottom line here is
whether we are going to make a real commitment to making C++ usable as
a backend extension language --- and for the reasons I mentioned, that
would entail a lot more than renaming a few identifiers. It was already
pointed out upthread that wrapping the inclusions in extern "C" {...}
would fix the identifier part of the problem from the user side, so I do
not see the point of fixing it from our side unless we are prepared to
buy into a lot of other changes. A C++ writer who is unwilling to add
the extern{} bit around inclusions of C headers seems unlikely to "work
with us" as regards to error-throwing conventions, for instance.
regards, tom lane
On Thu, 2007-28-06 at 02:07 -0400, Tom Lane wrote:
> It was already pointed out upthread that wrapping the inclusions in
> extern "C" {...} would fix the identifier part of the problem from
> the user side
No, my point about extern "C" was that I don't think we need to add it
to the Postgres headers. As far as I can see, it doesn't help with the
identifier part of the problem:
% cat foo.cpp
extern "C" {
#include "postgres.h"
#include "commands/trigger.h"
}
int
main(void)
{
return 0;
}
% g++ -I$HOME/postgres/head/prefix/include/server foo.cpp
/home/neilc/postgres/head/prefix/include/server/nodes/primnodes.h:1078:
error: expected unqualified-id before 'using'
/home/neilc/postgres/head/prefix/include/server/nodes/primnodes.h:1078:
error: abstract declarator 'List*' used as declaration
/home/neilc/postgres/head/prefix/include/server/nodes/primnodes.h:1078:
error: expected ';' before 'using'
/home/neilc/postgres/head/prefix/include/server/nodes/parsenodes.h:167:
error: expected unqualified-id before 'typeid'
/home/neilc/postgres/head/prefix/include/server/nodes/parsenodes.h:238:
error: expected unqualified-id before 'typename'
[ ... and so on ... ]
-Neil
Tom Lane wrote:
> Sure, but we don't break them just on a whim. The bottom line here is
> whether we are going to make a real commitment to making C++ usable as
> a backend extension language --- and for the reasons I mentioned, that
> would entail a lot more than renaming a few identifiers. It was already
> pointed out upthread that wrapping the inclusions in extern "C" {...}
> would fix the identifier part of the problem from the user side, so I do
> not see the point of fixing it from our side unless we are prepared to
> buy into a lot of other changes. A C++ writer who is unwilling to add
> the extern{} bit around inclusions of C headers seems unlikely to "work
> with us" as regards to error-throwing conventions, for instance.
Adding an extern "C" arround the header files is not the problem, altough it would make life a little easier. I have
oftenseen it in C-only header files, and it will do no harm to anybody using a C-only compiler.
The real problem are the C++-keyword used in spi.h and trigger.h
The C++ compiler (tested with gcc 4.1.0) even complains, if C++ keywords are used inside extern "C" { ... } blocks. The
extern"C" only tells the compiler to switch off name-mangling - it does not allow any kind of plain-old C. With some
drawbacks,it is perfectly legal to use C++ features inside an extern "C" block.
I am speaking about 1 function argument, where it absolutely doesn't matter, how it is named in a header files.
And I am speaking about 5 struct member where it should not be a real problem for client libraries to rename them to
somethingvery similar.
If there is any chance to get this patch applied, I will of course regenerate it out of the HEAD-revision of the CVS
repository.If there is any objection about the terminology of the renamed keywords, lets discuss. If, however clients
ofspi.h and trigger.h shall never, ever even think about using a C++ compiler in their projects, I will stop any
discussionhere and patch each newer version of Postgres for my own private fork.
Regards, Jacob
--
GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free SMS.
Alle Infos und kostenlose Anmeldung: http://www.gmx.net/de/go/freemail
On Thu, 2007-28-06 at 01:15 -0400, Tom Lane wrote:
> Sure, but we don't break them just on a whim. The bottom line here is
> whether we are going to make a real commitment to making C++ usable as
> a backend extension language --- and for the reasons I mentioned, that
> would entail a lot more than renaming a few identifiers. It was already
> pointed out upthread that wrapping the inclusions in extern "C" {...}
> would fix the identifier part of the problem from the user side, so I do
No, wrong. Adding extern "C" does not fix the C++-keywords as identifiers
problem. Adding extern "C" only tells the compiler to switch off
name-mangling. A C++-compiler does not allow any kind of plain-old C in
such blocks. With some drawbacks, it is even perfectly legal to use some
C++ features inside an extern "C" block.
> not see the point of fixing it from our side unless we are prepared to
> buy into a lot of other changes. A C++ writer who is unwilling to add
> the extern{} bit around inclusions of C headers seems unlikely to "work
> with us" as regards to error-throwing conventions, for instance.
I would have never posted any patch, if just adding extern "C" { } would
have solved my problems, but as mentioned above, it doesn't.
Jacob
--
Psssst! Schon vom neuen GMX MultiMessenger gehört?
Der kanns mit allen: http://www.gmx.net/de/go/multimessenger
On 6/29/07, Jacob Rief <jacob.rief@gmx.at> wrote:
> On Thu, 2007-28-06 at 01:15 -0400, Tom Lane wrote:
> > Sure, but we don't break them just on a whim. The bottom line here is
> > whether we are going to make a real commitment to making C++ usable as
> > a backend extension language --- and for the reasons I mentioned, that
> > would entail a lot more than renaming a few identifiers. It was already
> > pointed out upthread that wrapping the inclusions in extern "C" {...}
> > would fix the identifier part of the problem from the user side, so I do
>
> No, wrong. Adding extern "C" does not fix the C++-keywords as identifiers
> problem. Adding extern "C" only tells the compiler to switch off
> name-mangling. A C++-compiler does not allow any kind of plain-old C in
> such blocks. With some drawbacks, it is even perfectly legal to use some
> C++ features inside an extern "C" block.
I think you are better off wrpapping your C++ code to small
pure-C module and export only that to Postgres-interfacing
C code. Thus you keep away making C++ seeing Postgres code.
It is really a mistake to think C as subset of C++. They are
really different languages. C-as-subset-of-C++ works only
with trivial C code. If you have big, old, real-life codebase
like Postgres, it really does not pay off to force it to comply
with C++ rules. Just to avoid the work of writing intermediate
wrapper code.
--
marko
On Thu, 2007-06-28 at 12:08 +0200, Jacob Rief wrote: > If there is any chance to get this patch applied, I will of course regenerate > it out of the HEAD-revision of the CVS repository. I don't see a reason to reject the patch. All the arguments about why using C++ in the backend is ill-advised are well-taken, but the patch does *not* require "making a real commitment to making C++ usable as a backend extension language", it just obviates the need for some people to patch the source. So +1 from me. -Neil
Neil Conway <neilc@samurai.com> writes:
> I don't see a reason to reject the patch. All the arguments about why
> using C++ in the backend is ill-advised are well-taken, but the patch
> does *not* require "making a real commitment to making C++ usable as a
> backend extension language", it just obviates the need for some people
> to patch the source.
... at the cost of forcing other people to patch their source. If this
were just an internal backend change it'd be OK, but by definition the
patch is changing APIs that third-party code may depend on. That's why
I think there needs to be a stronger argument than "might as well do
it", and that stronger argument has got to discuss whether we are really
supporting C++ in the backend.
There's also a slippery-slope problem: if we accept making these headers
C++-clean, why not every other backend header? Once we buy into the
principle, you can bet that we'll get requests to sanitize every header
that's of any interest. So I'd want to see some estimate of how many
changes that entails, not just fixing the set of things that spi.h
depends on.
regards, tom lane
> Neil Conway <neilc@samurai.com> writes: > > I don't see a reason to reject the patch. All the arguments about why > > using C++ in the backend is ill-advised are well-taken, but the patch > > does *not* require "making a real commitment to making C++ usable as > a > > backend extension language", it just obviates the need for some > people > > to patch the source. > > ... at the cost of forcing other people to patch their source. If this > were just an internal backend change it'd be OK, but by definition the > patch is changing APIs that third-party code may depend on. That's why > I think there needs to be a stronger argument than "might as well do > it", and that stronger argument has got to discuss whether we are > really > supporting C++ in the backend. > > There's also a slippery-slope problem: if we accept making these > headers > C++-clean, why not every other backend header? Once we buy into the > principle, you can bet that we'll get requests to sanitize every header > that's of any interest. So I'd want to see some estimate of how many > changes that entails, not just fixing the set of things that spi.h > depends on. > > regards, tom lane > I've tried compiling the entire backend in C++ in the past, just to see how much of a problem it would be. The effort to fix the header files is minimal, just a few global search-and-replace operations for C++ keywords such as "typename", "typeid", "namespace" and "using", and doing the appropriate stuff to use the native C++ bool, true, and false. After that, it's mostly a matter of changing places that rely on implicit casts from void * to add explicit casts. One day of effort at most. So, the programmer effort is quite small, but it does change quite a few .c files (especially "typename"). And in the end, you can't actually run full C++ code unless you do something about the setjmp longjmp (TRY() etc), as they are incompatible with C++ destructors, although you could use C++ if you avoided constructors/destructors/try-catch/etc. Here is what I think is a complete list of C++ keywords not in C (most are not using in Postgres): asm dynamic_cast namespace reinterpret_cast try bool explicit new static_cast typeid catch false operator template typename class friend private this using const_cast inline public throw virtual delete mutable protected true wchar_t Overall, I'm in favor of the change. I don't think the effort to "sanitize" all the headers is difficult at all.
"Chuck McDevitt" <cmcdevitt@greenplum.com> writes:
> I've tried compiling the entire backend in C++ in the past, just to see
> how much of a problem it would be.
Just for fun, I tried that experiment tonight on CVS HEAD: I used g++
from a current Fedora 6 installation (g++ Red Hat 4.1.1-51) instead of
the usual C compiler. It appears that the OP's patch is nearly complete
in terms of struct fields that'd have to be renamed: I find
src/include/nodes/memnodes.h:
MemoryContextMethods.delete
src/include/nodes/parsenodes.h:
TypeName.typeid
A_Const.typename
TypeCast.typename
ColumnDef.typename
XmlSerialize.typename
AlterDomainStmt.typename
CreateDomainStmt.typename
CreateEnumStmt.typename
src/include/nodes/primnodes.h:
JoinExpr.using
and it's reasonably arguable that none of these are very likely to be
referenced directly by third-party code; certainly not so likely as to
pose a bigger cross-version-compatibility problem than we routinely
create.
(I did not get as far as the PLs or contrib/, but those are even less
likely to be exporting anything that outside code would depend on.)
The problem that I see after trying the experiment, however, is that
actually building Postgres using a C++ compiler would require changes
enormously more invasive than just renaming some fields and function
argument names. And I have no confidence in our ability to *keep* the
headers C++-clean if there's not a buildfarm member out there building
it with a C++ compiler so it can gripe about re-introduction of C++
keywords. So I'm afraid the issue will just keep coming back.
Chuck mentioned the point that C++ rejects implicit casts from void*
to something else, but there are a lot of other problems, including
some that would require notational compromises I don't think we'd like
to make. Two examples:
* g++ rejects struct assignment if either source or destination is
accessed through a volatile pointer. We do that in a number of places,
mostly in xlog.c and bufmgr.c. Options I can see are not good:
1: don't use volatile (not acceptable)
2: cast away volatile (probably breaks the guarantee we want)
3: do the assignment explicitly field-by-field (sucks from a
maintenance point of view, not to mention legibility)
4: use memcpy (sucks for performance because structs are small,
and probably requires casting away volatile, see #2)
* I don't see how to make expression_tree_walker and
expression_tree_mutator work reasonably nicely; g++ is too picky about
the argument types of the walker/mutator function.
C++'s commandeering of "bool" might be an issue also; is it guaranteed
anywhere that that's the same size/alignment as "char"?
BTW, one problem I didn't understand at all was that g++ spit up on
bitand() and bitor() as function names. Those are not C++ keywords
to my knowledge; anyone have a theory?
Anyway, at this point my concern is not so much whether we could fix
it as what is the plan for keeping it fixed.
regards, tom lane
> > BTW, one problem I didn't understand at all was that g++ spit up on > bitand() and bitor() as function names. Those are not C++ keywords > to my knowledge; anyone have a theory? > I forgot these c++ keywords, as you never need to use them in a c++ program: and bitand compl not_eq or_eq xor_eq and_eq bitor not or xor They are equivalents of the operators for use on systems that don't have full character set support. Bitand is the same as &, and bitor is the same as |
> Tom Lane writes: > The problem that I see after trying the experiment, however, is that > actually building Postgres using a C++ compiler would require changes > enormously more invasive than just renaming some fields and function > argument names. And I have no confidence in our ability to *keep* the > headers C++-clean if there's not a buildfarm member out there building > it with a C++ compiler so it can gripe about re-introduction of C++ > keywords. So I'm afraid the issue will just keep coming back. I never was talking about compiling the whole Postgres-implementation with g++ (even if that would nice experience :) It'sonly about a few header-files, were it would even be legal (but dirty) to patch them after compilation, since they areabsolutely binary compatible. > Chuck mentioned the point that C++ rejects implicit casts from void* > to something else, but there are a lot of other problems, including > some that would require notational compromises I don't think we'd like > to make. Two examples: > * g++ rejects struct assignment if either source or destination is > accessed through a volatile pointer. We do that in a number of places, > mostly in xlog.c and bufmgr.c. Options I can see are not good: > 1: don't use volatile (not acceptable) > 2: cast away volatile (probably breaks the guarantee we want) > 3: do the assignment explicitly field-by-field (sucks from a > maintenance point of view, not to mention legibility) > 4: use memcpy (sucks for performance because structs are small, > and probably requires casting away volatile, see #2) > > * I don't see how to make expression_tree_walker and > expression_tree_mutator work reasonably nicely; g++ is too picky about > the argument types of the walker/mutator function. Only header files shall be C++ compatible. There is no reason and no need to make the whole implementation C++-conform. > Anyway, at this point my concern is not so much whether we could fix > it as what is the plan for keeping it fixed. A best practice statement, telling developers not use C++-keywords as identifiers in header-files. Regards, Jacob -- GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free SMS. Alle Infos und kostenlose Anmeldung: http://www.gmx.net/de/go/freemail
"Jacob Rief" <jacob.rief@gmx.at> writes:
>> Tom Lane writes:
>> Anyway, at this point my concern is not so much whether we could fix
>> it as what is the plan for keeping it fixed.
> A best practice statement, telling developers not use C++-keywords as identifiers in header-files.
That will have exactly zero effect. As was shown upthread, even
everyday C++ programmers don't remember what all the extra keywords are.
Guys who mainly code C are very unlikely to get this right if there's no
automatic check being applied.
regards, tom lane
Tom Lane writes: > That will have exactly zero effect. As was shown upthread, even > everyday C++ programmers don't remember what all the extra keywords are. > Guys who mainly code C are very unlikely to get this right if there's no > automatic check being applied. Just for curiosity, I would like to ask something. libpqxx is based on libpq, and thus includes headers-files from libpq. These header-files are C++-safe, otherwise libpqxx would'nt compile. Whats the point in having a two-class-society of headers files, some of which are compatible for C++-compilers (the client-side), and some which aren't (the server-side). My problem is, that I have to wrap a few C++-classes as internal Postgres-types in my code, and thus must include C++-header-files as well as server-side Postgres-header-files. Therefore I must use a C++-compiler, and the only problems so far, is the use of C++-keywords on the server-side-Postgres-headers. Regards, Jacob -- Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kanns mit allen: http://www.gmx.net/de/go/multimessenger
"Jacob Rief" <jacob.rief@gmx.at> writes:
> Just for curiosity, I would like to ask something.
> libpqxx is based on libpq, and thus includes headers-files from libpq.
> These header-files are C++-safe, otherwise libpqxx would'nt compile.
Well, if they are, it's only by chance, because there isn't much of
anything verifying that they are safe :-(.
> Whats the point in having a two-class-society of headers files, some
> of which are compatible for C++-compilers (the client-side), and some
> which aren't (the server-side).
It makes sense for libpq to be able to execute within a C++ application.
As was already debated, it's not nearly as clear that it makes sense to
try to use C++ for backend modules.
In any case, you are not helping here. I asked for a technical solution
to a specific problem, and you are giving me political arguments.
regards, tom lane
Tom Lane writes: > It makes sense for libpq to be able to execute within a C++ application. > As was already debated, it's not nearly as clear that it makes sense to > try to use C++ for backend modules. That correct. But I have to define some types using external libraries. > In any case, you are not helping here. I asked for a technical solution > to a specific problem, and you are giving me political arguments. How could a technical solution look like. Shall I write a test-programm to compile those header-files with g++ and put AC_CHECK_PROGS into configure.in to check if the build-system has installed a C++-compiler. Isn't that too exaggerated? Who wants to install a C++-compiler just to make a dummy compilation using a few header-files? Regards, Jacob -- Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kanns mit allen: http://www.gmx.net/de/go/multimessenger
Tom Lane wrote: > "Jacob Rief" <jacob.rief@gmx.at> writes: >> Just for curiosity, I would like to ask something. >> libpqxx is based on libpq, and thus includes headers-files from libpq. >> These header-files are C++-safe, otherwise libpqxx would'nt compile. > > Well, if they are, it's only by chance, because there isn't much of > anything verifying that they are safe :-(. wild idea - we could add a bit of configure magic and a special makefile target and add a buildfarm stage for that and have it automatically check for these things ? Stefan
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> wild idea - we could add a bit of configure magic and a special makefile
> target and add a buildfarm stage for that and have it automatically
> check for these things ?
I had been idly toying with the idea of making a tool to check for C++
problems, when it occurred to me that we already have almost the right
thing: src/tools/pginclude/pgcompinclude is a script that checks that
each .h file in the system can be compiled without any other preceding
inclusions except postgres.h. If run with a C++ compiler instead of C,
seems it would accomplish the task we want.
As it stands, pgcompinclude seems to lack a number of things but they're
all fixable:
* hard-wired compiler name and flags
* hard-wired paths
* should use postgres_fe.h not postgres.h for frontend-usable
include files (would need a clearer policy for which those are)
* possible use of unportable idioms (not sure, needs checked)
Given a cleaned-up version of pgcompinclude, we could add a buildfarm
option to invoke it using a specified compiler and options. I don't
think we need every buildfarm member doing that but getting a
representative sample doing so would be a Good Thing. Even without
considering the C++ angle, this is an area that's easy to break
and having quicker feedback would be good. The current practice of
manually running pgcompinclude once in awhile lacks a lot, notably
coverage of different sets of compile options (see notes at the bottom
of src/tools/pginclude/README).
Anybody feel like working on that?
regards, tom lane
Added to TODO: * Add automated check for invalid C++ source code constructs http://archives.postgresql.org/pgsql-patches/2007-07/msg00056.php --------------------------------------------------------------------------- Tom Lane wrote: > Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > > wild idea - we could add a bit of configure magic and a special makefile > > target and add a buildfarm stage for that and have it automatically > > check for these things ? > > I had been idly toying with the idea of making a tool to check for C++ > problems, when it occurred to me that we already have almost the right > thing: src/tools/pginclude/pgcompinclude is a script that checks that > each .h file in the system can be compiled without any other preceding > inclusions except postgres.h. If run with a C++ compiler instead of C, > seems it would accomplish the task we want. > > As it stands, pgcompinclude seems to lack a number of things but they're > all fixable: > * hard-wired compiler name and flags > * hard-wired paths > * should use postgres_fe.h not postgres.h for frontend-usable > include files (would need a clearer policy for which those are) > * possible use of unportable idioms (not sure, needs checked) > > Given a cleaned-up version of pgcompinclude, we could add a buildfarm > option to invoke it using a specified compiler and options. I don't > think we need every buildfarm member doing that but getting a > representative sample doing so would be a Good Thing. Even without > considering the C++ angle, this is an area that's easy to break > and having quicker feedback would be good. The current practice of > manually running pgcompinclude once in awhile lacks a lot, notably > coverage of different sets of compile options (see notes at the bottom > of src/tools/pginclude/README). > > Anybody feel like working on that? > > regards, tom lane -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +