> On Aug 29, 2025, at 20:45, Peter Eisentraut <pe...@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/
