On Wed, Nov 27, 2019 at 12:23:33PM +0000, Smith, Peter wrote: > * That is beyond the scope for what I wanted my patch to achieve; my > * use-cases are C code only.
Well, FWIW, I do have some extensions using __cplusplus and I am pretty sure that I am not the only one with that. The thing is that with your patch folks would not get any compilation failures *now* because all the declarations of StaticAssertDecl() are added within the .c files, but once a patch which includes a declaration in a header, something very likely to happen, is merged then we head into breaking suddenly the compilation of those modules. And that's not nice. That's also a point raised by Andres upthread. > I am happy if somebody else with more ability to test C++ properly > wants to add the __cplusplus variant of the new macro. In short, attached is an updated version of your patch which attempts to solve that. I have tested this with some cplusplus stuff, and GCC for both versions (static_assert is available in GCC >= 6, but a manual change of c.h does the trick). I have edited the patch a bit while on it, your assertions did not use project-style grammar, the use of parenthesis was inconsistent (see relpath.c for example), and pgindent has complained a bit. Also, I am bumping the patch to next CF for now. Do others have thoughts to share about this version? I would be actually fine to commit that, even if the message generated for the fallback versions is a bit crappy with a complain about a negative array size, but that's not new to this patch as we use that as well with StaticAssertStmt(). -- Michael
diff --git a/src/include/c.h b/src/include/c.h index 00e41ac546..91d6d50e76 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -845,11 +845,16 @@ extern void ExceptionalCondition(const char *conditionName, do { _Static_assert(condition, errmessage); } while(0) #define StaticAssertExpr(condition, errmessage) \ ((void) ({ StaticAssertStmt(condition, errmessage); true; })) +/* StaticAssertDecl is suitable for use at file scope. */ +#define StaticAssertDecl(condition, errmessage) \ + _Static_assert(condition, errmessage) #else /* !HAVE__STATIC_ASSERT */ #define StaticAssertStmt(condition, errmessage) \ ((void) sizeof(struct { int static_assert_failure : (condition) ? 1 : -1; })) #define StaticAssertExpr(condition, errmessage) \ StaticAssertStmt(condition, errmessage) +#define StaticAssertDecl(condition, errmessage) \ + extern void static_assert_func(int static_assert_failure[(condition) ? 1 : -1]) #endif /* HAVE__STATIC_ASSERT */ #else /* C++ */ #if defined(__cpp_static_assert) && __cpp_static_assert >= 200410 @@ -857,12 +862,16 @@ extern void ExceptionalCondition(const char *conditionName, static_assert(condition, errmessage) #define StaticAssertExpr(condition, errmessage) \ ({ static_assert(condition, errmessage); }) -#else +#define StaticAssertDecl(condition, errmessage) \ + static_assert(condition, errmessage) +#else /* !__cpp_static_assert */ #define StaticAssertStmt(condition, errmessage) \ do { struct static_assert_struct { int static_assert_failure : (condition) ? 1 : -1; }; } while(0) #define StaticAssertExpr(condition, errmessage) \ ((void) ({ StaticAssertStmt(condition, errmessage); })) -#endif +#define StaticAssertDecl(condition, errmessage) \ + extern void static_assert_func(int static_assert_failure[(condition) ? 1 : -1]) +#endif /* __cpp_static_assert */ #endif /* C++ */ diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c index bc62c6eef7..e7e992eb85 100644 --- a/src/backend/utils/adt/lockfuncs.c +++ b/src/backend/utils/adt/lockfuncs.c @@ -36,6 +36,9 @@ const char *const LockTagTypeNames[] = { "advisory" }; +StaticAssertDecl(lengthof(LockTagTypeNames) == (LOCKTAG_ADVISORY + 1), + "LockTagTypeNames array inconsistency"); + /* This must match enum PredicateLockTargetType (predicate_internals.h) */ static const char *const PredicateLockTagTypeNames[] = { "relation", @@ -43,6 +46,9 @@ static const char *const PredicateLockTagTypeNames[] = { "tuple" }; +StaticAssertDecl(lengthof(PredicateLockTagTypeNames) == (PREDLOCKTAG_TUPLE + 1), + "PredicateLockTagTypeNames array inconsistency"); + /* Working status for pg_lock_status */ typedef struct { diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index ba4edde71a..3dfffac88f 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -236,6 +236,9 @@ static const struct config_enum_entry bytea_output_options[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(bytea_output_options) == (BYTEA_OUTPUT_HEX + 2), + "bytea_output_options array inconsistency"); + /* * We have different sets for client and server message level options because * they sort slightly different (see "log" level), and because "fatal"/"panic" @@ -281,6 +284,9 @@ static const struct config_enum_entry intervalstyle_options[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(intervalstyle_options) == (INTSTYLE_ISO_8601 + 2), + "intervalstyle_options array inconsistency"); + static const struct config_enum_entry log_error_verbosity_options[] = { {"terse", PGERROR_TERSE, false}, {"default", PGERROR_DEFAULT, false}, @@ -288,6 +294,9 @@ static const struct config_enum_entry log_error_verbosity_options[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(log_error_verbosity_options) == (PGERROR_VERBOSE + 2), + "log_error_verbosity_options array inconsistency"); + static const struct config_enum_entry log_statement_options[] = { {"none", LOGSTMT_NONE, false}, {"ddl", LOGSTMT_DDL, false}, @@ -296,6 +305,9 @@ static const struct config_enum_entry log_statement_options[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(log_statement_options) == (LOGSTMT_ALL + 2), + "log_statement_options array inconsistency"); + static const struct config_enum_entry isolation_level_options[] = { {"serializable", XACT_SERIALIZABLE, false}, {"repeatable read", XACT_REPEATABLE_READ, false}, @@ -311,6 +323,9 @@ static const struct config_enum_entry session_replication_role_options[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(session_replication_role_options) == (SESSION_REPLICATION_ROLE_LOCAL + 2), + "session_replication_role_options array inconsistency"); + static const struct config_enum_entry syslog_facility_options[] = { #ifdef HAVE_SYSLOG {"local0", LOG_LOCAL0, false}, @@ -334,18 +349,27 @@ static const struct config_enum_entry track_function_options[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(track_function_options) == (TRACK_FUNC_ALL + 2), + "track_function_options array inconsistency"); + static const struct config_enum_entry xmlbinary_options[] = { {"base64", XMLBINARY_BASE64, false}, {"hex", XMLBINARY_HEX, false}, {NULL, 0, false} }; +StaticAssertDecl(lengthof(xmlbinary_options) == (XMLBINARY_HEX + 2), + "xmlbinary_options array inconsistency"); + static const struct config_enum_entry xmloption_options[] = { {"content", XMLOPTION_CONTENT, false}, {"document", XMLOPTION_DOCUMENT, false}, {NULL, 0, false} }; +StaticAssertDecl(lengthof(xmloption_options) == (XMLOPTION_CONTENT + 2), + "xmloption_options array inconsistency"); + /* * Although only "on", "off", and "safe_encoding" are documented, we * accept all the likely variants of "on" and "off". @@ -460,6 +484,9 @@ const struct config_enum_entry ssl_protocol_versions_info[] = { {NULL, 0, false} }; +StaticAssertDecl(lengthof(ssl_protocol_versions_info) == (PG_TLS1_3_VERSION + 2), + "ssl_protocol_versions_info array inconsistency"); + static struct config_enum_entry shared_memory_options[] = { #ifndef WIN32 {"sysv", SHMEM_TYPE_SYSV, false}, @@ -609,6 +636,9 @@ const char *const GucContext_Names[] = /* PGC_USERSET */ "user" }; +StaticAssertDecl(lengthof(GucContext_Names) == (PGC_USERSET + 1), + "GucContext_Names array inconsistency"); + /* * Displayable names for source types (enum GucSource) * @@ -632,6 +662,9 @@ const char *const GucSource_Names[] = /* PGC_S_SESSION */ "session" }; +StaticAssertDecl(lengthof(GucSource_Names) == (PGC_S_SESSION + 1), + "GucSource_Names array inconsistency"); + /* * Displayable names for the groupings defined in enum config_group */ @@ -743,6 +776,9 @@ const char *const config_group_names[] = NULL }; +StaticAssertDecl(lengthof(config_group_names) == (DEVELOPER_OPTIONS + 2), + "config_group_names array inconsistency"); + /* * Displayable names for GUC variable types (enum config_type) * @@ -757,6 +793,9 @@ const char *const config_type_names[] = /* PGC_ENUM */ "enum" }; +StaticAssertDecl(lengthof(config_type_names) == (PGC_ENUM + 1), + "config_type_names array inconsistency"); + /* * Unit conversion tables. * diff --git a/src/common/relpath.c b/src/common/relpath.c index 62b9553f1b..3aaf5ac736 100644 --- a/src/common/relpath.c +++ b/src/common/relpath.c @@ -37,6 +37,9 @@ const char *const forkNames[] = { "init" /* INIT_FORKNUM */ }; +StaticAssertDecl(lengthof(forkNames) == (MAX_FORKNUM + 1), + "forkNames array inconsistency"); + /* * forkname_to_number - look up fork number by name * diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index 4864274909..73d1d77f70 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -80,6 +80,9 @@ static const int dbObjectTypePriority[] = 38 /* DO_SUBSCRIPTION */ }; +StaticAssertDecl(lengthof(dbObjectTypePriority) == (DO_SUBSCRIPTION + 1), + "dbObjectTypePriority array inconsistency"); + static DumpId preDataBoundId; static DumpId postDataBoundId;
signature.asc
Description: PGP signature