Re: Proposal: Add more compile-time asserts to expose inconsistencies.

On 2019-12-04 15:16:25 +0900, Michael Paquier wrote:
> On Mon, Dec 02, 2019 at 07:55:45AM -0800, Andres Freund wrote:
> > Now that I'm back from vacation, I'll try to take a stab at b). It
> > should definitely doable to use the same approach for StaticAssertStmt,
> > the problematic case might be StaticAssertExpr.
>
> So you basically want to minimize the amount of code relying on
> external compiler expressions? Sounds like a plan. At quick glance,
> it seems that this should work. I haven't tested though. I'll wait
> for what you come up with then.

I don't know what you mean by "external compiler expressions"?

> >> +StaticAssertDecl(lengthof(LockTagTypeNames) == (LOCKTAG_ADVISORY + 1),
> >> + "LockTagTypeNames array inconsistency");
> >> +
> >
> > These error messages strike me as somewhat unhelpful. I'd probably just
> > reword them as "array length mismatch" or something like that.
>
> That's indeed better. Now I think that it is useful to have the
> structure name in the error message as well, no?

No. I think the cost of having the different error messages is much
higher than the cost of not having the struct name in there. Note that
you'll commonly get an error message including the actual source code
for the offending expression.

> > I think this patch ought to include at least one StaticAssertDecl in a
> > header, to make sure we get that part right across compilers. E.g. the
> > one in PageIsVerified()?
>
> No objections to have one, but I don't think that your suggestion is a
> good choice. This static assertion is based on size_t and BLCKSZ, and
> is located close to a code path where we have a specific logic based
> on both things.

Well, but it's a reliance that goes beyond that specific source code
location

> If in the future this code gets removed, then we'd likely miss to
> remove the static assertion if they are separated across multiple
> files.

It'll never get removed. There's just plainly no way that we'd use a
block size that's not a multiple of size_t.

Greetings,

Andres Freund