rename and move AssertVariableIsOfType
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.
Attachments:
0002-Change-StaticAssertVariableIsOfType-to-be-a-declarat.patchtext/plain; charset=UTF-8; name=0002-Change-StaticAssertVariableIsOfType-to-be-a-declarat.patchDownload+28-24
0001-Rename-AssertVariableIsOfType-to-StaticAssertVariabl.patchtext/plain; charset=UTF-8; name=0001-Rename-AssertVariableIsOfType-to-StaticAssertVariabl.patchDownload+67-68
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)
#endifSecond, 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.
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.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
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)
#endifSecond, 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.
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)
#endifSecond, 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
Attachments:
v1-0001-Change-StaticAssertStmt-to-StaticAssertDecl-in-ba.patchtext/x-diff; charset=us-asciiDownload+9-9
v1-0002-Change-StaticAssertStmt-to-StaticAssertDecl-in-de.patchtext/x-diff; charset=us-asciiDownload+7-6
v1-0003-Change-StaticAssertStmt-to-StaticAssertDecl-in-as.patchtext/x-diff; charset=us-asciiDownload+4-7
On 03.02.26 14:39, Bertrand Drouvot wrote:
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 scopeAs 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 trigger3/ 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.
I have committed this.
I changed TAR_TERMINATION_BLOCKS to TAR_NUM_TERMINATION_BLOCKS for a
little bit more clarity. And then I added a comment in c.h that
StaticAssertStmt() is deprecated.
Hi,
On Mon, Feb 16, 2026 at 09:40:28AM +0100, Peter Eisentraut wrote:
On 03.02.26 14:39, Bertrand Drouvot wrote:
After there are no remaining usages of StaticAssertStmt() and we may want to
deprecate it.I have committed this.
Thanks!
And then I added a comment in c.h that StaticAssertStmt() is deprecated.
I think that could be also a good candidate for using pg_attribute_deprecated()
proposed in [1]/messages/by-id/aRGa87Ab0f3ItWRV@ip-10-97-1-34.eu-west-3.compute.internal (cc-ing Álvaro as I think he had in mind to add the
pg_attribute_deprecated() to the tree).
Regards,
[1]: /messages/by-id/aRGa87Ab0f3ItWRV@ip-10-97-1-34.eu-west-3.compute.internal
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com