Thread: [HACKERS] simplehash vs. pgindent

In commit b30d3ea824c5ccba43d3e942704f20686e7dbab8, when Andres added
the simplehash stuff, he also added SH_TYPE, SH_ITERATOR, and
SH_STATUS to typedefs.list.  When I subsequently updated typedefs.list
from the buildfarm in acddbe221b084956a0efd6e4b6c6586e8fd994d7, it of
course removed those entries since they are merely placeholders for
real types, not anything that would ever appear in a symbol table.  So
now if you pgindent the simplehash stuff, it does stupid things.

I think we should add a file src/tools/pgindent/typedefs.extra.list.
Somehow teach pgindent to search whatever directory it searches for
typedefs.list for typedefs.extra.list as well.  Then we can put stuff
in there that the buildfarm doesn't find, and pgindent can combine the
files in load_typedefs().  That way we can still update typedefs.list
straight from the buildfarm, but there's a place to put manual entries
that we don't want to lose.  We could also teach load_typedefs() to
strip out lines that are empty or start with #, and then we'd be able
to put comments in typedefs.extra.list explaining why each entry needs
to be present there rather than relying on the normal mechanism.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> In commit b30d3ea824c5ccba43d3e942704f20686e7dbab8, when Andres added
> the simplehash stuff, he also added SH_TYPE, SH_ITERATOR, and
> SH_STATUS to typedefs.list.  When I subsequently updated typedefs.list
> from the buildfarm in acddbe221b084956a0efd6e4b6c6586e8fd994d7, it of
> course removed those entries since they are merely placeholders for
> real types, not anything that would ever appear in a symbol table.  So
> now if you pgindent the simplehash stuff, it does stupid things.

> I think we should add a file src/tools/pgindent/typedefs.extra.list.
> Somehow teach pgindent to search whatever directory it searches for
> typedefs.list for typedefs.extra.list as well.  Then we can put stuff
> in there that the buildfarm doesn't find, and pgindent can combine the
> files in load_typedefs().  That way we can still update typedefs.list
> straight from the buildfarm, but there's a place to put manual entries
> that we don't want to lose.  We could also teach load_typedefs() to
> strip out lines that are empty or start with #, and then we'd be able
> to put comments in typedefs.extra.list explaining why each entry needs
> to be present there rather than relying on the normal mechanism.

Or we could just fix the code so it doesn't use phony typedefs?
If a typedef is never used as a variable or field type, how much
do we really need it?
        regards, tom lane

On 2016-12-21 00:32:22 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > In commit b30d3ea824c5ccba43d3e942704f20686e7dbab8, when Andres added
> > the simplehash stuff, he also added SH_TYPE, SH_ITERATOR, and
> > SH_STATUS to typedefs.list.  When I subsequently updated typedefs.list
> > from the buildfarm in acddbe221b084956a0efd6e4b6c6586e8fd994d7, it of
> > course removed those entries since they are merely placeholders for
> > real types, not anything that would ever appear in a symbol table.  So
> > now if you pgindent the simplehash stuff, it does stupid things.

> > I think we should add a file src/tools/pgindent/typedefs.extra.list.
> > Somehow teach pgindent to search whatever directory it searches for
> > typedefs.list for typedefs.extra.list as well.  Then we can put stuff
> > in there that the buildfarm doesn't find, and pgindent can combine the
> > files in load_typedefs().  That way we can still update typedefs.list
> > straight from the buildfarm, but there's a place to put manual entries
> > that we don't want to lose.  We could also teach load_typedefs() to
> > strip out lines that are empty or start with #, and then we'd be able
> > to put comments in typedefs.extra.list explaining why each entry needs
> > to be present there rather than relying on the normal mechanism.

That sounds sane.


> Or we could just fix the code so it doesn't use phony typedefs?
> If a typedef is never used as a variable or field type, how much
> do we really need it?

I'm not sure what you mean by that. The typedefs are used, it's just
that the resultof the macros (aforementioned SH_TYPE etc) are named
differently than the macro name itself, and thus it's not contained in
the generated typedefs list.

We obviously can add a 'struct ' everywhere, but that doesn't really
seem to conform to style?

Regards,

Andres