Re: cpluspluscheck complains about use of register

Hi,

On 2022-03-08 10:59:02 -0800, Andres Freund wrote:
> On 2022-03-08 13:46:36 -0500, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > When running cpluspluscheck I get many many complaints like
> > > /tmp/pg-test-repo/src/include/port/atomics/arch-x86.h:143:23: warning: ISO C++17 does not allow ‘register’
storageclass specifier [-Wregister]
 
> >
> > Interesting, I don't see that here.
> 
> Probably a question of the gcc version. I think starting with 11 g++ defaults
> to C++ 17.
> 
> 
> > > It seems we should just remove the use of register?
> >
> > I have a vague idea that it was once important to say "register" if
> > you are going to use the variable in an asm snippet that requires it
> > to be in a register.  That might be wrong, or it might be obsolete
> > even if once true.  We could try taking these out and seeing if the
> > buildfarm complains.
> 
> We have several inline asm statements not using register despite using
> variables in a register (e.g. pg_atomic_compare_exchange_u32_impl()), so I
> wouldn't expect a problem with compilers we support.
> 
> Should we make configure test for -Wregister? There's at least one additional
> use of register that we'd have to change (pg_regexec).
> 
> 
> > (If so, maybe -Wno-register would help?)
> 
> That's what I did to work around the flood of warnings locally, so it'd
> work.

I hit this again while porting cplupluscheck to be invoked by meson as
well. ISTM that we should just remove the uses of register. Yes, some very old
compilers might generate worse code without register, but I don't think we
need to care about peak efficiency with neolithic compilers.

Fabien raised the concern that removing register might lead to accidentally
adding pointers to such variables - I don't find that convincing, because a)
such code is typically inside a helper inline anyway b) we don't use register
widely enough to ensure this.


Attached is a patch removing uses of register. The use in regexec.c could
remain, since we only try to keep headers C++ clean. But there really doesn't
seem to be a good reason to use register in that spot.

I tried to use -Wregister to keep us honest going forward, but unfortunately
it only works with a C++ compiler...

I tested this by redefining register to something else, and I grepped for
non-comment uses of register. Entirely possible that I missed something.

Greetings,

Andres Freund