Re: Raising our compiler requirements for 9.6 - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Raising our compiler requirements for 9.6
Date
Msg-id CA+TgmobJRofKvoGzqO2agg_C7q3mrgPWnkN8hHSyEiw6=isNdw@mail.gmail.com
Whole thread Raw
In response to Re: Raising our compiler requirements for 9.6  (Andres Freund <andres@anarazel.de>)
Responses Re: Raising our compiler requirements for 9.6
Re: Raising our compiler requirements for 9.6
List pgsql-hackers
On Sat, Aug 15, 2015 at 8:03 PM, Andres Freund <andres@anarazel.de> wrote:
>> That gave me new respect for STATIC_IF_INLINE.  While it does add tedious work
>> to the task of introducing a new batch of inline functions, the work is
>> completely mechanical.  Anyone can write it; anyone can review it; there's one
>> correct way to write it.
>
> What's the advantage of using STATIC_IF_INLINE over just #ifndef
> FRONTEND? That doesn't work well for entire headers in my opinion, but
> for individual functions it shouldn't be a problem? In fact we've done
> it for years for MemoryContextSwitchTo(). And by the problem definition
> such functions cannot be required by frontend code.
>
> STATIC_IF_INLINE is really tedious because to know whether it works you
> essentially need to recompile postgres with inlines enabled/disabled.
>
> In fact STATIC_IF_INLINE does *not* even help here unless we also detect
> compilers that support inlining but balk when inline functions reference
> symbols not available. There was an example of that in the buildfarm as
> of 2014, c.f. a9baeb361d63 . We'd have to disable inlining for such
> compilers.

The advantage of STATIC_IF_INLINE is that we had everything working in
that model.  We seem to be replacing problems that we had solved and
code that worked on our entire buildfarm with new problems that we
haven't solved yet and which don't seem to be a whole lot simpler than
the ones they replaced.

As far as I can see, the anticipated benefits of what we're doing here are:

- Get a cleaner separation of frontend and backend headers (this could
also be done independently of STATIC_IF_INLINE, but removing
STATIC_IF_INLINE increases the urgency).
- Eliminate multiple evaluations hazards.
- Modest improvements to code generation.

And the costs are:

- Lots of warnings with compilers that are not smart about static
inline, and probably significantly worse code generation.
- The possibility that may repeatedly break #define FRONTEND
compilation when we add static inline functions, where instead adding
macros would not have caused breakage, thus resulting in continual
tinkering with the header files.

What I'm basically worried about is that second one.  Actually, what
I'm specifically worried about is that we will break somebody's #ifdef
FRONTEND code, they will eventually complain, and we will refuse to
fix it because we don't think their use case is valid.  If that
happens even a few times, it will lead me to think that this whole
effort was misguided.  :-(

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Neil Conway
Date:
Subject: Re: Memory allocation in spi_printtup()
Next
From: Andres Freund
Date:
Subject: Re: Raising our compiler requirements for 9.6