static assert cleanup

On Fri, Dec 9, 2022 at 2:47 PM Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:

0003-Move-some-static-assertions-to-better-places.patch

This moves some that I thought were suboptimally placed but it could be
debated or refined.

+ * We really want ItemPointerData to be exactly 6 bytes.  This is rather a
+ * random place to check, but there is no better place.

Since the assert is no longer in a random function body, it seems we can
remove the second sentence.

--
John Naylor
EDB: http://www.enterprisedb.com

On Fri, Dec 9, 2022 at 6:47 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:

A number of static assertions could be moved to better places.

We first added StaticAssertStmt() in 2012, which required all static
assertions to be inside function bodies. We then added
StaticAssertDecl() in 2020, which enabled static assertions on file
level. We have a number of calls that were stuck in not-really-related
functions for this historical reason. This patch set cleans that up.

0001-Update-static-assert-usage-comment.patch

This updates the usage information in c.h to be more current and precise.

0002-Move-array-size-related-static-assertions.patch

This moves some obviously poorly placed ones.

0003-Move-some-static-assertions-to-better-places.patch

This moves some that I thought were suboptimally placed but it could be
debated or refined.

0004-Use-StaticAssertDecl-where-possible.patch

This just changes some StaticAssertStmt() to StaticAssertDecl() where
appropriate. It's optional.

Patch 0002

diff --git a/src/backend/utils/cache/syscache.c
b/src/backend/utils/cache/syscache.c
index eec644ec84..bb3dd6f4d2 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(SysCacheSize == (int) lengthof(cacheinfo),
+ "SysCacheSize does not match syscache.c's array");
+
 static CatCache *SysCache[SysCacheSize];

In almost every example I found of StaticAssertXXX, the lengthof(arr)
part came first in the condition. Since you are modifying this anyway,
should this one also be reversed for consistency?

======

Patch 0004

diff --git a/src/backend/executor/execExprInterp.c
b/src/backend/executor/execExprInterp.c
index 1dab2787b7..ec26ae506f 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(EEOP_LAST + 1 == lengthof(dispatch_table),
+ StaticAssertDecl(EEOP_LAST + 1 == lengthof(dispatch_table),
  "dispatch_table out of whack with ExprEvalOp");

Ditto the previous comment.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

On 11.12.22 23:18, Peter Smith wrote:

+StaticAssertDecl(SysCacheSize == (int) lengthof(cacheinfo),
+ "SysCacheSize does not match syscache.c's array");
+
static CatCache *SysCache[SysCacheSize];

In almost every example I found of StaticAssertXXX, the lengthof(arr)
part came first in the condition. Since you are modifying this anyway,
should this one also be reversed for consistency?

Makes sense. I have pushed this separately.

On 09.12.22 11:01, John Naylor wrote:

On Fri, Dec 9, 2022 at 2:47 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com
<mailto:peter.eisentraut@enterprisedb.com>> wrote:

0003-Move-some-static-assertions-to-better-places.patch

This moves some that I thought were suboptimally placed but it could be
debated or refined.

+ * We really want ItemPointerData to be exactly 6 bytes.  This is rather a
+ * random place to check, but there is no better place.

Since the assert is no longer in a random function body, it seems we can
remove the second sentence.

Committed with the discussed adjustments.