Re: Proposal: Add more compile-time asserts to exposeinconsistencies. - Mailing list pgsql-hackers

Hi Peter, Peter, :)


On 2019-10-28 00:30:11 +0000, Smith, Peter wrote:
> From: Andres Freund <andres@anarazel.de> Sent: Sunday, 27 October 2019 12:03 PM
> > My proposal for this really was just to use this as a fallback for when static assert isn't available. Which in
turnI was just suggesting because Tom wanted a fallback.
 
>
> The patch is updated to use "extern" technique only when  "_Static_assert" is unavailable.

Cool.


>  /*
>   * forkname_to_number - look up fork number by name
> diff --git a/src/include/c.h b/src/include/c.h
> index d752cc0..3e24ff4 100644
> --- a/src/include/c.h
> +++ b/src/include/c.h
> @@ -838,11 +838,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
> @@ -858,7 +863,6 @@ extern void ExceptionalCondition(const char *conditionName,
>  #endif
>  #endif                            /* C++
>   */

Peter Smith:

Is there a reason to not just make StaticAssertExpr and StaticAssertStmt
be the same? I don't want to proliferate variants that users have to
understand if there's no compelling need.  Nor do I think do we really
need two different fallback implementation for static asserts.

As far as I can tell we should be able to use the prototype based
approach in all the cases where we currently use the "negative bit-field
width" approach?

Should then also update
 * Otherwise we fall back on a kluge that assumes the compiler will complain
 * 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.


Peter Eisentraut:

Looking at the cplusplus variant, I'm somewhat surprised to see that you
made both fallback and plain version unconditionally use GCC style
compound expressions:

commit a2c8e5cfdb9d82ae6d4bb8f37a4dc7cbeca63ec1
Author: Peter Eisentraut <peter_e@gmx.net>
Date:   2016-08-30 12:00:00 -0400

    Add support for static assertions in 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

Was that intentional? The C version intentionally uses compound
expressions only for the _Static_assert case, where configure tests for
the compound expression support?  As far as I can tell this'll not allow
using our headers e.g. with msvc in C++ mode if somebody introduce a
static assertion in a header - which seems like a likely and good
outcome with the changes proposed here?


Btw, it looks to me like msvc supports using the C++ static_assert()
even in C mode:
https://godbolt.org/z/b_dxDW

Greetings,

Andres Freund