Static assertions cleanup

Because we added StaticAssertStmt() first before StaticAssertDecl(),
some uses as well as the instructions in c.h are now a bit backwards
from the "native" way static assertions are meant to be used in C.
This updates the guidance and moves some static assertions to better
places.

Specifically, since the addition of StaticAssertDecl(), we can put
static assertions at the file level.  This moves a number of static
assertions out of function bodies, where they might have been stuck
out of necessity, to perhaps better places at the file level or in
header files.

Also, when the static assertion appears in a position where a
declaration is allowed, then using StaticAssertDecl() is more native
than StaticAssertStmt().

Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/941a04e7-dd6f-c0e4-8cdf-a33b3338cbda%40enterprisedb.com

7 files changed, 43 insertions, 16 deletions

diff --git a/src/include/access/gin.h b/src/include/access/gin.h
index fff861e2192..119ed685bb4 100644
--- a/src/include/access/gin.h
+++ b/src/include/access/gin.h
@@ -57,6 +57,9 @@ typedef struct GinStatsData
  */
 typedef char GinTernaryValue;
 
+StaticAssertDecl(sizeof(GinTernaryValue) == sizeof(bool),
+				 "sizes of GinTernaryValue and bool are not equal");
+
 #define GIN_FALSE		0		/* item is not present / does not match */
 #define GIN_TRUE		1		/* item is present / matches */
 #define GIN_MAYBE		2		/* don't know if item is present / don't know
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 9561c835f21..c1af814e8d0 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -426,6 +426,9 @@ do { \
 	(tup)->t_choice.t_heap.t_field3.t_xvac = (xid); \
 } while (0)
 
+StaticAssertDecl(MaxOffsetNumber < SpecTokenOffsetNumber,
+				 "invalid speculative token constant");
+
 #define HeapTupleHeaderIsSpeculative(tup) \
 ( \
 	(ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 8e4f6864e59..7d5a6fa558e 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -467,6 +467,13 @@ typedef struct BTVacState
 #define BT_IS_POSTING				0x2000
 
 /*
+ * Mask allocated for number of keys in index tuple must be able to fit
+ * maximum possible number of index attributes
+ */
+StaticAssertDecl(BT_OFFSET_MASK >= INDEX_MAX_KEYS,
+				 "BT_OFFSET_MASK can't fit INDEX_MAX_KEYS");
+
+/*
  * Note: BTreeTupleIsPivot() can have false negatives (but not false
  * positives) when used with !heapkeyspace indexes
  */
diff --git a/src/include/c.h b/src/include/c.h
index 98cdd285dd9..bd6d8e5bf53 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -847,47 +847,50 @@ extern void ExceptionalCondition(const char *conditionName,
  * If the "condition" (a compile-time-constant expression) evaluates to false,
  * throw a compile error using the "errmessage" (a string literal).
  *
- * gcc 4.6 and up supports _Static_assert(), but there are bizarre syntactic
- * placement restrictions.  Macros StaticAssertStmt() and StaticAssertExpr()
+ * C11 has _Static_assert(), and most C99 compilers already support that.  For
+ * portability, we wrap it into StaticAssertDecl().  _Static_assert() is a
+ * "declaration", and so it must be placed where for example a variable
+ * declaration would be valid.  As long as we compile with
+ * -Wno-declaration-after-statement, that also means it cannot be placed after
+ * statements in a function.  Macros StaticAssertStmt() and StaticAssertExpr()
  * make it safe to use as a statement or in an expression, respectively.
- * The macro StaticAssertDecl() is suitable for use at file scope (outside of
- * any function).
  *
- * 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.
+ * For compilers without _Static_assert(), 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.
  */
 #ifndef __cplusplus
 #ifdef HAVE__STATIC_ASSERT
+#define StaticAssertDecl(condition, errmessage) \
+	_Static_assert(condition, errmessage)
 #define StaticAssertStmt(condition, errmessage) \
 	do { _Static_assert(condition, errmessage); } while(0)
 #define StaticAssertExpr(condition, errmessage) \
 	((void) ({ StaticAssertStmt(condition, errmessage); true; }))
-#define StaticAssertDecl(condition, errmessage) \
-	_Static_assert(condition, errmessage)
 #else							/* !HAVE__STATIC_ASSERT */
+#define StaticAssertDecl(condition, errmessage) \
+	extern void static_assert_func(int static_assert_failure[(condition) ? 1 : -1])
 #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
+#define StaticAssertDecl(condition, errmessage) \
+	static_assert(condition, errmessage)
 #define StaticAssertStmt(condition, errmessage) \
 	static_assert(condition, errmessage)
 #define StaticAssertExpr(condition, errmessage) \
 	({ static_assert(condition, errmessage); })
-#define StaticAssertDecl(condition, errmessage) \
-	static_assert(condition, errmessage)
 #else							/* !__cpp_static_assert */
+#define StaticAssertDecl(condition, errmessage) \
+	extern void static_assert_func(int static_assert_failure[(condition) ? 1 : -1])
 #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); }))
-#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/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index 06368e23667..1b648b7196e 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -247,4 +247,12 @@ typedef struct ControlFileData
  */
 #define PG_CONTROL_FILE_SIZE		8192
 
+/*
+ * Ensure that the size of the pg_control data structure is sane.
+ */
+StaticAssertDecl(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE,
+				 "pg_control is too large for atomic disk writes");
+StaticAssertDecl(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE,
+				 "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
+
 #endif							/* PG_CONTROL_H */
diff --git a/src/include/common/int128.h b/src/include/common/int128.h
index 8f035cf4cb2..4b17aa9689e 100644
--- a/src/include/common/int128.h
+++ b/src/include/common/int128.h
@@ -177,7 +177,7 @@ static inline void
 int128_add_int64_mul_int64(INT128 *i128, int64 x, int64 y)
 {
 	/* INT64_AU32 must use arithmetic right shift */
-	StaticAssertStmt(((int64) -1 >> 1) == (int64) -1,
+	StaticAssertDecl(((int64) -1 >> 1) == (int64) -1,
 					 "arithmetic right shift is needed");
 
 	/*----------
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index a494cb598ff..dd818e16abc 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -59,6 +59,9 @@ typedef struct LWLock
  */
 #define LWLOCK_PADDED_SIZE	PG_CACHE_LINE_SIZE
 
+StaticAssertDecl(sizeof(LWLock) <= LWLOCK_PADDED_SIZE,
+				 "Miscalculated LWLock padding");
+
 /* LWLock, padded to a full cache line size */
 typedef union LWLockPadded
 {