Re: Proposal: Add more compile-time asserts to exposeinconsistencies. - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Proposal: Add more compile-time asserts to exposeinconsistencies.
Date
Msg-id 20200131.141542.898059852336948855.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Proposal: Add more compile-time asserts to exposeinconsistencies.  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Proposal: Add more compile-time asserts to exposeinconsistencies.  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
At Fri, 31 Jan 2020 11:47:01 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Tue, Dec 24, 2019 at 02:47:29PM +0900, Michael Paquier wrote:
> > I am still seeing that a couple of points need an extra lookup:
> > - Addition of a Decl() in at least one header of the core code.
> 
> I agree with the addition of Decl() definition in a header, and could
> not think about something better than one for bufpage.h for the
> all-zero check case, so I went with that.  Attached is a 0001 which
> adds the definition for StaticAssertDecl() for C and C++ for all code
> paths.  If there are no objections, I would like to commit this
> version.  There is no fancy refactoring in it, and small progress is
> better than no progress.  I have also reworked the comments in the
> patch, and did some testing on Windows.

As a cross check, it cleanly applied and worked as expected. The
fallback definition of StaticAssertDecl for C worked for gcc 8.3.


- * Macros to support compile-time assertion checks.
+ * Macros to support compile-time and declaration assertion checks.

All the StaticAssert things check compile-time assertion.  I think
that the name StaticAssertDecl doesn't mean "declaration assertion",
but means "static assertion as a declaration". Is the change needed?


- * If the "condition" (a compile-time-constant expression) evaluates to false,
- * throw a compile error using the "errmessage" (a string literal).
+ * If the "condition" (a compile-time-constant or declaration expression)
+ * evaluates to false, throw a compile error using the "errmessage" (a
+ * string literal).

I'm not sure what the "declaration expression" here means.  I think
the term generally means "a variable declaration in expressions",
something like "r = y * (int x = blah)".  In that sense, the parameter
for StaticAssertDecl is just a compile-time constant expression. Is it
a right rewrite?

> > - Perhaps unifying the fallback implementation between C and C++, with
> > a closer lookup in particular at StaticAssertStmt() and StaticAssertExpr().
> 
> Seeing nothing happening on this side.  I took a shot at all that, and
> I have hacked my way through it with 0002 which is an attempt to unify
> the fallback implementation for C and C++.  This is not fully baked
> yet, and it is perhaps a matter of taste if this makes the code more
> readable or not.  I think it does, because it reduces the parts
> dedicated to assertion definitions from four to three.  Anyway, let's
> discuss about that.

+1 as far as the unification is right.  I'm not sure, but at least it
worked for gcc 8.3.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [Patch] Make pg_checksums skip foreign tablespace directories
Next
From: Michael Paquier
Date:
Subject: Re: MSVC installs too much stuff?