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

Hi Peter, Peter, :)

On 2019-10-28 00:30:11 +0000, Smith, Peter wrote:
> From: Andres Freund <andres(at)anarazel(dot)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 turn I 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(at)gmx(dot)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