Обсуждение: [COMMITTERS] pgsql: Generate fmgr prototypes automatically
Generate fmgr prototypes automatically Gen_fmgrtab.pl creates a new file fmgrprotos.h, which contains prototypes for all functions registered in pg_proc.h. This avoids having to manually maintain these prototypes across a random variety of header files. It also automatically enforces a correct function signature, and since there are warnings about missing prototypes, it will detect functions that are defined but not registered in pg_proc.h (or otherwise used). Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/352a24a1f9d6f7d4abb1175bfd22acc358f43140 Modified Files -------------- contrib/btree_gist/btree_bit.c | 1 + contrib/btree_gist/btree_bytea.c | 1 + contrib/btree_gist/btree_date.c | 1 + contrib/btree_gist/btree_interval.c | 1 + contrib/btree_gist/btree_time.c | 1 + contrib/intarray/_int_selfuncs.c | 1 + contrib/lo/lo.c | 2 +- contrib/spi/moddatetime.c | 2 +- src/backend/Makefile | 15 +- src/backend/access/brin/brin.c | 1 + src/backend/access/brin/brin_inclusion.c | 5 +- src/backend/access/brin/brin_minmax.c | 5 +- src/backend/access/gin/ginfast.c | 1 + src/backend/access/gist/gist.c | 1 + src/backend/access/hash/hash.c | 1 + src/backend/access/hash/hashfunc.c | 10 + src/backend/access/index/amapi.c | 1 + src/backend/access/nbtree/nbtree.c | 1 + src/backend/access/spgist/spgutils.c | 1 + src/backend/access/transam/xlogfuncs.c | 1 - src/backend/catalog/namespace.c | 16 - src/backend/catalog/pg_proc.c | 4 - src/backend/commands/define.c | 2 +- src/backend/executor/nodeSamplescan.c | 1 + src/backend/foreign/foreign.c | 4 - src/backend/storage/smgr/smgrtype.c | 1 + src/backend/tsearch/dict_ispell.c | 1 + src/backend/tsearch/dict_simple.c | 1 + src/backend/tsearch/dict_synonym.c | 1 + src/backend/tsearch/ts_selfuncs.c | 1 + src/backend/utils/.gitignore | 1 + src/backend/utils/Gen_fmgrtab.pl | 35 +- src/backend/utils/Makefile | 12 +- src/backend/utils/adt/array_selfuncs.c | 1 + src/backend/utils/adt/array_typanalyze.c | 1 + src/backend/utils/adt/ascii.c | 1 + src/backend/utils/adt/geo_selfuncs.c | 1 + src/backend/utils/adt/jsonb_op.c | 1 + src/backend/utils/adt/network_gist.c | 1 + src/backend/utils/adt/network_selfuncs.c | 1 + src/backend/utils/adt/network_spgist.c | 1 + src/backend/utils/adt/numeric.c | 6 +- src/backend/utils/adt/pg_upgrade_support.c | 12 - src/backend/utils/adt/pgstatfuncs.c | 100 --- src/backend/utils/adt/rangetypes_spgist.c | 7 - src/backend/utils/adt/tsgistidx.c | 1 + src/backend/utils/adt/tsquery_gist.c | 1 + src/backend/utils/adt/tsquery_op.c | 1 + src/backend/utils/adt/tsrank.c | 1 + src/backend/utils/adt/tsvector.c | 1 + src/backend/utils/adt/varbit.c | 1 + src/include/Makefile | 3 +- src/include/access/amapi.h | 2 - src/include/access/brin.h | 6 - src/include/access/gin_private.h | 4 - src/include/access/gist_private.h | 1 - src/include/access/hash.h | 22 - src/include/access/nbtree.h | 1 - src/include/access/spgist.h | 1 - src/include/access/xlog_fn.h | 37 - src/include/catalog/pg_proc.h | 1 + src/include/commands/async.h | 5 - src/include/commands/sequence.h | 8 - src/include/commands/trigger.h | 2 - src/include/libpq/be-fsstubs.h | 31 - src/include/replication/logicalfuncs.h | 7 - src/include/replication/origin.h | 14 - src/include/replication/slot.h | 6 - src/include/replication/walreceiver.h | 1 - src/include/replication/walsender.h | 2 - src/include/storage/smgr.h | 6 - src/include/tsearch/ts_type.h | 89 --- src/include/tsearch/ts_utils.h | 117 --- src/include/utils/.gitignore | 1 + src/include/utils/acl.h | 14 - src/include/utils/array.h | 50 -- src/include/utils/ascii.h | 6 - src/include/utils/builtins.h | 1186 +--------------------------- src/include/utils/bytea.h | 25 - src/include/utils/cash.h | 48 -- src/include/utils/date.h | 113 --- src/include/utils/datetime.h | 3 - src/include/utils/formatting.h | 12 - src/include/utils/geo_decls.h | 262 +----- src/include/utils/inet.h | 27 - src/include/utils/int8.h | 104 --- src/include/utils/json.h | 64 -- src/include/utils/jsonb.h | 67 -- src/include/utils/nabstime.h | 63 -- src/include/utils/pg_lsn.h | 16 - src/include/utils/rangetypes.h | 80 -- src/include/utils/selfuncs.h | 28 - src/include/utils/snapmgr.h | 1 - src/include/utils/timestamp.h | 125 --- src/include/utils/varbit.h | 46 -- src/include/utils/xml.h | 33 - src/tools/msvc/Solution.pm | 10 +- src/tools/msvc/clean.bat | 2 + 98 files changed, 125 insertions(+), 2899 deletions(-)
Peter Eisentraut <peter_e@gmx.net> writes:
> Generate fmgr prototypes automatically
Buildfarm's quite unhappy with this. Did you check parallel make?
regards, tom lane
Peter Eisentraut <peter_e@gmx.net> writes:
> Generate fmgr prototypes automatically
BTW, now that I've looked through this patch ... why does it add
+#include "nodes/nodes.h"
+#include "nodes/pg_list.h"
to utils/builtins.h? AFAICS that shouldn't be necessary, since
there are no declarations in builtins.h that weren't there before.
And seeing that this results in builtins.h being included in even more
places than before, surely we want to minimize that file's inclusion
footprint.
regards, tom lane
On 1/17/17 2:35 PM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> Generate fmgr prototypes automatically > > BTW, now that I've looked through this patch ... why does it add > > +#include "nodes/nodes.h" > +#include "nodes/pg_list.h" > > to utils/builtins.h? AFAICS that shouldn't be necessary, since > there are no declarations in builtins.h that weren't there before. > And seeing that this results in builtins.h being included in even more > places than before, surely we want to minimize that file's inclusion > footprint. There are things in builtins.h that technically need those declarations. It seems to work without it now, but at some point during the refactoring they were necessary. I can remove them again. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 1/17/17 2:35 PM, Tom Lane wrote:
>> BTW, now that I've looked through this patch ... why does it add
>>
>> +#include "nodes/nodes.h"
>> +#include "nodes/pg_list.h"
>>
>> to utils/builtins.h?
> There are things in builtins.h that technically need those declarations.
> It seems to work without it now, but at some point during the
> refactoring they were necessary. I can remove them again.
[ looks around ... ] Hm, it looks like the reason it works without
them is that utils/sortsupport.h pulls in relcache.h, which pulls in a
veritable buttload of stuff; those two and a whole lot more beside.
That seems like a pretty bad idea. It's surely not the fault of this
patch that the sortsupport patch didn't give a damn about inclusion
footprints; but I still think we'd be well advised to minimize
builtins.h's footprint now that it's going to need to be included very
far and wide.
It looks like we could get rid of the need for sortsupport.h in
builtins.h via the good old "struct foo" forward reference trick,
that is
struct SortSupportData;
extern void varstr_sortsupport(struct SortSupportData *ssup, Oid collid, bool bpchar);
but I wonder if that's going far enough. We'd still be propagating
pg_list.h into a lot of places that don't necessarily need it.
Alternatively ... is there a specific reason why you chose to make
builtins.h the key inclusion file for this change, rather than having
callers include fmgrprotos.h directly? It seems like the stuff remaining
in builtins.h is just a laundry list of random utility functions.
Maybe dispersing those to other headers is the thing to do.
regards, tom lane