Revise tree-walk APIs to improve spec compliance & silence warnings.

expression_tree_walker and allied functions have traditionally
declared their callback functions as, say, "bool (*walker) ()"
to allow for variation in the declared types of the callback
functions' context argument.  This is apparently going to be
forbidden by the next version of the C standard, and the latest
version of clang warns about that.  In any case it's always
been pretty poor for error-detection purposes, so fixing it is
a good thing to do.

What we want to do is change the callback argument declarations to
be like "bool (*walker) (Node *node, void *context)", which is
correct so far as expression_tree_walker and friends are concerned,
but not change the actual callback functions.  Strict compliance with
the C standard would require changing them to declare their arguments
as "void *context" and then cast to the appropriate context struct
type internally.  That'd be very invasive and it would also introduce
a bunch of opportunities for future bugs, since we'd no longer have
any check that the correct sort of context object is passed by outside
callers or internal recursion cases.  Therefore, we're just going
to ignore the standard's position that "void *" isn't necessarily
compatible with struct pointers.  No machine built in the last forty
or so years actually behaves that way, so it's not worth introducing
bug hazards for compatibility with long-dead hardware.

Therefore, to silence these compiler warnings, introduce a layer of
macro wrappers that cast the supplied function name to the official
argument type.  Thanks to our use of -Wcast-function-type, this will
still produce a warning if the supplied function is seriously
incompatible with the required signature, without going as far as
the official spec restriction does.

This method fixes the problem without any need for source code changes
outside nodeFuncs.h/.c.  However, it is an ABI break because the
physically called functions now have names ending in "_impl".  Hence
we can only fix it this way in HEAD.  In the back branches, we'll have
to settle for disabling -Wdeprecated-non-prototype.

Discussion: https://postgr.es/m/CA+hUKGKpHPDTv67Y+s6yiC8KH5OXeDg6a-twWo_xznKTcG0kSA@mail.gmail.com
This commit is contained in:
Tom Lane 2022-09-20 18:03:22 -04:00
parent eccb607e19
commit 1c27d16e6e
2 changed files with 387 additions and 330 deletions

File diff suppressed because it is too large Load Diff

View File

@ -15,6 +15,8 @@
#include "nodes/parsenodes.h"
struct PlanState; /* avoid including execnodes.h too */
/* flags bits for query_tree_walker and query_tree_mutator */
#define QTW_IGNORE_RT_SUBQUERIES 0x01 /* subqueries in rtable */
@ -32,6 +34,14 @@
/* callback function for check_functions_in_node */
typedef bool (*check_function_callback) (Oid func_id, void *context);
/* callback functions for tree walkers */
typedef bool (*tree_walker_callback) (Node *node, void *context);
typedef bool (*planstate_tree_walker_callback) (struct PlanState *planstate,
void *context);
/* callback functions for tree mutators */
typedef Node *(*tree_mutator_callback) (Node *node, void *context);
extern Oid exprType(const Node *expr);
extern int32 exprTypmod(const Node *expr);
@ -129,34 +139,84 @@ get_notclausearg(const void *notclause)
extern bool check_functions_in_node(Node *node, check_function_callback checker,
void *context);
extern bool expression_tree_walker(Node *node, bool (*walker) (),
void *context);
extern Node *expression_tree_mutator(Node *node, Node *(*mutator) (),
void *context);
/*
* The following functions are usually passed walker or mutator callbacks
* that are declared like "bool walker(Node *node, my_struct *context)"
* rather than "bool walker(Node *node, void *context)" as a strict reading
* of the C standard would require. Changing the callbacks' declarations
* to "void *" would create serious hazards of passing them the wrong context
* struct type, so we respectfully decline to support the standard's position
* that a pointer to struct is incompatible with "void *". Instead, silence
* related compiler warnings by inserting casts into these macro wrappers.
*/
extern bool query_tree_walker(Query *query, bool (*walker) (),
void *context, int flags);
extern Query *query_tree_mutator(Query *query, Node *(*mutator) (),
void *context, int flags);
#define expression_tree_walker(n, w, c) \
expression_tree_walker_impl(n, (tree_walker_callback) (w), c)
#define expression_tree_mutator(n, m, c) \
expression_tree_mutator_impl(n, (tree_mutator_callback) (m), c)
extern bool range_table_walker(List *rtable, bool (*walker) (),
void *context, int flags);
extern List *range_table_mutator(List *rtable, Node *(*mutator) (),
void *context, int flags);
#define query_tree_walker(q, w, c, f) \
query_tree_walker_impl(q, (tree_walker_callback) (w), c, f)
#define query_tree_mutator(q, m, c, f) \
query_tree_mutator_impl(q, (tree_mutator_callback) (m), c, f)
extern bool range_table_entry_walker(RangeTblEntry *rte, bool (*walker) (),
void *context, int flags);
#define range_table_walker(rt, w, c, f) \
range_table_walker_impl(rt, (tree_walker_callback) (w), c, f)
#define range_table_mutator(rt, m, c, f) \
range_table_mutator_impl(rt, (tree_mutator_callback) (m), c, f)
extern bool query_or_expression_tree_walker(Node *node, bool (*walker) (),
void *context, int flags);
extern Node *query_or_expression_tree_mutator(Node *node, Node *(*mutator) (),
void *context, int flags);
#define range_table_entry_walker(r, w, c, f) \
range_table_entry_walker_impl(r, (tree_walker_callback) (w), c, f)
extern bool raw_expression_tree_walker(Node *node, bool (*walker) (),
#define query_or_expression_tree_walker(n, w, c, f) \
query_or_expression_tree_walker_impl(n, (tree_walker_callback) (w), c, f)
#define query_or_expression_tree_mutator(n, m, c, f) \
query_or_expression_tree_mutator_impl(n, (tree_mutator_callback) (m), c, f)
#define raw_expression_tree_walker(n, w, c) \
raw_expression_tree_walker_impl(n, (tree_walker_callback) (w), c)
#define planstate_tree_walker(ps, w, c) \
planstate_tree_walker_impl(ps, (planstate_tree_walker_callback) (w), c)
extern bool expression_tree_walker_impl(Node *node,
tree_walker_callback walker,
void *context);
extern Node *expression_tree_mutator_impl(Node *node,
tree_mutator_callback mutator,
void *context);
extern bool query_tree_walker_impl(Query *query,
tree_walker_callback walker,
void *context, int flags);
extern Query *query_tree_mutator_impl(Query *query,
tree_mutator_callback mutator,
void *context, int flags);
extern bool range_table_walker_impl(List *rtable,
tree_walker_callback walker,
void *context, int flags);
extern List *range_table_mutator_impl(List *rtable,
tree_mutator_callback mutator,
void *context, int flags);
extern bool range_table_entry_walker_impl(RangeTblEntry *rte,
tree_walker_callback walker,
void *context, int flags);
extern bool query_or_expression_tree_walker_impl(Node *node,
tree_walker_callback walker,
void *context, int flags);
extern Node *query_or_expression_tree_mutator_impl(Node *node,
tree_mutator_callback mutator,
void *context, int flags);
extern bool raw_expression_tree_walker_impl(Node *node,
tree_walker_callback walker,
void *context);
extern bool planstate_tree_walker_impl(struct PlanState *planstate,
planstate_tree_walker_callback walker,
void *context);
struct PlanState;
extern bool planstate_tree_walker(struct PlanState *planstate, bool (*walker) (),
void *context);
#endif /* NODEFUNCS_H */