On Wed, Jun 17, 2020 at 5:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> See the "Default Definitions", down near the end.
Oh, yeah. I didn't realize you meant just inside this file itself.
That is slightly awkward. I initially thought there was no problem
because there seem to be no remaining non-default definitions of
S_LOCK, but I now see that the other macros still do have some
non-default definitions. Hmm.
> > Really? Multiple layers of macros seem like they pretty clearly make
> > the source code harder to understand. There are plenty of places where
> > such devices are necessary for one reason or another, but it doesn't
> > seem like something we ought to keep around for no reason.
>
> I wouldn't object to making the outer-layer macros in spin.h into static
> inlines; as mentioned, that might have some debugging benefits. But I
> think messing with s_lock.h for marginal cosmetic reasons is a foolish
> idea. For one thing, there's no way whoever does it can verify all the
> architecture-specific stanzas. (I don't think we even have all of them
> covered in the buildfarm.)
It would be a pretty mechanical change to use a separate preprocessor
symbol for the conditional and just define the static inline functions
on the spot. There might be one or two goofs, but if those platforms
are not in the buildfarm, they're either dead and they don't matter,
or someone will tell us what we did wrong. I don't know. I don't have
a huge desire to spend time cleaning up s_lock.h and I do think it's
better not to churn stuff around just for the heck of it, but I'm also
sympathetic to Andres's point that using macros everywhere is
debugger-unfriendly.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company