Hi, On Tue, Feb 03, 2026 at 09:09:17AM +0100, Peter Eisentraut wrote: > On 27.01.26 13:55, Bertrand Drouvot wrote: > > Hi, > > > > On Mon, Jan 26, 2026 at 01:17:15PM +0100, Peter Eisentraut wrote: > > > I'm proposing two changes: > > > > > > First, rename AssertVariableIsOfType to StaticAssertVariableIsOfType. The > > > current name suggests that it is a run-time assertion (like "Assert"), but > > > it's not. The name change makes that clearer. > > > > > > I doubt that the current name is used in many extensions, but if > > > necessary, > > > extension code could adapt to this quite easily with something like > > > > > > #if PG_VERSION_NUM < ... > > > #define StaticAssertVariableIsOfType(x, y) AssertVariableIsOfType(x, y) > > > #endif > > > > > > Second, change the underlying implementation of > > > StaticAssertVariableIsOfType > > > to use StaticAssertDecl instead of StaticAssertStmt. This makes > > > StaticAssertVariableIsOfType behave more like a normal static assertion, > > > and > > > in many cases we can move the current instances to a more natural position > > > at file scope. This is similar to previous commits like 493eb0da31b. > > > > Both make sense and looks good to me. > > Thanks, committed. > > > Once they are in, I'm wondering if the remaining StaticAssertStmt ones: > > > > src/backend/backup/basebackup.c: StaticAssertStmt(2 * > > TAR_BLOCK_SIZE <= BLCKSZ, > > src/backend/storage/lmgr/deadlock.c: StaticAssertStmt(MAX_BACKENDS_BITS > > <= (32 - 3), > > src/backend/utils/mmgr/aset.c: StaticAssertStmt(ALLOC_CHUNK_LIMIT == > > ALLOCSET_SEPARATE_THRESHOLD, > > > > could be replaced by StaticAssertDecl() too (that has not been done in > > 493eb0da31b > > and (from a quick scan) not mentioned in the linked thread). I did not look > > in > > details so maybe there is good reasons to keep them. > > Yeah, maybe it would be good to get rid of these remaining few. I suppose > we could just change Stmt to Decl and put braces around the block, but maybe > there are some more elegant places to move these.
Yeah, I gave it a try and I did not choose the same place for all the files. 1/ basebackup.c Since changing the remaining StaticAssertStmt to StaticAssertDecl introduces a duplicate, I thought it would make sense to: - remove the StaticAssertStmt - move the existing StaticAssertDecl at file scope As it depends of the literal "2" also used in some computation then I introduced TAR_TERMINATION_BLOCKS and used it in the StaticAssertDecl and the functions. 2/ deadlock.c It makes sense to keep it near the related code, so: - changed to StaticAssertDecl - Added new braces to avoid Wdeclaration-after-statement to trigger 3/ aset.c Changes the StaticAssertStmt to StaticAssertDecl and move it to file scope (that looks more appropriate). Attached 3 patches to ease the review. After there are no remaining usages of StaticAssertStmt() and we may want to deprecate it. Thoughts? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From f032b3acdf30e4632cb6d8e7e88f6dee9e0e0dc8 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Tue, 3 Feb 2026 10:32:30 +0000 Subject: [PATCH v1 1/3] Change StaticAssertStmt to StaticAssertDecl in basebackup.c Changing the StaticAssertStmt to StaticAssertDecl results in having the same StaticAssertDecl() in 2 functions. So, it makes more sense to move it to a file scope instead. Also as it depends on some computations based on 2 tar blocks, then define TAR_TERMINATION_BLOCKS. --- src/backend/backup/basebackup.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) 100.0% src/backend/backup/ diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index 463c0756b5e..6200777b032 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -78,6 +78,11 @@ typedef struct pg_checksum_type manifest_checksum_type; } basebackup_options; +#define TAR_TERMINATION_BLOCKS 2 + +StaticAssertDecl(TAR_TERMINATION_BLOCKS * TAR_BLOCK_SIZE <= BLCKSZ, + "BLCKSZ too small for " CppAsString2(TAR_TERMINATION_BLOCKS) " tar termination blocks"); + static int64 sendTablespace(bbsink *sink, char *path, Oid spcoid, bool sizeonly, struct backup_manifest_info *manifest, IncrementalBackupInfo *ib); @@ -382,10 +387,8 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, else { /* Properly terminate the tarfile. */ - 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); + memset(sink->bbs_buffer, 0, TAR_TERMINATION_BLOCKS * TAR_BLOCK_SIZE); + bbsink_archive_contents(sink, TAR_TERMINATION_BLOCKS * TAR_BLOCK_SIZE); /* OK, that's the end of the archive. */ bbsink_end_archive(sink); @@ -635,10 +638,8 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, } /* Properly terminate the tar file. */ - StaticAssertStmt(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); + memset(sink->bbs_buffer, 0, TAR_TERMINATION_BLOCKS * TAR_BLOCK_SIZE); + bbsink_archive_contents(sink, TAR_TERMINATION_BLOCKS * TAR_BLOCK_SIZE); /* OK, that's the end of the archive. */ bbsink_end_archive(sink); -- 2.34.1
>From e4031bae0878602bc440c42d11fabdcbe2b76dc1 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Tue, 3 Feb 2026 10:50:20 +0000 Subject: [PATCH v1 2/3] Change StaticAssertStmt to StaticAssertDecl in deadlock.c Changing the StaticAssertStmt to StaticAssertDecl and keep in the function scope. Adding new braces to avoid Wdeclaration-after-statement to trigger. --- src/backend/storage/lmgr/deadlock.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) 100.0% src/backend/storage/lmgr/ diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index 8334a887618..fa03e50ce80 100644 --- a/src/backend/storage/lmgr/deadlock.c +++ b/src/backend/storage/lmgr/deadlock.c @@ -192,11 +192,13 @@ InitDeadLockChecking(void) * last MaxBackends entries in possibleConstraints[] are reserved as * output workspace for FindLockCycle. */ - StaticAssertStmt(MAX_BACKENDS_BITS <= (32 - 3), - "MAX_BACKENDS_BITS too big for * 4"); - maxPossibleConstraints = MaxBackends * 4; - possibleConstraints = - (EDGE *) palloc(maxPossibleConstraints * sizeof(EDGE)); + { + StaticAssertDecl(MAX_BACKENDS_BITS <= (32 - 3), + "MAX_BACKENDS_BITS too big for * 4"); + maxPossibleConstraints = MaxBackends * 4; + possibleConstraints = + (EDGE *) palloc(maxPossibleConstraints * sizeof(EDGE)); + } MemoryContextSwitchTo(oldcxt); } -- 2.34.1
>From a44bb97b4a0094a6456a33eacb1d330d727390b9 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Tue, 3 Feb 2026 13:01:12 +0000 Subject: [PATCH v1 3/3] Change StaticAssertStmt to StaticAssertDecl in aset.c Changing the StaticAssertStmt to StaticAssertDecl and move it to file scope. --- src/backend/utils/mmgr/aset.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) 100.0% src/backend/utils/mmgr/ diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index ae7d1647aea..161c2e2d3df 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -87,6 +87,10 @@ #define ALLOC_CHUNK_FRACTION 4 /* We allow chunks to be at most 1/4 of maxBlockSize (less overhead) */ +/* ALLOC_CHUNK_LIMIT must be equal to ALLOCSET_SEPARATE_THRESHOLD */ +StaticAssertDecl(ALLOC_CHUNK_LIMIT == ALLOCSET_SEPARATE_THRESHOLD, + "ALLOC_CHUNK_LIMIT != ALLOCSET_SEPARATE_THRESHOLD"); + /*-------------------- * The first block allocated for an allocset has size initBlockSize. * Each time we have to allocate another block, we double the block size @@ -501,12 +505,6 @@ AllocSetContextCreateInternal(MemoryContext parent, * requests that are all the maximum chunk size we will waste at most * 1/8th of the allocated space. * - * Also, allocChunkLimit must not exceed ALLOCSET_SEPARATE_THRESHOLD. - */ - StaticAssertStmt(ALLOC_CHUNK_LIMIT == ALLOCSET_SEPARATE_THRESHOLD, - "ALLOC_CHUNK_LIMIT != ALLOCSET_SEPARATE_THRESHOLD"); - - /* * Determine the maximum size that a chunk can be before we allocate an * entire AllocBlock dedicated for that chunk. We set the absolute limit * of that size as ALLOC_CHUNK_LIMIT but we reduce it further so that we -- 2.34.1
