Horton

Re: allow benign typedef redefinitions (C11)

Chao Li <li.evan.chao@gmail.com>

View thread View on postgr.es
From: Chao Li <li.evan.chao@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-09-01T03:32:11Z
Lists: pgsql-hackers 

> On Aug 29, 2025, at 20:45, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> (After this, it would probably be possible to undertake some deeper efforts to untangle header files, with the help of IWYU.  But that is a separate project.)
> <0001-Improve-pgbench-definition-of-yyscan_t.patch><0002-Allow-redeclaration-of-typedef-yyscan_t.patch><0003-Remove-workarounds-against-repeat-typedefs.patch><0004-Improve-ExplainState-type-handling-in-header-files.patch><0005-Update-various-forward-declarations-to-use-typedef.patch><0006-Remove-hbaPort-type.patch><0007-Change-fmgr.h-typedefs-to-use-original-names.patch>



I applied the patch files and build passed without any error and warning on my MacBook.

Just a few small comments:

1. For 0001, I think “#ifdef YY_TYPEDEF_YY_SCANNER_T” is not needed, but I see you have removed that in 0002, so this is not a comment now.

2. For 0002, looks like som duplicate code for definitions of

Union YYSTYPE;
#define YYLTYPE int
Typedef void *yyscan_t;

Can we put them into a separate header file say src/include/yy_types.h?

3. For 0002, this is not your change, I am just pointing it out: “#define YYSTYPE char *”, where “YYSTYPE” is defined as “union” everywhere else. Can we rename it to something like “YYSTYPE_CHARPTR” here to avoid confusion? I tried to rename it and build still passed.

4. For 0004

diff --git a/src/include/commands/explain_dr.h b/src/include/commands/explain_dr.h
index 55da63d66bd..ce424aa2a55 100644
--- a/src/include/commands/explain_dr.h
+++ b/src/include/commands/explain_dr.h
@@ -16,7 +16,7 @@
 #include "executor/instrument.h"
 #include "tcop/dest.h"

-struct ExplainState;                   /* avoid including explain.h here */
+typedef struct ExplainState ExplainState;      /* avoid including explain.h here */

“Struct ExplainState” is defined in explain_state.h, so the comment here should be “avoid including explain_state.h here”.

Same thing applies to explain_format.h and fdwapi.h.

5. For 0005

diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 03b214755c2..d490ef3b506 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -35,9 +35,9 @@ typedef struct IndexOptInfo IndexOptInfo;
 typedef struct SpecialJoinInfo SpecialJoinInfo;

 /* It also seems best not to include plannodes.h, params.h, or htup.h here */
-struct PlannedStmt;
-struct ParamListInfoData;
-struct HeapTupleData;
+typedef struct PlannedStmt PlannedStmt;
+typedef struct ParamListInfoData ParamListInfoData;
+typedef struct HeapTupleData *HeapTuple;

Why don’t define ParamListInfo in the same way as HeapTuple like “typedef structure ParamListInfoData *ParamListInfo”. I tried that in my local and build still passed.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Commits

  1. Move instrumentation-related structs to instrument_node.h

  2. Don't include access/htup_details.h in executor/tuptable.h

  3. Don't include execnodes.h in brin.h or gin.h

  4. Do a tiny bit of header file maintenance

  5. Don't include execnodes.h in replication/conflict.h

  6. Update some more forward declarations to use typedef

  7. Change fmgr.h typedefs to use original names

  8. Improve ExplainState type handling in header files

  9. Remove hbaPort type

  10. Remove workarounds against repeat typedefs

  11. Update various forward declarations to use typedef

  12. Allow redeclaration of typedef yyscan_t

  13. Improve pgbench definition of yyscan_t

  14. Delay extraction of TIDBitmap per page offsets

  15. Remove pgrminclude and associated scripts

  16. Allow to use HeapTupleData embedded in [Buffer]HeapTupleTableSlot.