Re: Raising our compiler requirements for 9.6

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Noah Misch <noah(at)leadboat(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Raising our compiler requirements for 9.6
Date: 2015-08-06 10:56:47
Message-ID: 20150806105647.GG12214@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
0001-Don-t-include-low-level-locking-code-from-frontend-c.patch text/x-patch 7.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Antonin Houska 2015-08-06 13:04:28 Thinko in processing of SHM message size info?
Previous Message Ildus Kurbangaliev 2015-08-06 10:01:15 Re: RFC: replace pg_stat_activity.waiting with something more descriptive