Re: Proposal: Add more compile-time asserts to expose inconsistencies.

Hi,

On 2019-09-19 04:46:27 +0000, Smith, Peter wrote:
> In the attached patch example I have defined a new macro
> StaticAssertDecl. A typical usage of it is shown in the relpath.c
> file.

I'm in favor of adding that - in fact, when I was working on adding a a
static_assert wrapper, I was advocating for only supporting compilers
that know static_assert, so we can put these in the global scope. The
number of compilers that don't support static_assert is pretty small
today, especially compared to 2012, when we added these.

https://www.postgresql.org/message-id/E1TIW1p-0002yE-AY%40gemulon.postgresql.org
https://www.postgresql.org/message-id/27446.1349024252%40sss.pgh.pa.us

Tom, you were arguing for restricting to file scope to get broader
compatibility, are you ok with adding a seperate *Decl version?

Or perhaps it's time to just remove the fallback implementation? I think
we'd have to add proper MSVC support, but that seems like something we
should do anyway.

Back then I was wondering about using tyepedef to emulate static assert
that works both in file and block scope, but that struggles with needing
unique names.

FWIW, the perl5 implementation has precisely that problem. If it's used
in multiple headers (or a header and a .c file), two static asserts may
not be on the same line... - which one will only notice when using an
old compiler.

I wonder if defining the fallback static assert code to something like
extern void static_assert_func(int static_assert_failed[(condition) ? 1 : -1]);
isn't a solution, however. I *think* that's standard C. Seems to work at
least with gcc, clang, msvc, icc.

Re standard: C99's "6.7 Declarations" + 6.7.1 defines 'declaration' to
include extern specifiers and in 6.7.1 5) says "The declaration of an
identifier for a function that has block scope shall have no explicit
storage-class specifier other than extern.". And "6.8 Statements and
blocks", via "6.8.2 Compound statement" allows declarations in statements.

You can play with a good few compilers at: https://godbolt.org/z/fl0Mzu

> The goal was to leave all existing Postgres static assert macros unchanged, but to allow static asserts to be added in the code at file scope without the need for the explicit ct_asserts function.

It'd probably worthwhile to move many of the current ones.

> Notice, in reality the StaticAssertDecl macro still uses a static function as a wrapper for the StaticAssertStmt, but now the function is not visible in the source.

I think this implementation is not ok, due to the unique-name issue.

Greetings,

Andres Freund