Feedback-ID: ie3de48e3:Fastmail
Date: Sun, 28 Sep 2025 18:54:23 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter@eisentraut.org>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: allow benign typedef redefinitions (C11)
Message-ID: <202509281643.jcbwanjuy7yh@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="smaq5tawmbjl6jcu"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <832524.1759070119@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/202509281643.jcbwanjuy7yh%40alvherre.pgsql>
Precedence: bulk


--smaq5tawmbjl6jcu
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2025-Sep-28, Tom Lane wrote:

> =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de> writes:
> > The changes to inet.h and varbit.h are surprising, but required.
> 
> Why?  Those headers compile now, and you've not removed any includes
> that they might indirectly depend on.

That's true.

> If what you mean is that some of the macros in these headers require
> varatt.h to be used, I do not agree that that means we should put
> varatt.h into these headers.  We've frequently relied on exactly
> that property of macro definitions to avoid excessive or circular
> inclusions.  Moreover, even if we wanted to change that policy,
> I know of no simple way to check it.

We used to have a script called pgdefine that would create a .c file
that would expand all macros defined in a header, and then ensure that
the C file compiled.  It was removed only recently (Dec. 2024), in
commit 5af699066f81, though it's true that apparently nobody had used in
a while.

> I think what we're trying to do here is reduce the #includes of
> header files to be just enough to compile the headers themselves.
> That will bleed out into more #include's required in .c files,
> but that's fine by me.

Ok, that requires adding varatt.h to just two .c files -- attached.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

--smaq5tawmbjl6jcu
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="v2-0001-Remove-genam.h-s-dependency-on-relcache.h-and-tid.patch"

From 5860cc0d4a1e4f9eace2eb521f0f7416e97709ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Sun, 28 Sep 2025 11:15:51 +0200
Subject: [PATCH v2] Remove genam.h's dependency on relcache.h and tidbitmap.h

---
 contrib/btree_gist/btree_bit.c         | 1 +
 src/backend/utils/adt/network_spgist.c | 1 +
 src/include/access/amapi.h             | 2 ++
 src/include/access/genam.h             | 6 ++++--
 src/include/access/nbtree.h            | 1 +
 5 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/contrib/btree_gist/btree_bit.c b/contrib/btree_gist/btree_bit.c
index 0df2ae20d8b..9199f886097 100644
--- a/contrib/btree_gist/btree_bit.c
+++ b/contrib/btree_gist/btree_bit.c
@@ -8,6 +8,7 @@
 #include "utils/fmgrprotos.h"
 #include "utils/sortsupport.h"
 #include "utils/varbit.h"
+#include "varatt.h"
 
 /* GiST support functions */
 PG_FUNCTION_INFO_V1(gbt_bit_compress);
diff --git a/src/backend/utils/adt/network_spgist.c b/src/backend/utils/adt/network_spgist.c
index 602276a35c3..a84747d9275 100644
--- a/src/backend/utils/adt/network_spgist.c
+++ b/src/backend/utils/adt/network_spgist.c
@@ -37,6 +37,7 @@
 #include "catalog/pg_type.h"
 #include "utils/fmgrprotos.h"
 #include "utils/inet.h"
+#include "varatt.h"
 
 
 static int	inet_spg_node_number(const inet *val, int commonbits);
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index 2b4482dc1e6..63dd41c1f21 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -15,6 +15,8 @@
 #include "access/cmptype.h"
 #include "access/genam.h"
 #include "access/stratnum.h"
+#include "nodes/nodes.h"
+#include "nodes/pg_list.h"
 
 /*
  * We don't wish to include planner header files here, since most of an index
diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index ac62f6a6abd..e23eb04650b 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -17,15 +17,17 @@
 #include "access/htup.h"
 #include "access/sdir.h"
 #include "access/skey.h"
-#include "nodes/tidbitmap.h"
 #include "storage/buf.h"
 #include "storage/lockdefs.h"
-#include "utils/relcache.h"
 #include "utils/snapshot.h"
 
 /* We don't want this file to depend on execnodes.h. */
 typedef struct IndexInfo IndexInfo;
 typedef struct TupleTableSlot TupleTableSlot;
+/* or tidbitmap.h */
+typedef struct TIDBitmap TIDBitmap;
+/* or relcache.h */
+typedef struct RelationData *Relation;
 
 
 /*
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 9ab467cb8fd..db1345f54c8 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -22,6 +22,7 @@
 #include "catalog/pg_index.h"
 #include "lib/stringinfo.h"
 #include "storage/bufmgr.h"
+#include "storage/dsm.h"
 #include "storage/shm_toc.h"
 #include "utils/skipsupport.h"
 
-- 
2.47.3


--smaq5tawmbjl6jcu--