Macro nesting hell

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Macro nesting hell
Дата
Msg-id 4407.1435763473@sss.pgh.pa.us
обсуждение исходный текст
Ответы Re: Macro nesting hell  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
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, %ebpjns    .L80movq    LocalBufferBlockPointers(%rip), %raxmovq    40(%rsp), %rdxmovq    (%rax,%rdx),
%raxjmp   .L81
 
.L80:movq    24(%rsp), %raxaddq    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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Sawada Masahiko
Дата:
Сообщение: Re: Support for N synchronous standby servers - take 2
Следующее
От: Sawada Masahiko
Дата:
Сообщение: Re: Freeze avoidance of very large table.