From: Tom Lane <tgl@sss.pgh.pa.us>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: allow benign typedef redefinitions (C11)
Date: Mon, 29 Sep 2025 13:08:30 -0400
Message-ID: <214791.1759165710@sss.pgh.pa.us> (raw)
In-Reply-To: <202509291356.o5t6ny2hoa3q@alvherre.pgsql>
References: <202509291356.o5t6ny2hoa3q@alvherre.pgsql>
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> I was thinking about your (Tom's) idea of having some sort of header
> inclusion policy. Our current situation is that cross-module inclusions
> are quite widespread and the dependencies can probably be seen as a
> tight web (modules probably being defined as our subdirectories inside
> src/include). A blanket prohibition of inclusion of headers of other
> modules would certainly not work, but if we imagine that within each
> module we have a hierarchy of sorts, then it would make sense to have a
> policy that other modules can include high level headers of other
> modules, but not lower-level headers. For instance, it's okay if
> files in src/include/executor include high-level brin.h, but it's not
> okay if it has to include brin_tuple.h which is supposed to be
> lower-level.
Yeah, I've been thinking about that off-and-on, and don't have
anything to present yet either.
> With that in mind, I gave another long stare to the doxygen reports for
> some of these files. It now seems to me that removing tidbitmap.h from
> genam.h is somewhat bogus, because after all tidbitmap.h is a legitimate
> "library" which is okay to be included in other headers, and the real
> bug here is the fact that only very recently (commit bfe56cdf9a4e in Feb
> 2025) it acquired htup_details.h just in order to be able to define
> TBM_MAX_TUPLES_PER_PAGE. If we remove htup_details.h from tidbitmap.h,
> and we also remove the inclusion of relcache.h by adding a typedef for
> Relation, then genam.h is a much better behaved header than before.
> Hence the attached patches.
These patches seem fine to me.
> Another thing we should look into is splitting the ObjectType enum out
> of parsenodes.h into a new file of its own. We have objectaddress.h
> depending on the whole of parsenodes.h just to have that enum, for
> instance. I think that would be useful.
+1. I think that primnodes.h and parsenodes.h are always going to
need to be included by a whole lot of things, but to the extent that
we can remove those inclusions from relatively low-level headers,
it can't hurt.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@kurilemu.de, peter@eisentraut.org
Subject: Re: allow benign typedef redefinitions (C11)
In-Reply-To: <214791.1759165710@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox