static assertions in C++
Commit df1a699e5ba3232f373790b2c9485ddf720c4a70 introduced a
StaticAssertStmt() into a header file, which will fail if a module
written in C++ uses that header file. Currently, that header file is
not widely used, but it's a potential problem if the use of static
assertions expands.
As discussed in
</messages/by-id/7775.1492448671@sss.pgh.pa.us>, a
more general solution would be to add specific C++ support for static
assertions in c.h. Here is a patch for that, extracted from my
previously posted C++ patch set, but also a bit reworked from what was
previously posted.
Also attached is a little C++ test file that one can use to test this
out. (Just compiling it should cause a compiler error without the patch
and a static assertion failure with the patch.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v3-0001-Add-support-for-static-assertions-in-C.patchtext/plain; charset=UTF-8; name=v3-0001-Add-support-for-static-assertions-in-C.patch; x-mac-creator=0; x-mac-type=0Download
From 93adbf7bfa7afc1c8e25b037fce8227f3a225639 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 30 Aug 2016 12:00:00 -0400
Subject: [PATCH v3] Add support for static assertions in C++
This allows modules written in C++ to use or include header files that
use StaticAssertStmt() etc.
---
src/include/c.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/include/c.h b/src/include/c.h
index af799dc1df..e238edb7d1 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -749,6 +749,7 @@ typedef NameData *Name;
* about a negative width for a struct bit-field. This will not include a
* helpful error message, but it beats not getting an error at all.
*/
+#ifndef __cplusplus
#ifdef HAVE__STATIC_ASSERT
#define StaticAssertStmt(condition, errmessage) \
do { _Static_assert(condition, errmessage); } while(0)
@@ -760,6 +761,18 @@ typedef NameData *Name;
#define StaticAssertExpr(condition, errmessage) \
StaticAssertStmt(condition, errmessage)
#endif /* HAVE__STATIC_ASSERT */
+#else /* C++ */
+#if defined(__cpp_static_assert) && __cpp_static_assert >= 200410
+#define StaticAssertStmt(condition, errmessage) \
+ static_assert(condition, errmessage)
+#else
+/* not worth providing a workaround */
+#define StaticAssertStmt(condition, errmessage) \
+ ((void) 0)
+#endif
+#define StaticAssertExpr(condition, errmessage) \
+ ({ StaticAssertStmt(condition, errmessage); true; })
+#endif /* C++ */
/*
base-commit: b5c75feca7ffb2667c42b86286e262d6cb709b76
--
2.14.1
On Thu, Aug 31, 2017 at 4:43 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
Commit df1a699e5ba3232f373790b2c9485ddf720c4a70 introduced a
StaticAssertStmt() into a header file, which will fail if a module
written in C++ uses that header file. Currently, that header file is
not widely used, but it's a potential problem if the use of static
assertions expands.As discussed in
</messages/by-id/7775.1492448671@sss.pgh.pa.us>, a
more general solution would be to add specific C++ support for static
assertions in c.h. Here is a patch for that, extracted from my
previously posted C++ patch set, but also a bit reworked from what was
previously posted.
I like the concept of being more C++-compatible, but I'm not sure
about the idea of not providing a workaround, given that we went to
the extreme of doing this:
#define StaticAssertStmt(condition, errmessage) \
((void) sizeof(struct { int static_assert_failure :
(condition) ? 1 : -1; }))
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Aug 31, 2017 at 4:43 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:As discussed in
</messages/by-id/7775.1492448671@sss.pgh.pa.us>, a
more general solution would be to add specific C++ support for static
assertions in c.h. Here is a patch for that, extracted from my
previously posted C++ patch set, but also a bit reworked from what was
previously posted.
I like the concept of being more C++-compatible, but I'm not sure
about the idea of not providing a workaround,
Meh. We support ancient versions of C for backwards compatibility
reasons, but considering that compiling backend code with C++ isn't
officially supported at all, I'm not sure we need to cater to ancient
C++ compilers. We could quibble about the value of "ancient" of
course --- Peter, do you have an idea when this construct became
widely supported?
I do think it might be a better idea to put a #error there instead
of silently disabling static assertions. Then at least we could
hope to get complaints if anyone *is* trying to use ancient C++,
and thereby gauge whether it's worth working any harder for this.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Aug 31, 2017 at 6:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Meh. We support ancient versions of C for backwards compatibility
reasons, but considering that compiling backend code with C++ isn't
officially supported at all, I'm not sure we need to cater to ancient
C++ compilers. We could quibble about the value of "ancient" of
course --- Peter, do you have an idea when this construct became
widely supported?I do think it might be a better idea to put a #error there instead
of silently disabling static assertions. Then at least we could
hope to get complaints if anyone *is* trying to use ancient C++,
and thereby gauge whether it's worth working any harder for this.
I guess my question was whether we couldn't just use the same
workaround we use for old C compilers.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 1, 2017 at 8:28 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Aug 31, 2017 at 6:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Meh. We support ancient versions of C for backwards compatibility
reasons, but considering that compiling backend code with C++ isn't
officially supported at all, I'm not sure we need to cater to ancient
C++ compilers. We could quibble about the value of "ancient" of
course --- Peter, do you have an idea when this construct became
widely supported?I do think it might be a better idea to put a #error there instead
of silently disabling static assertions. Then at least we could
hope to get complaints if anyone *is* trying to use ancient C++,
and thereby gauge whether it's worth working any harder for this.I guess my question was whether we couldn't just use the same
workaround we use for old C compilers.
This got unanswered and the thread has stalled for two months, so for
now I am marking the patch as returned with feedback.
--
Michael
On 11/29/17 00:35, Michael Paquier wrote:
On Fri, Sep 1, 2017 at 8:28 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Aug 31, 2017 at 6:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Meh. We support ancient versions of C for backwards compatibility
reasons, but considering that compiling backend code with C++ isn't
officially supported at all, I'm not sure we need to cater to ancient
C++ compilers. We could quibble about the value of "ancient" of
course --- Peter, do you have an idea when this construct became
widely supported?I do think it might be a better idea to put a #error there instead
of silently disabling static assertions. Then at least we could
hope to get complaints if anyone *is* trying to use ancient C++,
and thereby gauge whether it's worth working any harder for this.I guess my question was whether we couldn't just use the same
workaround we use for old C compilers.This got unanswered and the thread has stalled for two months, so for
now I am marking the patch as returned with feedback.
The answer to that question is "because it doesn't work".
I'd still like a review of this patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Nov 29, 2017 at 9:26 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
I'd still like a review of this patch.
I don't think there's much to review apart from this one issue.
Neither Tom nor I seem to be convinced about:
+/* not worth providing a workaround */
I suggested that it was worth providing a workaround, and Tom
suggested that the case might be so rare that we could just #error if
happens. If you agree that it's never likely to come up, I suggest
going with Tom's #error proposal; otherwise, I suggest trying to find
a workable workaround.
Apart from that, the only thing I see is that it seems like the
comment block just before your code changes might need some updating.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Nov 29, 2017 at 9:26 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:I'd still like a review of this patch.
I don't think there's much to review apart from this one issue.
Neither Tom nor I seem to be convinced about:
+/* not worth providing a workaround */
I suggested that it was worth providing a workaround, and Tom
suggested that the case might be so rare that we could just #error if
happens. If you agree that it's never likely to come up, I suggest
going with Tom's #error proposal; otherwise, I suggest trying to find
a workable workaround.
Right now, we have the property that every build enforces static
assertions, albeit with variable quality of the error messages.
I strongly disagree that it's okay to throw that property away.
I do think that we could put an #error here instead, and wait to see
if anyone complains before expending effort on a workaround.
Apart from that, the only thing I see is that it seems like the
comment block just before your code changes might need some updating.
Agreed, that would need some love as well. I have no other comments
either.
regards, tom lane
On 2017-11-29 09:41:15 -0500, Robert Haas wrote:
On Wed, Nov 29, 2017 at 9:26 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:I'd still like a review of this patch.
I don't think there's much to review apart from this one issue.
Neither Tom nor I seem to be convinced about:+/* not worth providing a workaround */
FWIW, I think that's a perfectly reasonable choice. Adding complications
in making static assertions work for random archaic compilers when
compiling with c++ just doesn't seem worth more than a few mins of
thought.
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2017-11-29 09:41:15 -0500, Robert Haas wrote:
+/* not worth providing a workaround */
FWIW, I think that's a perfectly reasonable choice. Adding complications
in making static assertions work for random archaic compilers when
compiling with c++ just doesn't seem worth more than a few mins of
thought.
I don't think anyone is advocating that we need to develop a solution
that works, at least not pending somebody actually complaining that
they want to build PG with an ancient C++ compiler. I just want
"we don't support this" to be spelled "#error", rather than dumping off
a load of reasoning about what might happen without functioning static
asserts --- on a weird compiler, no less --- onto our future selves.
regards, tom lane
On 2017-11-29 16:39:14 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2017-11-29 09:41:15 -0500, Robert Haas wrote:
+/* not worth providing a workaround */
FWIW, I think that's a perfectly reasonable choice. Adding complications
in making static assertions work for random archaic compilers when
compiling with c++ just doesn't seem worth more than a few mins of
thought.I don't think anyone is advocating that we need to develop a solution
that works, at least not pending somebody actually complaining that
they want to build PG with an ancient C++ compiler. I just want
"we don't support this" to be spelled "#error", rather than dumping off
a load of reasoning about what might happen without functioning static
asserts --- on a weird compiler, no less --- onto our future selves.
C++ static asserts are somewhat new (C++11), so I'm unconvinced by that.
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2017-11-29 16:39:14 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
FWIW, I think that's a perfectly reasonable choice. Adding complications
in making static assertions work for random archaic compilers when
compiling with c++ just doesn't seem worth more than a few mins of
thought.
I don't think anyone is advocating that we need to develop a solution
that works, at least not pending somebody actually complaining that
they want to build PG with an ancient C++ compiler. I just want
"we don't support this" to be spelled "#error", rather than dumping off
a load of reasoning about what might happen without functioning static
asserts --- on a weird compiler, no less --- onto our future selves.
C++ static asserts are somewhat new (C++11), so I'm unconvinced by that.
Wait a minute --- you were just saying that only archaic C++ compilers
were at issue. You don't get to have that both ways.
regards, tom lane
On 11/29/17 10:03, Tom Lane wrote:
Right now, we have the property that every build enforces static
assertions, albeit with variable quality of the error messages.
I strongly disagree that it's okay to throw that property away.
I do think that we could put an #error here instead, and wait to see
if anyone complains before expending effort on a workaround.
I guess the question is whether we would rather be able to have users
continue to use older C++ compilers, or be super picky about static
assertions.
In the g++ line, the oldest compiler that supports static assertions is
g++-6, and g++-5 doesn't support it. I think that is recent enough to
be a concern.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 11/29/17 10:03, Tom Lane wrote:
Right now, we have the property that every build enforces static
assertions, albeit with variable quality of the error messages.
I strongly disagree that it's okay to throw that property away.
I do think that we could put an #error here instead, and wait to see
if anyone complains before expending effort on a workaround.
I guess the question is whether we would rather be able to have users
continue to use older C++ compilers, or be super picky about static
assertions.
Uh, what is this "continue to use" bit? We've never supported
building the backend with C++ compilers. Nor, since the whole
point here is that StaticAssert doesn't work with C++, is there
any existing tradition of building extensions that use StaticAssert
with C++. So I think it's highly likely that if we put in
an #error there, there will be exactly zero complaints.
I'd much rather continue to be sure that StaticAssert does what it
says on the tin than retroactively improve the compatibility situation
for older compilers.
(BTW, why is it that we can't fall back on the negative-width-bitfield
trick for old g++?)
regards, tom lane
On 12/11/17 16:45, Tom Lane wrote:
I guess the question is whether we would rather be able to have users
continue to use older C++ compilers, or be super picky about static
assertions.Uh, what is this "continue to use" bit? We've never supported
building the backend with C++ compilers.
The problem is static assertions in inline functions in header files.
We do support using the header files in C++ extensions. But now there
is a problem that if a C++ extension happens to pull in a header file
that has a static assertion, it doesn't compile anymore, but it used to
before the static assertion was introduced.
(Currently, we have very few static assertions in header files, so the
problem is not relevant in practice yet, but the use of both static
assertions and inline functions is clearly expanding.)
(BTW, why is it that we can't fall back on the negative-width-bitfield
trick for old g++?)
The complaint is
error: types may not be defined in 'sizeof' expressions
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 12/11/17 16:45, Tom Lane wrote:
(BTW, why is it that we can't fall back on the negative-width-bitfield
trick for old g++?)
The complaint is
error: types may not be defined in 'sizeof' expressions
Hmm, well, surely there's more than one way to do that; the sizeof
is just a convenient way to wrap it in C. Wouldn't a typedef serve
just as well?
(Googling the topic shows that this wheel has been invented
before, BTW.)
regards, tom lane
On 12/11/17 17:12, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 12/11/17 16:45, Tom Lane wrote:
(BTW, why is it that we can't fall back on the negative-width-bitfield
trick for old g++?)The complaint is
error: types may not be defined in 'sizeof' expressionsHmm, well, surely there's more than one way to do that; the sizeof
is just a convenient way to wrap it in C. Wouldn't a typedef serve
just as well?
Here is another attempt, which has the desired effect with the handful
of compilers I have available.
(With the recent changes to AllocSetContextCreate() using a static
assertion, the current state now breaks actual extensions written in C++
in some configurations, so this has become a bit of a must-fix for PG11.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v4-0001-Add-support-for-static-assertions-in-C.patchtext/plain; charset=UTF-8; name=v4-0001-Add-support-for-static-assertions-in-C.patch; x-mac-creator=0; x-mac-type=0Download
From d631fcb1fb2babef618bc9b3eba3f5591970a609 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 30 Aug 2016 12:00:00 -0400
Subject: [PATCH v4] Add support for static assertions in C++
This allows modules written in C++ to use or include header files that
use StaticAssertStmt() etc.
---
src/include/c.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/include/c.h b/src/include/c.h
index 11fcffbae3..22535a7deb 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -754,6 +754,7 @@ typedef NameData *Name;
* about a negative width for a struct bit-field. This will not include a
* helpful error message, but it beats not getting an error at all.
*/
+#ifndef __cplusplus
#ifdef HAVE__STATIC_ASSERT
#define StaticAssertStmt(condition, errmessage) \
do { _Static_assert(condition, errmessage); } while(0)
@@ -765,6 +766,19 @@ typedef NameData *Name;
#define StaticAssertExpr(condition, errmessage) \
StaticAssertStmt(condition, errmessage)
#endif /* HAVE__STATIC_ASSERT */
+#else /* C++ */
+#if defined(__cpp_static_assert) && __cpp_static_assert >= 200410
+#define StaticAssertStmt(condition, errmessage) \
+ static_assert(condition, errmessage)
+#define StaticAssertExpr(condition, errmessage) \
+ StaticAssertStmt(condition, errmessage)
+#else
+#define StaticAssertStmt(condition, errmessage) \
+ do { struct static_assert_struct { int static_assert_failure : (condition) ? 1 : -1; }; } while(0)
+#define StaticAssertExpr(condition, errmessage) \
+ ({ StaticAssertStmt(condition, errmessage); })
+#endif
+#endif /* C++ */
/*
base-commit: 56a95ee5118bf6d46e261b8d352f7dafac10587d
--
2.15.1
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 12/11/17 17:12, Tom Lane wrote:
Hmm, well, surely there's more than one way to do that; the sizeof
is just a convenient way to wrap it in C. Wouldn't a typedef serve
just as well?
Here is another attempt, which has the desired effect with the handful
of compilers I have available.
I can confirm that the negative-bitfield-width method employed here
has the desired effects (i.e., error or not, without unwanted warnings)
on the oldest C++ compilers I have handy, namely
$ g++ -v
Reading specs from /usr/local/lib/gcc-lib/hppa2.0-hp-hpux10.20/2.95.3/specs
gcc version 2.95.3 20010315 (release)
$ g++ -v
Using built-in specs.
Target: powerpc-apple-darwin8
Configured with: /private/var/tmp/gcc/gcc-5341.obj~1/src/configure --disable-checking -enable-werror --prefix=/usr --mandir=/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^.-]*$/s/$/-4.0/ --with-gxx-include-dir=/include/c++/4.0.0 --with-slibdir=/usr/lib --build=powerpc-apple-darwin8 --host=powerpc-apple-darwin8 --target=powerpc-apple-darwin8
Thread model: posix
gcc version 4.0.1 (Apple Computer, Inc. build 5341)
I do not have a well-informed opinion on whether
#if defined(__cpp_static_assert) && __cpp_static_assert >= 200410
is an appropriate test for static_assert() being available, but I'm
pretty suspicious of it because none of my C++ compilers seem to
take that path, not even recent stuff like clang 9.0.0. However,
since the negative-bitfield-width code path works anyway, that's
something we could refine later.
In short, I think this could be committed as-is, but later we might want
to do some more research on how to tell whether static_assert() is
available.
regards, tom lane
On 12/20/17 00:57, Tom Lane wrote:
I do not have a well-informed opinion on whether
#if defined(__cpp_static_assert) && __cpp_static_assert >= 200410
is an appropriate test for static_assert() being available, but I'm
pretty suspicious of it because none of my C++ compilers seem to
take that path, not even recent stuff like clang 9.0.0.
For clang, you apparently need to pass -std=c++11 or higher. With g++
=6, that's the default.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 12/20/17 00:57, Tom Lane wrote:
I do not have a well-informed opinion on whether
#if defined(__cpp_static_assert) && __cpp_static_assert >= 200410
is an appropriate test for static_assert() being available, but I'm
pretty suspicious of it because none of my C++ compilers seem to
take that path, not even recent stuff like clang 9.0.0.
For clang, you apparently need to pass -std=c++11 or higher. With g++
=6, that's the default.
Ah. I did try -std=c++0x on one machine, but forgot to do so with
the newer clang. That does seem to make it take the static_assert
path on recent macOS.
regards, tom lane
On 12/20/17 10:51, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 12/20/17 00:57, Tom Lane wrote:
I do not have a well-informed opinion on whether
#if defined(__cpp_static_assert) && __cpp_static_assert >= 200410
is an appropriate test for static_assert() being available, but I'm
pretty suspicious of it because none of my C++ compilers seem to
take that path, not even recent stuff like clang 9.0.0.For clang, you apparently need to pass -std=c++11 or higher. With g++
=6, that's the default.
Ah. I did try -std=c++0x on one machine, but forgot to do so with
the newer clang. That does seem to make it take the static_assert
path on recent macOS.
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services