warning when compiling utils/tqual.h

I noticed (by running "cd src/include ; make check" with the attached
patch applied) that since commit b89e151054 ("Introduce logical
decoding.") tqual.h now emits this warning when compiled standalone:

/pgsql/source/HEAD/src/include/utils/tqual.h:101:13: warning: ‘struct HTAB’ declared inside parameter list [enabled by default]
/pgsql/source/HEAD/src/include/utils/tqual.h:101:13: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]

The prototype in question is:

/*
* To avoid leaking to much knowledge about reorderbuffer implementation
* details this is implemented in reorderbuffer.c not tqual.c.
*/
extern bool ResolveCminCmaxDuringDecoding(HTAB *tuplecid_data,
Snapshot snapshot,
HeapTuple htup,
Buffer buffer,
CommandId *cmin, CommandId *cmax);

Now there are two easy ways to silence this:

1. add "struct HTAB;" to the previous line in tqual.h, i.e. a forward
declaration. We have very few of these; most of the time ISTM we prefer
a solution like the next one:

2. add #include "utils/hsearch.h" at the top of tqual.h.

Now my inclination is to use the second one, because it seems cleaner to
me and avoid having a forward struct decl in an unrelated header, which
will later bleed into other source files. Do we have a project
guideline saying that we prefer option two, and does that explain why we
have so few declarations as in option one?

There is of course a third choice which is to dictate that this function
ought to be declared in reorderbuffer.h; but that would have the
unpleasant side-effect that tqual.c would need to #include that.

Opinions please? I would like to have a guideline about this so I can
reason in a principled way in future cases in which this kind of
question arises.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services