simplehash vs. pgindent

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers