Macro nesting hell

Last night my ancient HP compiler spit up on HEAD:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pademelon&dt=2015-07-01%2001%3A30%3A18
complaining thus:
cpp: "brin_pageops.c", line 626: error 4018: Macro param too large after substitution - use -H option.
I was able to revive pademelon by adding a new compiler flag as suggested,
but after looking at what the preprocessor is emitting, I can't say that
I blame it for being unhappy. This simple-looking line

Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf)));

is expanding to this:

do { if (!(((((BrinSpecialSpace *) ( ((void) ((bool) (! (!(((const void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) != ((void *)0)))) || (ExceptionalCondition("!(((const void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) !
>= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) != ((void *)0)))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((void) ((bool) (! (!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >!
= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)) || (ExceptionalCondition("!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Bl!
ock) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((void) ((bool) (! (!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special >= (__builtin_offsetof (PageHeaderData, pd_linp)))) || (ExceptionalCondition("!(((PageHeader) (((Page)( ((void) ((bool) (!!
(!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special >= (__builtin_offsetof (PageHeaderData, pd_linp)))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (char *) ((char *) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldb!
uf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) + ((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition("!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)", ("FailedAssertion"), "brin_pageops.c", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition("!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))", ("FailedAssertion!
"), "brin_pageops.c", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special) ))->vector[(((uintptr_t) ((1)) + ((8) - 1)) & ~((uintptr_t) ((8) - 1))) / sizeof(uint16) - 1]) == 0xF093))) ExceptionalCondition("!(((((BrinSpecialSpace *) ( ((void) ((bool) (! (!(((const void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) *!
8192) ))) != ((void *)0)))) || (ExceptionalCondition(\"!(((const void*)(((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\\\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\\\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\\\\\", (\\\\\\\"FailedAssertion\\\\\\\"), \\\\\\\"brin_pageops.c\\\\\\\", 626), 0)))), (oldbuf) != 0 ))\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) != ((void *)0)))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((void) ((bool) (! (!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool!
) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)) || (ExceptionalCondition(\"!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\\\"!(( ((void) ((bool) (! (!((oldb!
uf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\\\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\\\\\", (\\\\\\\"FailedAssertion\\\\\\\"), \\\\\\\"brin_pageops.c\\\\\\\", 626), 0)))), (oldbuf) != 0 ))\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special <= 8192)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((void) ((bool) (! (!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\",!
(\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special >= (__builtin_offsetof (PageHeaderData, pd_linp)))) || (ExceptionalCondition(\"!(((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\\\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\\\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\\\\\", (\\\\\\\"FailedAssertion\\\\\\\"), \\\\\\\"brin_pageops.c\\\\\\\", 626), 0)))), (oldbuf) != 0 ))\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - !
1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special >= (__builtin_offsetof (PageHeaderData, pd_linp)))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (char *) ((char *) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))) + ((PageHeader) (((Page)( ((void) ((bool) (! (!(( ((void) ((bool) (! (!((!
oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), (oldbuf) != 0 ))) || (ExceptionalCondition(\"!(( ((void) ((bool) (! (!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)) || (ExceptionalCondition(\\\"!((oldbuf) <= NBuffers && (oldbuf) >= -NLocBuffer)\\\", (\\\"FailedAssertion\\\"), \\\"brin_pageops.c\\\", 626), 0)))), (oldbuf) != 0 ))\", (\"FailedAssertion\"), \"brin_pageops.c\", 626), 0)))), ((oldbuf) < 0) ? LocalBufferBlockPointers[-(oldbuf) - 1] : (Block) (BufferBlocks + ((Size) ((oldbuf) - 1)) * 8192) ))))->pd_special) ))->vector[(((uintptr_t) ((1)) + ((8) - 1)) & ~((uintptr_t) ((8) - 1))) / sizeof(uint16) - 1]) == 0xF093))", ("FailedAssertion"), "brin_pageops.c", 626); } while (0);

The problem here is that we've got several nested levels of macros that
each feel free to evaluate their arguments multiple times. Quite aside
from the risk of breaking toolchains, this has got to be inefficient.
A quick look at the generated source code shows five separate occurrences
of the sequence

testl %ebp, %ebp
jns .L80
movq LocalBufferBlockPointers(%rip), %rax
movq 40(%rsp), %rdx
movq (%rax,%rdx), %rax
jmp .L81
.L80:
movq 24(%rsp), %rax
addq BufferBlocks(%rip), %rax

corresponding to the BufferGetBlock() macro. At least gcc seems to
have figured out that it only needed to test BufferIsValid(buffer)
once, but still, this is awful.

And then there's the risk of outright bugs from multiple evaluations
of arguments.

I'm thinking we really ought to mount a campaign to replace some of these
macros with inlined-if-possible functions.

regards, tom lane