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

19 files changed, 48 insertions, 72 deletions

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 747db503761..42756a9e6df 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5783,10 +5783,6 @@ heap_finish_speculative(Relation relation, ItemPointer tid)
 
 	htup = (HeapTupleHeader) PageGetItem(page, lp);
 
-	/* SpecTokenOffsetNumber should be distinguishable from any real offset */
-	StaticAssertStmt(MaxOffsetNumber < SpecTokenOffsetNumber,
-					 "invalid speculative token constant");
-
 	/* NO EREPORT(ERROR) from here till changes are logged */
 	START_CRIT_SECTION();
 
@@ -7921,7 +7917,7 @@ index_delete_sort(TM_IndexDeleteOp *delstate)
 	const int	gaps[9] = {1968, 861, 336, 112, 48, 21, 7, 3, 1};
 
 	/* Think carefully before changing anything here -- keep swaps cheap */
-	StaticAssertStmt(sizeof(TM_IndexDelete) <= 8,
+	StaticAssertDecl(sizeof(TM_IndexDelete) <= 8,
 					 "element size exceeds 8 bytes");
 
 	for (int g = 0; g < lengthof(gaps); g++)
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index ff260c393ab..10f13e6d415 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2486,13 +2486,6 @@ _bt_check_natts(Relation rel, bool heapkeyspace, Page page, OffsetNumber offnum)
 	Assert(offnum >= FirstOffsetNumber &&
 		   offnum <= PageGetMaxOffsetNumber(page));
 
-	/*
-	 * Mask allocated for number of keys in index tuple must be able to fit
-	 * maximum possible number of index attributes
-	 */
-	StaticAssertStmt(BT_OFFSET_MASK >= INDEX_MAX_KEYS,
-					 "BT_OFFSET_MASK can't fit INDEX_MAX_KEYS");
-
 	itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum));
 	tupnatts = BTreeTupleGetNAtts(itup, rel);
 
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 77d9894dab3..8832efc7c11 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -275,7 +275,7 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
 						   bool all_xact_same_page)
 {
 	/* Can't use group update when PGPROC overflows. */
-	StaticAssertStmt(THRESHOLD_SUBTRANS_CLOG_OPT <= PGPROC_MAX_CACHED_SUBXIDS,
+	StaticAssertDecl(THRESHOLD_SUBTRANS_CLOG_OPT <= PGPROC_MAX_CACHED_SUBXIDS,
 					 "group clog threshold less than PGPROC cached subxids");
 
 	/*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a31fbbff78d..91473b00d90 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3884,15 +3884,6 @@ WriteControlFile(void)
 	char		buffer[PG_CONTROL_FILE_SIZE];	/* need not be aligned */
 
 	/*
-	 * Ensure that the size of the pg_control data structure is sane.  See the
-	 * comments for these symbols in pg_control.h.
-	 */
-	StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE,
-					 "pg_control is too large for atomic disk writes");
-	StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE,
-					 "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
-
-	/*
 	 * Initialize version and compatibility-check fields
 	 */
 	ControlFile->pg_control_version = PG_CONTROL_VERSION;
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 7aa5f6e44d1..c00ac14c0be 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -370,7 +370,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 			else
 			{
 				/* Properly terminate the tarfile. */
-				StaticAssertStmt(2 * TAR_BLOCK_SIZE <= BLCKSZ,
+				StaticAssertDecl(2 * TAR_BLOCK_SIZE <= BLCKSZ,
 								 "BLCKSZ too small for 2 tar blocks");
 				memset(sink->bbs_buffer, 0, 2 * TAR_BLOCK_SIZE);
 				bbsink_archive_contents(sink, 2 * TAR_BLOCK_SIZE);
@@ -1745,7 +1745,7 @@ _tarWriteHeader(bbsink *sink, const char *filename, const char *linktarget,
 		 * large enough to fit an entire tar block. We double-check by means
 		 * of these assertions.
 		 */
-		StaticAssertStmt(TAR_BLOCK_SIZE <= BLCKSZ,
+		StaticAssertDecl(TAR_BLOCK_SIZE <= BLCKSZ,
 						 "BLCKSZ too small for tar block");
 		Assert(sink->bbs_buffer_length >= TAR_BLOCK_SIZE);
 
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 7f3e64b5ae6..30394dccf56 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -191,6 +191,12 @@ static const Oid object_classes[] = {
 	TransformRelationId			/* OCLASS_TRANSFORM */
 };
 
+/*
+ * Make sure object_classes is kept up to date with the ObjectClass enum.
+ */
+StaticAssertDecl(lengthof(object_classes) == LAST_OCLASS + 1,
+				 "object_classes[] must cover all ObjectClasses");
+
 
 static void findDependentObjects(const ObjectAddress *object,
 								 int objflags,
@@ -2550,12 +2556,6 @@ add_object_address(ObjectClass oclass, Oid objectId, int32 subId,
 {
 	ObjectAddress *item;
 
-	/*
-	 * Make sure object_classes is kept up to date with the ObjectClass enum.
-	 */
-	StaticAssertStmt(lengthof(object_classes) == LAST_OCLASS + 1,
-					 "object_classes[] must cover all ObjectClasses");
-
 	/* enlarge array if needed */
 	if (addrs->numrefs >= addrs->maxrefs)
 	{
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index be48886511e..957406ae697 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -496,7 +496,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		&&CASE_EEOP_LAST
 	};
 
-	StaticAssertStmt(lengthof(dispatch_table) == EEOP_LAST + 1,
+	StaticAssertDecl(lengthof(dispatch_table) == EEOP_LAST + 1,
 					 "dispatch_table out of whack with ExprEvalOp");
 
 	if (unlikely(state == NULL))
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index ee7f52218ab..c9bab85e82f 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -1443,7 +1443,7 @@ scram_mock_salt(const char *username)
 	 * not larger than the SHA256 digest length.  If the salt is smaller, the
 	 * caller will just ignore the extra data.)
 	 */
-	StaticAssertStmt(PG_SHA256_DIGEST_LENGTH >= SCRAM_DEFAULT_SALT_LEN,
+	StaticAssertDecl(PG_SHA256_DIGEST_LENGTH >= SCRAM_DEFAULT_SALT_LEN,
 					 "salt length greater than SHA256 digest length");
 
 	ctx = pg_cryptohash_create(PG_SHA256);
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 46e91441ac5..870b9076970 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -127,6 +127,12 @@ static const char *const UserAuthName[] =
 	"peer"
 };
 
+/*
+ * Make sure UserAuthName[] tracks additions to the UserAuth enum
+ */
+StaticAssertDecl(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
+				 "UserAuthName[] must match the UserAuth enum");
+
 
 static List *tokenize_expand_file(List *tokens, const char *outer_filename,
 								  const char *inc_filename, int elevel,
@@ -3117,11 +3123,5 @@ hba_getauthmethod(hbaPort *port)
 const char *
 hba_authname(UserAuth auth_method)
 {
-	/*
-	 * Make sure UserAuthName[] tracks additions to the UserAuth enum
-	 */
-	StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
-					 "UserAuthName[] must match the UserAuth enum");
-
 	return UserAuthName[auth_method];
 }
diff --git a/src/backend/port/atomics.c b/src/backend/port/atomics.c
index ba274bed931..ff4771ae370 100644
--- a/src/backend/port/atomics.c
+++ b/src/backend/port/atomics.c
@@ -54,7 +54,7 @@ pg_extern_compiler_barrier(void)
 void
 pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr)
 {
-	StaticAssertStmt(sizeof(ptr->sema) >= sizeof(slock_t),
+	StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t),
 					 "size mismatch of atomic_flag vs slock_t");
 
 #ifndef HAVE_SPINLOCKS
@@ -105,7 +105,7 @@ pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr)
 void
 pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_)
 {
-	StaticAssertStmt(sizeof(ptr->sema) >= sizeof(slock_t),
+	StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t),
 					 "size mismatch of atomic_uint32 vs slock_t");
 
 	/*
@@ -181,7 +181,7 @@ pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
 void
 pg_atomic_init_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 val_)
 {
-	StaticAssertStmt(sizeof(ptr->sema) >= sizeof(slock_t),
+	StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t),
 					 "size mismatch of atomic_uint64 vs slock_t");
 
 	/*
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index a5ad36ca780..528b2e96438 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -108,6 +108,9 @@ extern slock_t *ShmemLock;
 /* Must be greater than MAX_BACKENDS - which is 2^23-1, so we're fine. */
 #define LW_SHARED_MASK				((uint32) ((1 << 24)-1))
 
+StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
+				 "MAX_BACKENDS too big for lwlock.c");
+
 /*
  * There are three sorts of LWLock "tranches":
  *
@@ -466,12 +469,6 @@ LWLockShmemSize(void)
 void
 CreateLWLocks(void)
 {
-	StaticAssertStmt(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
-					 "MAX_BACKENDS too big for lwlock.c");
-
-	StaticAssertStmt(sizeof(LWLock) <= LWLOCK_PADDED_SIZE,
-					 "Miscalculated LWLock padding");
-
 	if (!IsUnderPostmaster)
 	{
 		Size		spaceLocks = LWLockShmemSize();
diff --git a/src/backend/storage/page/itemptr.c b/src/backend/storage/page/itemptr.c
index 9011337aa81..5c98a5ec2b8 100644
--- a/src/backend/storage/page/itemptr.c
+++ b/src/backend/storage/page/itemptr.c
@@ -18,6 +18,12 @@
 
 
 /*
+ * We really want ItemPointerData to be exactly 6 bytes.
+ */
+StaticAssertDecl(sizeof(ItemPointerData) == 3 * sizeof(uint16),
+				 "ItemPointerData struct is improperly padded");
+
+/*
  * ItemPointerEquals
  *	Returns true if both item pointers point to the same item,
  *	 otherwise returns false.
@@ -28,13 +34,6 @@
 bool
 ItemPointerEquals(ItemPointer pointer1, ItemPointer pointer2)
 {
-	/*
-	 * We really want ItemPointerData to be exactly 6 bytes.  This is rather a
-	 * random place to check, but there is no better place.
-	 */
-	StaticAssertStmt(sizeof(ItemPointerData) == 3 * sizeof(uint16),
-					 "ItemPointerData struct is improperly padded");
-
 	if (ItemPointerGetBlockNumber(pointer1) ==
 		ItemPointerGetBlockNumber(pointer2) &&
 		ItemPointerGetOffsetNumber(pointer1) ==
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index c024928bc8d..152e1b7e0c4 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -4187,7 +4187,8 @@ int64_div_fast_to_numeric(int64 val1, int log10val2)
 	{
 		static int	pow10[] = {1, 10, 100, 1000};
 
-		StaticAssertStmt(lengthof(pow10) == DEC_DIGITS, "mismatch with DEC_DIGITS");
+		StaticAssertDecl(lengthof(pow10) == DEC_DIGITS, "mismatch with DEC_DIGITS");
+
 		if (unlikely(pg_mul_s64_overflow(val1, pow10[DEC_DIGITS - m], &val1)))
 		{
 			/*
diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c
index e272fca0756..cf23aeb5ead 100644
--- a/src/backend/utils/adt/tsginidx.c
+++ b/src/backend/utils/adt/tsginidx.c
@@ -236,8 +236,6 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
 		 * query.
 		 */
 		gcv.first_item = GETQUERY(query);
-		StaticAssertStmt(sizeof(GinTernaryValue) == sizeof(bool),
-						 "sizes of GinTernaryValue and bool are not equal");
 		gcv.check = (GinTernaryValue *) check;
 		gcv.map_item_operand = (int *) (extra_data[0]);
 
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index 2093776809f..3baf5f7d903 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -75,6 +75,14 @@ typedef struct
 	((MaxAllocSize - offsetof(pg_snapshot, xip)) / sizeof(FullTransactionId))
 
 /*
+ * Compile-time limits on the procarray (MAX_BACKENDS processes plus
+ * MAX_BACKENDS prepared transactions) guarantee nxip won't be too large.
+ */
+StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
+				 "possible overflow in pg_current_snapshot()");
+
+
+/*
  * Helper to get a TransactionId from a 64-bit xid with wraparound detection.
  *
  * It is an ERROR if the xid is in the future.  Otherwise, returns true if
@@ -402,13 +410,6 @@ pg_current_snapshot(PG_FUNCTION_ARGS)
 	if (cur == NULL)
 		elog(ERROR, "no active snapshot set");
 
-	/*
-	 * Compile-time limits on the procarray (MAX_BACKENDS processes plus
-	 * MAX_BACKENDS prepared transactions) guarantee nxip won't be too large.
-	 */
-	StaticAssertStmt(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
-					 "possible overflow in pg_current_snapshot()");
-
 	/* allocate */
 	nxip = cur->xcnt;
 	snap = palloc(PG_SNAPSHOT_SIZE(nxip));
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 8b359683539..5f17047047b 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -1040,6 +1040,9 @@ static const struct cachedesc cacheinfo[] = {
 	}
 };
 
+StaticAssertDecl(lengthof(cacheinfo) == SysCacheSize,
+				 "SysCacheSize does not match syscache.c's array");
+
 static CatCache *SysCache[SysCacheSize];
 
 static bool CacheInitialized = false;
@@ -1068,9 +1071,6 @@ InitCatalogCache(void)
 {
 	int			cacheId;
 
-	StaticAssertStmt(lengthof(cacheinfo) == SysCacheSize,
-					 "SysCacheSize does not match syscache.c's array");
-
 	Assert(!CacheInitialized);
 
 	SysCacheRelationOidSize = SysCacheSupportingRelOidSize = 0;
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index b6a8bbcd596..b45a34abcee 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -306,7 +306,7 @@ AllocSetFreeIndex(Size size)
 					tsize;
 
 		/* Statically assert that we only have a 16-bit input value. */
-		StaticAssertStmt(ALLOC_CHUNK_LIMIT < (1 << 16),
+		StaticAssertDecl(ALLOC_CHUNK_LIMIT < (1 << 16),
 						 "ALLOC_CHUNK_LIMIT must be less than 64kB");
 
 		tsize = size - 1;
@@ -358,10 +358,10 @@ AllocSetContextCreateInternal(MemoryContext parent,
 	AllocBlock	block;
 
 	/* ensure MemoryChunk's size is properly maxaligned */
-	StaticAssertStmt(ALLOC_CHUNKHDRSZ == MAXALIGN(ALLOC_CHUNKHDRSZ),
+	StaticAssertDecl(ALLOC_CHUNKHDRSZ == MAXALIGN(ALLOC_CHUNKHDRSZ),
 					 "sizeof(MemoryChunk) is not maxaligned");
 	/* check we have enough space to store the freelist link */
-	StaticAssertStmt(sizeof(AllocFreeListLink) <= (1 << ALLOC_MINBITS),
+	StaticAssertDecl(sizeof(AllocFreeListLink) <= (1 << ALLOC_MINBITS),
 					 "sizeof(AllocFreeListLink) larger than minimum allocation size");
 
 	/*
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index b432a92be31..1f17905b5a3 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -167,7 +167,7 @@ GenerationContextCreate(MemoryContext parent,
 	GenerationBlock *block;
 
 	/* ensure MemoryChunk's size is properly maxaligned */
-	StaticAssertStmt(Generation_CHUNKHDRSZ == MAXALIGN(Generation_CHUNKHDRSZ),
+	StaticAssertDecl(Generation_CHUNKHDRSZ == MAXALIGN(Generation_CHUNKHDRSZ),
 					 "sizeof(MemoryChunk) is not maxaligned");
 
 	/*
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 6df0839b6ac..c2f9bb6ad34 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -151,7 +151,7 @@ SlabContextCreate(MemoryContext parent,
 	int			i;
 
 	/* ensure MemoryChunk's size is properly maxaligned */
-	StaticAssertStmt(Slab_CHUNKHDRSZ == MAXALIGN(Slab_CHUNKHDRSZ),
+	StaticAssertDecl(Slab_CHUNKHDRSZ == MAXALIGN(Slab_CHUNKHDRSZ),
 					 "sizeof(MemoryChunk) is not maxaligned");
 	Assert(MAXALIGN(chunkSize) <= MEMORYCHUNK_MAX_VALUE);