Re: Macro nesting hell

On 2015-07-01 12:55:48 -0300, Alvaro Herrera wrote:
> Tom Lane wrote:
> > 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:

I just hit this in clang which also warns about too long literals unless
you silence it...

> Wow, that's kind of amazing. I think this particular case boils down to
> just PageGetSpecialPointer (bufpage.h) and BufferGetBlock (bufmgr.h).

Inlining just BufferGetBlock already helps sufficiently to press it
below 4k (the standard's limit IIRC), but that doesn't mean we shouldn't
go a bit further.

> > I'm thinking we really ought to mount a campaign to replace some of these
> > macros with inlined-if-possible functions.
>
> My guess is that changing a very small amount of them will do a large
> enough portion of the job.

I think it'd be good to convert the macros in bufpage.h and bufmgr.h
that either currently have multiple evaluation hazards, or have a
AssertMacro() in them. The former for obvious reasons, the latter
because that immediately makes them rather large (on and it implies
multiple evaluation hazards anyway).

That'd mean inlining PageSetPageSizeAndVersion(), PageGetSpecialSize(),
PageGetSpecialPointer(), PageGetItem(), PageGetMaxOffsetNumber(),
PageIsPrunable(), PageSetPrunable(), PageClearPrunable(),
BufferIsValid(), BufferGetBlock(), BufferGetPageSize().

Greetings,

Andres Freund