Horton

Re: allow benign typedef redefinitions (C11)

Peter Eisentraut <peter@eisentraut.org>

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

Attachments

On 01.09.25 05:32, Chao Li wrote:
> 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.

Yes, this is just so that the code after applying patch 0001 still 
works, even if some code is later removed.

> 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?

Each scanner/parser might have its own idea of what type it wants to 
use, so this might not work.  It's better not to get into the 
complications of this for this patch set.

> 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.

The name YYSTYPE is required by bison.  I'm not sure how it still works 
without it.  But anyway, see above, it's probably better not to crack 
open the bison/flex mess again here.

> 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.

Yes, good catch.  The original patch that introduced these comments 
actually did replace an include of explain.h, but arguably even that was 
already too broad.  So I agree we should fix these comments.

> 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.

Yes, that seems more correct.

Thanks.  New patch set attached.

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.