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

From Andres Freund
Subject Re: Raising our compiler requirements for 9.6
Date
Msg-id 20150806105647.GG12214@awork2.anarazel.de
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  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 2015-08-06 09:09:02 +0200, Andres Freund wrote:
> It really doesn't. It's just fallout from indirectly including lwlock.h
> which includes an atomic variable. The include path leading to it is
>
> In file included from /home/andres/src/postgresql/src/include/storage/lwlock.h:19:0,
>                  from /home/andres/src/postgresql/src/include/storage/lock.h:18,
>                  from /home/andres/src/postgresql/src/include/access/tuptoaster.h:18,
>                  from /home/andres/src/postgresql/src/bin/pg_resetxlog/pg_resetxlog.c:49:
> /home/andres/src/postgresql/src/include/port/atomics.h:41:2: error: #error "THOU SHALL NOT REQUIRE ATOMICS"
>  #error "THOU SHALL NOT REQUIRE ATOMICS"
>
> There's other path's (slot.h) as well if that were fixed.
>
> > Should these things have a different, stub implementation for FRONTEND
> > code?
>
> I'm honestly not yet sure what the best approach here would be.
>
> One approach is to avoid including lwlock.h/slot.h in frontend
> code. That'll require some minor surgery and adding a couple includes,
> but it doesn't look that bad.

Patch doing that attached.

It's easy enough right now, but I'm not entirely sure how feasible it is
going forward. I mean it's rather good if frontends do not end up
including s_lock.h, lwlock.h, atomics.h and such. But if some more code
ends up using lwlocks inline instead of referencing them that might get
harder. On the other hand no code doing that has business being included
by frontend code.  In the end I think that this separation is worth some
pain.

As a consequence I think we should actually add a bunch of #ifdef
FRONTEND #error checks in code we definitely do not want to included in
frontend code.  The attached patch so far adds a check to atomics.h,
lwlock.h and s_lock.h.

Before ea9df812d - "Relax the requirement that all lwlocks be stored in
a single array." it was kinda ok that lock.h includes lwlock.h since
that didn't expose its implementation details much. But after that it
started to need s_lock.h exposed (and now atomics.h as well). I think
that changed the game somewhat.

Comments?

- Andres

Attachment

pgsql-hackers by date:

Previous
From: Ildus Kurbangaliev
Date:
Subject: Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Next
From: Antonin Houska
Date:
Subject: Thinko in processing of SHM message size info?